[gjs: 1/2] cairo: Add type checking to Path, Pattern, and Surface

From: Philip Chimento <pchimento src gnome org>
To: commits-list gnome org
Cc:
Subject: [gjs: 1/2] cairo: Add type checking to Path, Pattern, and Surface
Date: Wed, 20 Nov 2019 00:11:10 +0000 (UTC)
commit e7ca4d37ed3446ede0daa0a99d5488b5c37d0f20
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Oct 26 22:52:35 2019 -0700

    cairo: Add type checking to Path, Pattern, and Surface
    
    Previously, passing the wrong kind of object where a Cairo.Path,
    Cairo.Pattern (subclass), or Cairo.Surface (subclass) was expected,
    would crash. This adds type checking to avoid these crashes.
    
    The normal way to do this would be with JS_HasInstance(), but since
    Cairo.Pattern and Cairo.Surface are abstract classes, that won't work.
    JS_HasInstance() takes a constructor, and abstract classes don't have a
    constructor. Instead, we have to check the passed-in object's prototype
    chain. It turns out there isn't a JSAPI function to do that, so we add
    gjs_object_in_prototype_chain() for this purpose.
    
    In addition we add missing error checking in a few places.
    
    Closes: #49.

 gjs/jsapi-class.h                 |  5 ++++
 gjs/jsapi-dynamic-class.cpp       | 35 +++++++++++++++++++++++++
 installed-tests/js/testCairo.js   | 22 ++++++++++++++++
 modules/cairo-context.cpp         | 47 ++++++++++++---------------------
 modules/cairo-gradient.cpp        | 11 +++++---
 modules/cairo-image-surface.cpp   | 24 +++++++++++------
 modules/cairo-path.cpp            | 30 ++++++++++-----------
 modules/cairo-pattern.cpp         | 39 ++++++++++++++++-----------
 modules/cairo-private.h           | 18 ++++++-------
 modules/cairo-surface-pattern.cpp | 32 ++++++++++++++---------
 modules/cairo-surface.cpp         | 55 +++++++++++++++++++++++++--------------
 11 files changed, 204 insertions(+), 114 deletions(-)
---
diff --git a/gjs/jsapi-class.h b/gjs/jsapi-class.h
index c090336f..757a65dd 100644
--- a/gjs/jsapi-class.h
+++ b/gjs/jsapi-class.h
@@ -325,4 +325,9 @@ GJS_DEFINE_PROTO_FUNCS_WITH_PARENT(cname, no_parent)
 GJS_USE
 JS::Value gjs_dynamic_property_private_slot(JSObject *accessor_obj);
 
+GJS_JSAPI_RETURN_CONVENTION
+bool gjs_object_in_prototype_chain(JSContext* cx, JS::HandleObject proto,
+                                   JS::HandleObject check_obj,
+                                   bool* is_in_chain);
+
 #endif  // GJS_JSAPI_CLASS_H_
diff --git a/gjs/jsapi-dynamic-class.cpp b/gjs/jsapi-dynamic-class.cpp
index 3849bb07..6b6cb427 100644
--- a/gjs/jsapi-dynamic-class.cpp
+++ b/gjs/jsapi-dynamic-class.cpp
@@ -265,3 +265,38 @@ gjs_dynamic_property_private_slot(JSObject *accessor_obj)
     return js::GetFunctionNativeReserved(accessor_obj,
                                          DYNAMIC_PROPERTY_PRIVATE_SLOT);
 }
+
+/**
+ * gjs_object_in_prototype_chain:
+ * @cx:
+ * @proto: The prototype which we are checking if @check_obj has in its chain
+ * @check_obj: The object to check
+ * @is_in_chain: (out): Whether @check_obj has @proto in its prototype chain
+ *
+ * Similar to JS_HasInstance() but takes into account abstract classes defined
+ * with JS_InitClass(), which JS_HasInstance() does not. Abstract classes don't
+ * have constructors, and JS_HasInstance() requires a constructor.
+ *
+ * Returns: false if an exception was thrown, true otherwise.
+ */
+bool gjs_object_in_prototype_chain(JSContext* cx, JS::HandleObject proto,
+                                   JS::HandleObject check_obj,
+                                   bool* is_in_chain) {
+    JS::RootedObject object_prototype(cx, JS_GetObjectPrototype(cx, check_obj));
+    if (!object_prototype)
+        return false;
+
+    JS::RootedObject proto_iter(cx);
+    if (!JS_GetPrototype(cx, check_obj, &proto_iter))
+        return false;
+    while (proto_iter != object_prototype) {
+        if (proto_iter == proto) {
+            *is_in_chain = true;
+            return true;
+        }
+        if (!JS_GetPrototype(cx, proto_iter, &proto_iter))
+            return false;
+    }
+    *is_in_chain = false;
+    return true;
+}
diff --git a/installed-tests/js/testCairo.js b/installed-tests/js/testCairo.js
index f3d6fd41..08686960 100644
--- a/installed-tests/js/testCairo.js
+++ b/installed-tests/js/testCairo.js
@@ -177,6 +177,13 @@ describe('Cairo', function () {
         });
     });
 
+    describe('pattern', function () {
+        it('has typechecks', function () {
+            expect(() => cr.setSource({})).toThrow();
+            expect(() => cr.setSource(surface)).toThrow();
+        });
+    });
+
     describe('solid pattern', function () {
         it('can be created from RGB static method', function () {
             let p1 = Cairo.SolidPattern.createRGB(1, 2, 3);
@@ -220,6 +227,21 @@ describe('Cairo', function () {
         });
     });
 
+    describe('path', function () {
+        it('has typechecks', function () {
+            expect(() => cr.appendPath({})).toThrow();
+            expect(() => cr.appendPath(surface)).toThrow();
+        });
+    });
+
+    describe('surface', function () {
+        it('has typechecks', function () {
+            expect(() => new Cairo.Context({})).toThrow();
+            const pattern = new Cairo.SurfacePattern(surface);
+            expect(() => new Cairo.Context(pattern)).toThrow();
+        });
+    });
+
     describe('GI test suite', function () {
         describe('for context', function () {
             it('can be marshalled as a return value', function () {
diff --git a/modules/cairo-context.cpp b/modules/cairo-context.cpp
index 963d7663..e7b79632 100644
--- a/modules/cairo-context.cpp
+++ b/modules/cairo-context.cpp
@@ -268,7 +268,6 @@ _gjs_cairo_context_construct_internal(JSContext       *context,
 GJS_NATIVE_CONSTRUCTOR_DECLARE(cairo_context)
 {
     GJS_NATIVE_CONSTRUCTOR_VARIABLES(cairo_context)
-    cairo_surface_t *surface;
     cairo_t *cr;
 
     GJS_NATIVE_CONSTRUCTOR_PRELUDE(cairo_context);
@@ -278,11 +277,10 @@ GJS_NATIVE_CONSTRUCTOR_DECLARE(cairo_context)
                              "surface", &surface_wrapper))
         return false;
 
-    surface = gjs_cairo_surface_get_surface(context, surface_wrapper);
-    if (!surface) {
-        gjs_throw(context, "first argument to Context() should be a surface");
+    cairo_surface_t* surface =
+        gjs_cairo_surface_get_surface(context, surface_wrapper);
+    if (!surface)
         return false;
-    }
 
     cr = cairo_create(surface);
 
@@ -415,18 +413,15 @@ appendPath_func(JSContext *context,
 {
     GJS_GET_PRIV(context, argc, vp, argv, obj, GjsCairoContext, priv);
     JS::RootedObject path_wrapper(context);
-    cairo_path_t *path;
     cairo_t* cr = priv ? priv->cr : nullptr;
 
     if (!gjs_parse_call_args(context, "path", argv, "o",
                              "path", &path_wrapper))
         return false;
 
-    path = gjs_cairo_path_get_path(context, path_wrapper);
-    if (!path) {
-        gjs_throw(context, "first argument to appendPath() should be a path");
+    cairo_path_t* path = gjs_cairo_path_get_path(context, path_wrapper);
+    if (!path)
         return false;
-    }
 
     cairo_append_path(cr, path);
     argv.rval().setUndefined();
@@ -477,18 +472,16 @@ mask_func(JSContext *context,
 {
     GJS_GET_PRIV(context, argc, vp, argv, obj, GjsCairoContext, priv);
     JS::RootedObject pattern_wrapper(context);
-    cairo_pattern_t *pattern;
     cairo_t* cr = priv ? priv->cr : nullptr;
 
     if (!gjs_parse_call_args(context, "mask", argv, "o",
                              "pattern", &pattern_wrapper))
         return false;
 
-    pattern = gjs_cairo_pattern_get_pattern(context, pattern_wrapper);
-    if (!pattern) {
-        gjs_throw(context, "first argument to mask() should be a pattern");
+    cairo_pattern_t* pattern =
+        gjs_cairo_pattern_get_pattern(context, pattern_wrapper);
+    if (!pattern)
         return false;
-    }
 
     cairo_mask(cr, pattern);
 
@@ -508,7 +501,6 @@ maskSurface_func(JSContext *context,
     GJS_GET_PRIV(context, argc, vp, argv, obj, GjsCairoContext, priv);
     JS::RootedObject surface_wrapper(context);
     double x, y;
-    cairo_surface_t *surface;
     cairo_t* cr = priv ? priv->cr : nullptr;
 
     if (!gjs_parse_call_args(context, "maskSurface", argv, "off",
@@ -517,11 +509,10 @@ maskSurface_func(JSContext *context,
                              "y", &y))
         return false;
 
-    surface = gjs_cairo_surface_get_surface(context, surface_wrapper);
-    if (!surface) {
-        gjs_throw(context, "first argument to maskSurface() should be a surface");
+    cairo_surface_t* surface =
+        gjs_cairo_surface_get_surface(context, surface_wrapper);
+    if (!surface)
         return false;
-    }
 
     cairo_mask_surface(cr, surface, x, y);
 
@@ -599,18 +590,16 @@ setSource_func(JSContext *context,
 {
     GJS_GET_PRIV(context, argc, vp, argv, obj, GjsCairoContext, priv);
     JS::RootedObject pattern_wrapper(context);
-    cairo_pattern_t *pattern;
     cairo_t* cr = priv ? priv->cr : nullptr;
 
     if (!gjs_parse_call_args(context, "setSource", argv, "o",
                              "pattern", &pattern_wrapper))
         return false;
 
-    pattern = gjs_cairo_pattern_get_pattern(context, pattern_wrapper);
-    if (!pattern) {
-        gjs_throw(context, "first argument to setSource() should be a pattern");
+    cairo_pattern_t* pattern =
+        gjs_cairo_pattern_get_pattern(context, pattern_wrapper);
+    if (!pattern)
         return false;
-    }
 
     cairo_set_source(cr, pattern);
 
@@ -631,7 +620,6 @@ setSourceSurface_func(JSContext *context,
     GJS_GET_PRIV(context, argc, vp, argv, obj, GjsCairoContext, priv);
     JS::RootedObject surface_wrapper(context);
     double x, y;
-    cairo_surface_t *surface;
     cairo_t* cr = priv ? priv->cr : nullptr;
 
     if (!gjs_parse_call_args(context, "setSourceSurface", argv, "off",
@@ -640,11 +628,10 @@ setSourceSurface_func(JSContext *context,
                              "y", &y))
         return false;
 
-    surface = gjs_cairo_surface_get_surface(context, surface_wrapper);
-    if (!surface) {
-        gjs_throw(context, "first argument to setSourceSurface() should be a surface");
+    cairo_surface_t* surface =
+        gjs_cairo_surface_get_surface(context, surface_wrapper);
+    if (!surface)
         return false;
-    }
 
     cairo_set_source_surface(cr, surface, x, y);
 
diff --git a/modules/cairo-gradient.cpp b/modules/cairo-gradient.cpp
index e6f7461b..05c086d5 100644
--- a/modules/cairo-gradient.cpp
+++ b/modules/cairo-gradient.cpp
@@ -56,7 +56,6 @@ addColorStopRGB_func(JSContext *context,
 {
     GJS_GET_THIS(context, argc, vp, argv, obj);
     double offset, red, green, blue;
-    cairo_pattern_t *pattern;
 
     if (!gjs_parse_call_args(context, "addColorStopRGB", argv, "ffff",
                              "offset", &offset,
@@ -65,7 +64,9 @@ addColorStopRGB_func(JSContext *context,
                              "blue", &blue))
         return false;
 
-    pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    cairo_pattern_t* pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    if (!pattern)
+        return false;
 
     cairo_pattern_add_color_stop_rgb(pattern, offset, red, green, blue);
 
@@ -84,7 +85,6 @@ addColorStopRGBA_func(JSContext *context,
 {
     GJS_GET_THIS(context, argc, vp, argv, obj);
     double offset, red, green, blue, alpha;
-    cairo_pattern_t *pattern;
 
     if (!gjs_parse_call_args(context, "addColorStopRGBA", argv, "fffff",
                              "offset", &offset,
@@ -94,7 +94,10 @@ addColorStopRGBA_func(JSContext *context,
                              "alpha", &alpha))
         return false;
 
-    pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    cairo_pattern_t* pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    if (!pattern)
+        return false;
+
     cairo_pattern_add_color_stop_rgba(pattern, offset, red, green, blue, alpha);
 
     if (!gjs_cairo_check_status(context, cairo_pattern_status(pattern), "pattern"))
diff --git a/modules/cairo-image-surface.cpp b/modules/cairo-image-surface.cpp
index f2382e8b..590f2df9 100644
--- a/modules/cairo-image-surface.cpp
+++ b/modules/cairo-image-surface.cpp
@@ -117,7 +117,6 @@ getFormat_func(JSContext *context,
                JS::Value *vp)
 {
     GJS_GET_THIS(context, argc, vp, rec, obj);
-    cairo_surface_t *surface;
     cairo_format_t format;
 
     if (argc > 1) {
@@ -125,7 +124,10 @@ getFormat_func(JSContext *context,
         return false;
     }
 
-    surface = gjs_cairo_surface_get_surface(context, obj);
+    cairo_surface_t* surface = gjs_cairo_surface_get_surface(context, obj);
+    if (!surface)
+        return false;
+
     format = cairo_image_surface_get_format(surface);
 
     if (!gjs_cairo_check_status(context, cairo_surface_status(surface), "surface"))
@@ -142,7 +144,6 @@ getWidth_func(JSContext *context,
               JS::Value *vp)
 {
     GJS_GET_THIS(context, argc, vp, rec, obj);
-    cairo_surface_t *surface;
     int width;
 
     if (argc > 1) {
@@ -150,7 +151,10 @@ getWidth_func(JSContext *context,
         return false;
     }
 
-    surface = gjs_cairo_surface_get_surface(context, obj);
+    cairo_surface_t* surface = gjs_cairo_surface_get_surface(context, obj);
+    if (!surface)
+        return false;
+
     width = cairo_image_surface_get_width(surface);
 
     if (!gjs_cairo_check_status(context, cairo_surface_status(surface), "surface"))
@@ -167,7 +171,6 @@ getHeight_func(JSContext *context,
                JS::Value *vp)
 {
     GJS_GET_THIS(context, argc, vp, rec, obj);
-    cairo_surface_t *surface;
     int height;
 
     if (argc > 1) {
@@ -175,7 +178,10 @@ getHeight_func(JSContext *context,
         return false;
     }
 
-    surface = gjs_cairo_surface_get_surface(context, obj);
+    cairo_surface_t* surface = gjs_cairo_surface_get_surface(context, obj);
+    if (!surface)
+        return false;
+
     height = cairo_image_surface_get_height(surface);
 
     if (!gjs_cairo_check_status(context, cairo_surface_status(surface), "surface"))
@@ -192,7 +198,6 @@ getStride_func(JSContext *context,
                JS::Value *vp)
 {
     GJS_GET_THIS(context, argc, vp, rec, obj);
-    cairo_surface_t *surface;
     int stride;
 
     if (argc > 1) {
@@ -200,7 +205,10 @@ getStride_func(JSContext *context,
         return false;
     }
 
-    surface = gjs_cairo_surface_get_surface(context, obj);
+    cairo_surface_t* surface = gjs_cairo_surface_get_surface(context, obj);
+    if (!surface)
+        return false;
+
     stride = cairo_image_surface_get_stride(surface);
 
     if (!gjs_cairo_check_status(context, cairo_surface_status(surface), "surface"))
diff --git a/modules/cairo-path.cpp b/modules/cairo-path.cpp
index a5052515..62ed78f6 100644
--- a/modules/cairo-path.cpp
+++ b/modules/cairo-path.cpp
@@ -99,26 +99,26 @@ gjs_cairo_path_from_path(JSContext    *context,
     return object;
 }
 
-
 /**
  * gjs_cairo_path_get_path:
- * @context: the context
- * @object: path wrapper
+ * @cx: the context
+ * @path_wrapper: path wrapper
  *
  * Returns: the path attached to the wrapper.
- *
  */
-cairo_path_t *
-gjs_cairo_path_get_path(JSContext *context,
-                        JSObject  *object)
-{
-    GjsCairoPath *priv;
-
-    g_return_val_if_fail(context, nullptr);
-    g_return_val_if_fail(object, nullptr);
-
-    priv = (GjsCairoPath*) JS_GetPrivate(object);
-    if (!priv)
+cairo_path_t* gjs_cairo_path_get_path(JSContext* cx,
+                                      JS::HandleObject path_wrapper) {
+    g_return_val_if_fail(cx, nullptr);
+    g_return_val_if_fail(path_wrapper, nullptr);
+
+    auto* priv = static_cast<GjsCairoPath*>(JS_GetInstancePrivate(
+        cx, path_wrapper, &gjs_cairo_path_class, nullptr));
+    if (!priv) {
+        gjs_throw(cx, "Expected Cairo.Path but got %s",
+                  JS_GetClass(path_wrapper)->name);
         return nullptr;
+    }
+
+    g_assert(priv->path);
     return priv->path;
 }
diff --git a/modules/cairo-pattern.cpp b/modules/cairo-pattern.cpp
index c2a307dc..c9adf6c1 100644
--- a/modules/cairo-pattern.cpp
+++ b/modules/cairo-pattern.cpp
@@ -66,7 +66,6 @@ getType_func(JSContext *context,
              JS::Value *vp)
 {
     GJS_GET_THIS(context, argc, vp, rec, obj);
-    cairo_pattern_t *pattern;
     cairo_pattern_type_t type;
 
     if (argc > 1) {
@@ -74,7 +73,10 @@ getType_func(JSContext *context,
         return false;
     }
 
-    pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    cairo_pattern_t* pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    if (!pattern)
+        return false;
+
     type = cairo_pattern_get_type(pattern);
 
     if (!gjs_cairo_check_status(context, cairo_pattern_status(pattern), "pattern"))
@@ -183,25 +185,32 @@ gjs_cairo_pattern_from_pattern(JSContext       *context,
 
 /**
  * gjs_cairo_pattern_get_pattern:
- * @context: the context
- * @object: pattern wrapper
- *
- * Returns: the pattern attaches to the wrapper.
+ * @cx: the context
+ * @pattern_wrapper: pattern wrapper
  *
+ * Returns: the pattern attached to the wrapper.
  */
-cairo_pattern_t *
-gjs_cairo_pattern_get_pattern(JSContext *context,
-                              JSObject  *object)
-{
-    GjsCairoPattern *priv;
+cairo_pattern_t* gjs_cairo_pattern_get_pattern(
+    JSContext* cx, JS::HandleObject pattern_wrapper) {
+    g_return_val_if_fail(cx, nullptr);
+    g_return_val_if_fail(pattern_wrapper, nullptr);
 
-    g_return_val_if_fail(context, nullptr);
-    g_return_val_if_fail(object, nullptr);
+    JS::RootedObject proto(cx, gjs_cairo_pattern_get_proto(cx));
+
+    bool is_pattern_subclass = false;
+    if (!gjs_object_in_prototype_chain(cx, proto, pattern_wrapper,
+                                       &is_pattern_subclass))
+        return nullptr;
+    if (!is_pattern_subclass) {
+        gjs_throw(cx, "Expected Cairo.Pattern but got %s",
+                  JS_GetClass(pattern_wrapper)->name);
+        return nullptr;
+    }
 
-    priv = (GjsCairoPattern*) JS_GetPrivate(object);
+    auto* priv = static_cast<GjsCairoPattern*>(JS_GetPrivate(pattern_wrapper));
     if (!priv)
         return nullptr;
 
+    g_assert(priv->pattern);
     return priv->pattern;
 }
-
diff --git a/modules/cairo-private.h b/modules/cairo-private.h
index 7634c76f..29083901 100644
--- a/modules/cairo-private.h
+++ b/modules/cairo-private.h
@@ -65,9 +65,9 @@ bool gjs_cairo_path_define_proto(JSContext              *cx,
 GJS_JSAPI_RETURN_CONVENTION
 JSObject *       gjs_cairo_path_from_path               (JSContext       *context,
                                                          cairo_path_t    *path);
-GJS_USE
-cairo_path_t *   gjs_cairo_path_get_path                (JSContext       *context,
-                                                         JSObject        *path_wrapper);
+GJS_JSAPI_RETURN_CONVENTION
+cairo_path_t* gjs_cairo_path_get_path(JSContext* cx,
+                                      JS::HandleObject path_wrapper);
 
 /* surface */
 GJS_USE
@@ -86,9 +86,9 @@ void             gjs_cairo_surface_finalize_surface     (JSFreeOp        *fop,
 GJS_JSAPI_RETURN_CONVENTION
 JSObject *       gjs_cairo_surface_from_surface         (JSContext       *context,
                                                          cairo_surface_t *surface);
-GJS_USE
-cairo_surface_t* gjs_cairo_surface_get_surface          (JSContext       *context,
-                                                         JSObject        *object);
+GJS_JSAPI_RETURN_CONVENTION
+cairo_surface_t* gjs_cairo_surface_get_surface(
+    JSContext* cx, JS::HandleObject surface_wrapper);
 
 /* image surface */
 GJS_JSAPI_RETURN_CONVENTION
@@ -153,9 +153,9 @@ void             gjs_cairo_pattern_finalize_pattern     (JSFreeOp        *fop,
 GJS_JSAPI_RETURN_CONVENTION
 JSObject*        gjs_cairo_pattern_from_pattern         (JSContext       *context,
                                                          cairo_pattern_t *pattern);
-GJS_USE
-cairo_pattern_t* gjs_cairo_pattern_get_pattern          (JSContext       *context,
-                                                         JSObject        *object);
+GJS_JSAPI_RETURN_CONVENTION
+cairo_pattern_t* gjs_cairo_pattern_get_pattern(
+    JSContext* cx, JS::HandleObject pattern_wrapper);
 
 /* gradient */
 GJS_USE
diff --git a/modules/cairo-surface-pattern.cpp b/modules/cairo-surface-pattern.cpp
index 613b9049..d8dc7e2f 100644
--- a/modules/cairo-surface-pattern.cpp
+++ b/modules/cairo-surface-pattern.cpp
@@ -40,7 +40,6 @@ GJS_DEFINE_PROTO_WITH_PARENT("SurfacePattern", cairo_surface_pattern,
 GJS_NATIVE_CONSTRUCTOR_DECLARE(cairo_surface_pattern)
 {
     GJS_NATIVE_CONSTRUCTOR_VARIABLES(cairo_surface_pattern)
-    cairo_surface_t *surface;
     cairo_pattern_t *pattern;
 
     GJS_NATIVE_CONSTRUCTOR_PRELUDE(cairo_surface_pattern);
@@ -50,11 +49,10 @@ GJS_NATIVE_CONSTRUCTOR_DECLARE(cairo_surface_pattern)
                              "surface", &surface_wrapper))
         return false;
 
-    surface = gjs_cairo_surface_get_surface(context, surface_wrapper);
-    if (!surface) {
-        gjs_throw(context, "first argument to SurfacePattern() should be a surface");
+    cairo_surface_t* surface =
+        gjs_cairo_surface_get_surface(context, surface_wrapper);
+    if (!surface)
         return false;
-    }
 
     pattern = cairo_pattern_create_for_surface(surface);
 
@@ -89,13 +87,15 @@ setExtend_func(JSContext *context,
 {
     GJS_GET_THIS(context, argc, vp, argv, obj);
     cairo_extend_t extend;
-    cairo_pattern_t *pattern;
 
     if (!gjs_parse_call_args(context, "setExtend", argv, "i",
                              "extend", &extend))
         return false;
 
-    pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    cairo_pattern_t* pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    if (!pattern)
+        return false;
+
     cairo_pattern_set_extend(pattern, extend);
 
     if (!gjs_cairo_check_status(context, cairo_pattern_status(pattern), "pattern"))
@@ -113,14 +113,16 @@ getExtend_func(JSContext *context,
 {
     GJS_GET_THIS(context, argc, vp, rec, obj);
     cairo_extend_t extend;
-    cairo_pattern_t *pattern;
 
     if (argc > 0) {
         gjs_throw(context, "SurfacePattern.getExtend() requires no arguments");
         return false;
     }
 
-    pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    cairo_pattern_t* pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    if (!pattern)
+        return false;
+
     extend = cairo_pattern_get_extend(pattern);
 
     if (!gjs_cairo_check_status(context, cairo_pattern_status(pattern), "pattern"))
@@ -139,13 +141,15 @@ setFilter_func(JSContext *context,
 {
     GJS_GET_THIS(context, argc, vp, argv, obj);
     cairo_filter_t filter;
-    cairo_pattern_t *pattern;
 
     if (!gjs_parse_call_args(context, "setFilter", argv, "i",
                              "filter", &filter))
         return false;
 
-    pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    cairo_pattern_t* pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    if (!pattern)
+        return false;
+
     cairo_pattern_set_filter(pattern, filter);
 
     if (!gjs_cairo_check_status(context, cairo_pattern_status(pattern), "pattern"))
@@ -163,14 +167,16 @@ getFilter_func(JSContext *context,
 {
     GJS_GET_THIS(context, argc, vp, rec, obj);
     cairo_filter_t filter;
-    cairo_pattern_t *pattern;
 
     if (argc > 0) {
         gjs_throw(context, "SurfacePattern.getFilter() requires no arguments");
         return false;
     }
 
-    pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    cairo_pattern_t* pattern = gjs_cairo_pattern_get_pattern(context, obj);
+    if (!pattern)
+        return false;
+
     filter = cairo_pattern_get_filter(pattern);
 
     if (!gjs_cairo_check_status(context, cairo_pattern_status(pattern), "pattern"))
diff --git a/modules/cairo-surface.cpp b/modules/cairo-surface.cpp
index a2bb9514..5b98c1b7 100644
--- a/modules/cairo-surface.cpp
+++ b/modules/cairo-surface.cpp
@@ -70,13 +70,12 @@ writeToPNG_func(JSContext *context,
 {
     GJS_GET_THIS(context, argc, vp, argv, obj);
     GjsAutoChar filename;
-    cairo_surface_t *surface;
 
     if (!gjs_parse_call_args(context, "writeToPNG", argv, "F",
                              "filename", &filename))
         return false;
 
-    surface = gjs_cairo_surface_get_surface(context, obj);
+    cairo_surface_t* surface = gjs_cairo_surface_get_surface(context, obj);
     if (!surface)
         return false;
 
@@ -95,7 +94,6 @@ getType_func(JSContext *context,
              JS::Value *vp)
 {
     GJS_GET_THIS(context, argc, vp, rec, obj);
-    cairo_surface_t *surface;
     cairo_surface_type_t type;
 
     if (argc > 1) {
@@ -103,7 +101,10 @@ getType_func(JSContext *context,
         return false;
     }
 
-    surface = gjs_cairo_surface_get_surface(context, obj);
+    cairo_surface_t* surface = gjs_cairo_surface_get_surface(context, obj);
+    if (!surface)
+        return false;
+
     type = cairo_surface_get_type(surface);
     if (!gjs_cairo_check_status(context, cairo_surface_status(surface),
                                 "surface"))
@@ -226,24 +227,33 @@ gjs_cairo_surface_from_surface(JSContext       *context,
 
 /**
  * gjs_cairo_surface_get_surface:
- * @context: the context
- * @object: surface wrapper
- *
- * Returns: the surface attaches to the wrapper.
+ * @cx: the context
+ * @surface_wrapper: surface wrapper
  *
+ * Returns: the surface attached to the wrapper.
  */
-cairo_surface_t *
-gjs_cairo_surface_get_surface(JSContext *context,
-                              JSObject *object)
-{
-    GjsCairoSurface *priv;
+cairo_surface_t* gjs_cairo_surface_get_surface(
+    JSContext* cx, JS::HandleObject surface_wrapper) {
+    g_return_val_if_fail(cx, nullptr);
+    g_return_val_if_fail(surface_wrapper, nullptr);
 
-    g_return_val_if_fail(context, nullptr);
-    g_return_val_if_fail(object, nullptr);
+    JS::RootedObject proto(cx, gjs_cairo_surface_get_proto(cx));
+
+    bool is_surface_subclass = false;
+    if (!gjs_object_in_prototype_chain(cx, proto, surface_wrapper,
+                                       &is_surface_subclass))
+        return nullptr;
+    if (!is_surface_subclass) {
+        gjs_throw(cx, "Expected Cairo.Surface but got %s",
+                  JS_GetClass(surface_wrapper)->name);
+        return nullptr;
+    }
 
-    priv = (GjsCairoSurface*) JS_GetPrivate(object);
+    auto* priv = static_cast<GjsCairoSurface*>(JS_GetPrivate(surface_wrapper));
     if (!priv)
         return nullptr;
+
+    g_assert(priv->surface);
     return priv->surface;
 }
 
@@ -269,11 +279,16 @@ surface_to_g_argument(JSContext      *context,
         return true;
     }
 
-    JSObject *obj;
-    cairo_surface_t *s;
+    if (!value.isObject()) {
+        GjsAutoChar display_name =
+            gjs_argument_display_name(arg_name, argument_type);
+        gjs_throw(context, "%s is not a Cairo.Surface", display_name.get());
+        return false;
+    }
 
-    obj = &value.toObject();
-    s = gjs_cairo_surface_get_surface(context, obj);
+    JS::RootedObject surface_wrapper(context, &value.toObject());
+    cairo_surface_t* s =
+        gjs_cairo_surface_get_surface(context, surface_wrapper);
     if (!s)
         return false;
     if (transfer == GI_TRANSFER_EVERYTHING)
[Date Prev][Date Next] [Thread Prev][Thread Next] [Thread Index] [Date Index] [Author Index]