[Webkit-unassigned] [Bug 188883] [GTK][WPE] Add API to inject/register user content in isolated worlds

[bugzilla-daemon at webkit.org]

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

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #347918|review?                     |review+
              Flags|                            |

--- Comment #4 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 347918
  --> https://bugs.webkit.org/attachment.cgi?id=347918
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347918&action=review

> Source/WebKit/UIProcess/API/glib/WebKitUserContent.cpp:44
> +    return *map.get().ensure(worldName, [&] { return API::UserContentWorld::worldWithName(String::fromUTF8(worldName)); }).iterator->value;

Nit: why [&]? It looks like the only thing you're capturing here is worldName. I would capture it explicitly.

> Source/WebKit/UIProcess/API/glib/WebKitUserContent.cpp:179
> + * webkit_user_style_sheet_new_for_world:

Hm... do we need it?

We definitely need webkit_user_script_new_for_world().

And it probably makes sense to add webkit_user_style_sheet_new_for_world() at the same time.

But would there be any real use case for it? The use case for running JS in isolated worlds is to prevent the JS from being accessible to the web page. Normally with a style you do want it to be accessible.

> Source/WebKit/UIProcess/API/glib/WebKitUserContentManager.cpp:304
> + * #WebKitUserContentManager::script-message-received signal,

Comma splice; use a semicolon

> Source/WebKit/UIProcess/API/glib/WebKitUserContentManager.cpp:308
> + * See also webkit_user_content_manager_register_script_message_handler()

.

> Source/WebKit/WebProcess/UserContent/WebUserContentController.cpp:112
> -        worldMap().ensure(world.first, [&] { return std::make_pair(InjectedBundleScriptWorld::create(world.second), 1); });
> +        worldMap().ensure(world.first, [&] {
> +#if PLATFORM(GTK) || PLATFORM(WPE)
> +            // The GLib API doesn't allow to create script worlds from the UI process, and the
> +            // world name is used as the identifier. Use the existing world if found.
> +            if (auto* existingWorld = InjectedBundleScriptWorld::find(world.second))
> +                return std::make_pair(Ref<InjectedBundleScriptWorld>(*existingWorld), 1);
> +#endif
> +            return std::make_pair(InjectedBundleScriptWorld::create(world.second), 1);
> +        });

I don't understand your explanation; can you try explaining it again? It's expected for WebUserContentController::addUserContentWorlds to use existing worlds, rather than add new worlds, only on WPE/GTK?

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitUserContentManager.cpp:365
> +    // Check that the "window.webkit.messageHandlers" namespace doesn't exist in isolated worlds.

Is this comment correct? If it were, I would expect the JS to return 'n', correct? But instead it fails to execute, because you can't execute JS in a world that doesn't exist yet?

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitUserContentManager.cpp:384
> +    // Post message in nain world should fail.

main

If you want to test even more, you could proceed to hook up the msg handler for the main world, and then verify that the proper handler gets called when you post the message from WebExtensionTestScriptWorld and the main world.

-- 
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/20180823/17482bb3/attachment-0001.html>