Changeset 242349 in webkit

Timestamp:

Mar 4, 2019, 2:34:04 AM (7 years ago)

Author:

Carlos Garcia Campos

Message:

[GLib] Returning G_TYPE_OBJECT from a constructor does not work
​https://bugs.webkit.org/show_bug.cgi?id=195206

Reviewed by Žan Doberšek.

Source/JavaScriptCore:

We are freeing the newly created object before returning from the constructor.

• API/glib/JSCCallbackFunction.cpp:

(JSC::JSCCallbackFunction::construct):

Tools:

Add a new test case.

• TestWebKitAPI/Tests/JavaScriptCore/glib/TestJSC.cpp:

(testJSCClass):

Location:

trunk

Files:

• Source/JavaScriptCore/API/glib/JSCCallbackFunction.cpp (modified) (2 diffs)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/JavaScriptCore/glib/TestJSC.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/API/glib/JSCCallbackFunction.cpp

@@ -206 +206 @@
     case G_TYPE_POINTER:
     case G_TYPE_OBJECT:
-        if (auto* ptr = returnValue.data[0].v_pointer) {
-            auto* retval = jscClassGetOrCreateJSWrapper(m_class.get(), ptr);
-            g_value_unset(&returnValue);
-            return toRef(retval);
-        }
+        if (auto* ptr = returnValue.data[0].v_pointer)
+            return toRef(jscClassGetOrCreateJSWrapper(m_class.get(), ptr));
         *exception = toRef(JSC::createTypeError(toJS(jsContext), "constructor returned null"_s));
         break;
@@ -217 +214 @@
         break;
     }
+    g_value_unset(&returnValue);
     return nullptr;
 }

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-03-04  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GLib] Returning G_TYPE_OBJECT from a constructor does not work
+        https://bugs.webkit.org/show_bug.cgi?id=195206
+
+        Reviewed by Žan Doberšek.
+
+        We are freeing the newly created object before returning from the constructor.
+
+        * API/glib/JSCCallbackFunction.cpp:
+        (JSC::JSCCallbackFunction::construct):
+
 2019-03-02  Darin Adler  <darin@apple.com>
 

trunk/Tools/ChangeLog

@@ +1 @@
+2019-03-04  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GLib] Returning G_TYPE_OBJECT from a constructor does not work
+        https://bugs.webkit.org/show_bug.cgi?id=195206
+
+        Reviewed by Žan Doberšek.
+
+        Add a new test case.
+
+        * TestWebKitAPI/Tests/JavaScriptCore/glib/TestJSC.cpp:
+        (testJSCClass):
+
 2019-03-04  Charlie Turner  <cturner@igalia.com>
 

trunk/Tools/TestWebKitAPI/Tests/JavaScriptCore/glib/TestJSC.cpp

@@ -1590 +1590 @@
 };
 
+static GFile* createGFile(const char* path)
+{
+    GFile* file = g_file_new_for_path(path);
+    auto* checker = static_cast<LeakChecker*>(g_object_get_data(G_OBJECT(jsc_context_get_current()), "leak-checker"));
+    checker->watch(file);
+    return file;
+}
+
 static void testJSCClass()
 {
@@ -2248 +2256 @@
         g_assert_did_throw(exceptionHandler, didThrow);
         g_assert_true(jsc_context_get_exception(context.get()) == previousException.get());
+    }
+
+    {
+        LeakChecker checker;
+        GRefPtr<JSCContext> context = adoptGRef(jsc_context_new());
+        checker.watch(context.get());
+        g_object_set_data(G_OBJECT(context.get()), "leak-checker", &checker);
+        ExceptionHandler exceptionHandler(context.get());
+
+        JSCClass* jscClass = jsc_context_register_class(context.get(), "GFile", nullptr, nullptr, reinterpret_cast<GDestroyNotify>(g_object_unref));
+        checker.watch(jscClass);
+
+        GRefPtr<JSCValue> constructor = adoptGRef(jsc_class_add_constructor(jscClass, nullptr, G_CALLBACK(createGFile), nullptr, nullptr, G_TYPE_OBJECT, 1, G_TYPE_STRING));
+        checker.watch(constructor.get());
+        g_assert_true(jsc_value_is_constructor(constructor.get()));
+        jsc_class_add_method(jscClass, "getPath", G_CALLBACK(g_file_get_path), nullptr, nullptr, G_TYPE_STRING, 0, G_TYPE_NONE);
+
+        jsc_context_set_value(context.get(), jsc_class_get_name(jscClass), constructor.get());
+
+        GRefPtr<JSCValue> file = adoptGRef(jsc_context_evaluate(context.get(), "f = new GFile('.');", -1));
+        checker.watch(file.get());
+        g_assert_true(jsc_value_is_object(file.get()));
+        g_assert_true(jsc_value_object_is_instance_of(file.get(), jsc_class_get_name(jscClass)));
+        GRefPtr<JSCValue> result = adoptGRef(jsc_context_evaluate(context.get(), "f instanceof GFile;", -1));
+        checker.watch(result.get());
+        g_assert_true(jsc_value_is_boolean(result.get()));
+        g_assert_true(jsc_value_to_boolean(result.get()));
+
+        g_assert_true(jsc_value_object_has_property(file.get(), "getPath"));
+        GRefPtr<JSCValue> value = adoptGRef(jsc_value_object_invoke_method(file.get(), "getPath", G_TYPE_NONE));
+        checker.watch(value.get());
+        g_assert_true(jsc_value_is_string(value.get()));
+        GUniquePtr<char> resultString(jsc_value_to_string(value.get()));
+        GUniquePtr<char> currentDirectory(g_get_current_dir());
+        g_assert_cmpstr(resultString.get(), ==, currentDirectory.get());
+
+        GRefPtr<JSCValue> value2 = adoptGRef(jsc_context_evaluate(context.get(), "f.getPath('.');", -1));
+        checker.watch(value2.get());
+        g_assert_true(jsc_value_is_string(value2.get()));
+        resultString.reset(jsc_value_to_string(value2.get()));
+        g_assert_cmpstr(resultString.get(), ==, currentDirectory.get());
     }
 }