[Webkit-unassigned] [Bug 183448] [GTK][WPE] Add API to convert between DOM and JSCValue

[bugzilla-daemon at webkit.org]

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

--- Comment #4 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Michael Catanzaro from comment #3)
> Comment on attachment 335931 [details]
> WIP patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=335931&action=review
> 
> > Source/WebKit/WebProcess/InjectedBundle/API/glib/WebKitFrame.cpp:27
> >  #include <jsc/JSCContextPrivate.h>
> 
> Alphabetize jsc below WebCore and above wtf: all the uppercase headers come
> first.
> 
> > Source/WebKit/WebProcess/InjectedBundle/API/glib/WebKitFrame.cpp:154
> > +JSCValue* webkit_frame_get_js_value_for_dom_object(WebKitFrame* frame, WebKitDOMObject* domObject)
> > +{
> > +    return webkit_frame_get_js_value_for_dom_object_in_script_world(frame, domObject, webkit_script_world_get_default());
> > +}
> 
> Would it make sense to force users to explicitly pass a WebKitScriptWorld?
> Could prevent mistakes?

99% of the cases you want to use the default, so this is a convenience function to get the value using the default world and will be documented as such. It's equivalent to webkit_frame_get_js_context and get_js_context_for_scripot_world.

> > Source/WebKit/WebProcess/InjectedBundle/API/glib/WebKitFrame.cpp:168
> > +        JSC::JSLockHolder lock(exec);
> 
> So why is a lock needed here, but not in webkit_dom_note_for_js_value()...
> because the JSC::ExecState is used?

We are creating a new value in the context here to wrap a node, while when getting the wrapped node, we are just querying an existing value. toWrapped is just a jsDynamicCast in the end.

> > Tools/TestWebKitAPI/Tests/WebKitGLib/WebProcessTest.cpp:47
> > +    g_print("Leaked objects in WebProcess:");
> > +    for (const auto object : s_watchedObjects)
> > +        g_print(" %s(%p)", g_type_name_from_instance(reinterpret_cast<GTypeInstance*>(object)), object);
> > +    g_print("\n");
> 
> This should use g_fprintf and stderr, so that it causes the test to fail.

No, we don't want the test to fail on the first error message, we want to see the whole message, the test will fail anyway because we assert right after this.

> > Tools/TestWebKitAPI/Tests/WebKitGtk/EditorTest.cpp:34
> > +    bool testSelectionchanged(WebKitWebPage* page)
> 
> testSelectionChanged

Right.

-- 
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/20180317/f453a337/attachment-0001.html>