Bug 138871

Summary:

[GTK] Provide convenience API in DOM bindings to post messages to user message handlers

Product:

WebKit

Reporter:

Adrian Perez <aperez>

Component:

WebKitGTK

Assignee:

Adrian Perez <aperez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cgarcia, commit-queue, darin, gustavo, pnormand, svillar

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

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	Details Formatted Diff Diff
Patch (8.60 KB, patch) 2014-11-19 11:40 PST, Adrian Perez	no flags	Details Formatted Diff Diff
Patch (8.54 KB, patch) 2014-11-20 04:00 PST, Adrian Perez	no flags	Details Formatted Diff Diff
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.