[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 #24 from Yusuke Suzuki <utatane.tea at gmail.com> ---
Comment on attachment 335929
  --> https://bugs.webkit.org/attachment.cgi?id=335929
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?

> 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.

> 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.

> 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.

-- 
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/20180318/7c1f1d00/attachment.html>