[Webkit-unassigned] [Bug 125990] [GTK] Allow passing extra initialization data to web extensions

[bugzilla-daemon at webkit.org]

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


Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #220378|review?                     |review-
               Flag|                            |




--- Comment #14 from Carlos Garcia Campos <cgarcia at igalia.com>  2014-01-05 06:05:14 PST ---
(From update of attachment 220378)
View in context: https://bugs.webkit.org/attachment.cgi?id=220378&action=review

Thanks for the new patch. I think we already agreed on having a signal WebKitWebContext::initialize-web-extensions and a method to set the init user data webkit_web_context_set_web_extensions_initialization_user_data(). Then we document that the best place to call both set_web_extensions_directory and set_web_extensions_initialization_user_data is in the initialize-web-extensions callback. If we want to avoid having to keep the user data around in the context all the time, we could clear the data after the signal is emitted, and document that set_web_extensions_initialization_user_data() should always be called when initialize-web-extensions is emitted. This way we also avoid the accumulator and the signal having to return a GVariant. Also in the web extensions api, we agreed on adding a new initialization function instead of webkit_web_extension_get_initialization_user_data(), something like webkit_web_extension_initialize_with_user_data() that receives the GVariant as a parameter. If this symbol is not present in the extension, we just fallback to webkit_web_extension_initialize(). See comment #11.

> Source/WebKit2/UIProcess/API/gtk/WebKitInjectedBundleClient.cpp:123
> +    GRefPtr<GVariant> extraData = webkitWebContextWebExtensionsLoaded(WEBKIT_WEB_CONTEXT(clientInfo));

WebExtensionsLoaded is not accurate, this is called even before the web process has been launched. Use the name of the signal that is going to be emitted:

webkitWebContextInitializeWebExtensions()

We could make that method build the final GVariant, so that we don't need more private api here. It emits the signal and then builds the GVariant with the extensions dir and user_data.

GRefPtr<GVariant> data = webkitWebContextInitializeWebExtensions(WEBKIT_WEB_CONTEXT(clientInfo));
GOwnPtr<gchar> dataString(g_variant_print(data.get(), TRUE));
return static_cast<WKTypeRef>(WKStringCreateWithUTF8CString(dataString.get()));

The function would return a float reference, so you don't need to use adoptGRef in this case.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebExtensionsInitialization.cpp:45
> +    GRefPtr<GDBusProxy> proxy = adoptGRef(bus->createProxy("org.webkit.gtk.WebExtensionTest",
> +        "/org/webkit/gtk/WebExtensionTest" , "org.webkit.gtk.WebExtensionTest", test->m_mainLoop));
> +    GRefPtr<GVariant> result = adoptGRef(g_dbus_proxy_call_sync(
> +        proxy.get(),
> +        "GetExtraDataAsString",
> +        nullptr,
> +        G_DBUS_CALL_FLAGS_NONE,
> +        -1, 0, 0));
> +    g_assert(result);

I think it would be a lot easier if you simply check the user data in the web extension. Here you simply pass "'extra data'" and in the web extension you assert if the user data is not "'extra data'"

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebExtensionsInitialization.cpp:65
> +    webkit_web_context_set_web_extensions_directory(webkit_web_context_get_default(), WEBKIT_TEST_WEB_EXTENSIONS_DIR);
> +    initializeWebExtensionsSignalHandlerId = g_signal_connect(webkit_web_context_get_default(),
> +        "initialize-web-extensions", G_CALLBACK(initializeWebExtensionsCallback), nullptr);

Create a class instead of adding this in beforeAll. Call webkit_web_context_set_web_extensions_directory in the signal hanlder too. You don't need to keep the handler id, you can use g_signal_handlers_disconnect_matched in the class destructor.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebExtensionsInitialization.cpp:71
> +    WebViewTest::add("WebKitWebView", "web-extensions-initialization-user-data", testWebExtensionsInitializationUserData);

Would it be possible to reuse the existing web extensions test file? 

"WebKitWebView" -> "WebKitWebContext"

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.