[Webkit-unassigned] [Bug 164061] [GTK][WPE] Initial implementation of JavaScriptCore glib bindings
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Mar 18 10:24:29 PDT 2018
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>
More information about the webkit-unassigned
mailing list