RESOLVED FIXED 138871
[GTK] Provide convenience API in DOM bindings to post messages to user message handlers 
https://bugs.webkit.org/show_bug.cgi?id=138871
Summary [GTK] Provide convenience API in DOM bindings to post messages to user messag...
Adrian Perez
Reported 2014-11-19 07:47:19 PST
As per this comment: https://bugs.webkit.org/show_bug.cgi?id=138411#c1 ”The "raw" API is not very convenient to use, we should add custom API to make it easier to use.”
Attachments
Patch (7.89 KB, patch)
2014-11-19 08:11 PST, Adrian Perez 		no flags
DetailsFormatted DiffDiff
Patch (8.60 KB, patch)
2014-11-19 11:40 PST, Adrian Perez 		no flags
DetailsFormatted DiffDiff
Patch (8.54 KB, patch)
2014-11-20 04:00 PST, Adrian Perez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Adrian Perez
Comment 1 2014-11-19 08:11:07 PST
Created attachment 241860 [details] Patch
Carlos Garcia Campos
Comment 2 2014-11-19 08:43:18 PST
Comment on attachment 241860 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241860&action=review Thanks! I have just some minor comments. > Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp:68 > +gboolean webkit_dom_dom_window_post_user_message(WebKitDOMDOMWindow* window, const gchar* handlerName, const gchar* message) This is equivalent to window.webkit.messageHandlers.<handler>.postMessage, so I think we could convert that into webkit_dom_dom_window_webkit_message_handlers_post_message(WebKitDOMDOMWindow* window, const gchar* handlerName, const gchar* message); > Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp:70 > +#if ENABLE(USER_MESSAGE_HANDLERS) We are building with USER_MESSAGE_HANDLERS unconditionally, so we can just remove the #if. > Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp:72 > + g_return_val_if_fail(WEBKIT_DOM_IS_DOM_WINDOW(window), FALSE); > + g_return_val_if_fail(message, FALSE); You should check handlerName is a valid pointer too > Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp:78 > + auto handler = domWindow->webkitNamespace()->messageHandlers()->handler(String::fromUTF8(handlerName), WebCore::mainThreadNormalWorld()); DOMWindow::webkitNamespace() can return nullptr, you should check it. > Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp:79 > + if (handler == nullptr) Don't compare to nullptr, use !handler > Source/WebCore/bindings/gobject/WebKitDOMCustom.h:51 > + */ Since: 2.8 > Source/WebCore/bindings/gobject/WebKitDOMCustom.symbols:3 > +gboolean webkit_dom_dom_window_post_user_message(WebKitDOMDOMWindow*, const gchar*, const gchar*) You should also add the symbols to the webkitdom.symbols file, right after webkit_dom_dom_window_get_type > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitUserContentManager.cpp:371 > + g_assert(test->registerHandler("dom-convenience")); > + > + test->loadHtml("<html></html>", nullptr); > + WebKitJavascriptResult* javascriptResult = test->waitUntilMessageReceived("dom-convenience"); > + g_assert(javascriptResult); > + GUniquePtr<char> valueString(WebViewTest::javascriptResultToCString(javascriptResult)); > + g_assert_cmpstr(valueString.get(), ==, "DocumentLoaded"); > + > + test->unregisterHandler("dom-convenience"); Maybe we can merge both tests, connect to both handlers and check that both messages are received and are the same.
Adrian Perez
Comment 3 2014-11-19 11:40:27 PST
Created attachment 241871 [details] Patch
Adrian Perez
Comment 4 2014-11-19 11:41:06 PST
Nits fixed, the patch should be good to go now :-)
Carlos Garcia Campos
Comment 5 2014-11-20 00:26:03 PST
Comment on attachment 241871 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241871&action=review > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitUserContentManager.cpp:355 > + g_assert(javascriptResult); Why duplicating the assert here? > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitUserContentManager.cpp:366 > + g_assert(javascriptResult); > + g_assert(javascriptResult); Ditto.
Adrian Perez
Comment 6 2014-11-20 04:00:29 PST
Created attachment 241939 [details] Patch
WebKit Commit Bot
Comment 7 2014-11-20 04:46:05 PST
Comment on attachment 241939 [details] Patch Clearing flags on attachment: 241939 Committed r176393: <http://trac.webkit.org/changeset/176393>
WebKit Commit Bot
Comment 8 2014-11-20 04:46:09 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.