[Webkit-unassigned] [Bug 164061] [GTK][WPE] Initial implementation of JavaScriptCore glib bindings

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=164061

--- Comment #25 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Yusuke Suzuki from comment #24)
> Comment on attachment 335929 [details]
> WIP patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=335929&action=review
> 
> Early review. I have one question about the design of this patch: why not
> just accepting GObject for a wrapped object?

We don't want to limit this to GObjects, we want to allow using a boxed type for example, or simply any C struct. On top of this patch, I plan to add convenience API specific for GType system, making it easier to expose GObjects directly, automatically handling things like the prototype hierarchy.

> > Source/JavaScriptCore/API/glib/JSAPIWrapperObjectGLib.cpp:54
> > +    JSC::Heap::heap(wrapperObject)->releaseSoon(std::unique_ptr<JSC::JSCGLibWrapperObject>(static_cast<JSC::JSCGLibWrapperObject*>(wrapperObject->wrappedObject())));
> 
> If a wrapped object is GObject, we can just do adoptGRef here, which is
> well-aligned to the objective C implementation.

In Objective C everything is an Object, that's no the case of GLib world.

> > Source/JavaScriptCore/API/glib/JSAPIWrapperObjectGLib.cpp:94
> > +    m_wrappedObject = wrappedObject;
> 
> And do g_object_ref here, which is aligned to the Objective C implementation.
> 
> > Source/JavaScriptCore/API/glib/JSCContext.h:124
> > +                                      GDestroyNotify      destroy_function);
> 
> What is the purpose of destroy_function? Why not just calling g_object_unref
> when the wrapper is dead?
> I think the current objective-c implementation does this.

In case of using a boxed type or any other custom struct here we would pass my_boxed_type_unref or my_custom_struct_free. See the example in the unit tests.

> > Source/JavaScriptCore/API/glib/JSCGLibWrapperObject.h:47
> > +class JSCGLibWrapperObject {
> > +    WTF_MAKE_FAST_ALLOCATED;
> > +public:
> > +    JSCGLibWrapperObject(gpointer object, GDestroyNotify destroyFunction)
> > +        : m_object(object)
> > +        , m_destroyFunction(destroyFunction)
> > +    {
> > +    }
> > +
> > +    ~JSCGLibWrapperObject()
> > +    {
> > +        if (m_destroyFunction)
> > +            m_destroyFunction(m_object);
> > +    }
> > +
> > +    gpointer object() const { return m_object; }
> > +
> > +private:
> > +    gpointer m_object { nullptr };
> > +    GDestroyNotify m_destroyFunction { nullptr };
> > +};
> 
> I think just wrapping gobject instead of having this class is cleaner and
> well-aligned to objective C implementation.

I agree it would be easier and cleaner, but would limit the API.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180319/0a81be41/attachment-0001.html>