Bug 103377 - [GTK] Add WebKit2 API for isolated worlds
Summary: [GTK] Add WebKit2 API for isolated worlds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on: 119743 120252 120268
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-27 01:21 PST by Zan Dobersek
Modified: 2013-08-26 08:08 PDT (History)
6 users (show)

See Also:


Attachments
Patch (29.61 KB, patch)
2013-08-22 06:09 PDT, Carlos Garcia Campos
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2012-11-27 01:21:09 PST
As requested on webkit-gtk mailing list.
http://lists.webkit.org/pipermail/webkit-gtk/2012-November/001293.html

There was at one time a desire to add this API to WebKit1, covered by bug #31508.
Comment 1 Carlos Garcia Campos 2013-08-22 06:09:13 PDT
Created attachment 209352 [details]
Patch
Comment 2 Carlos Garcia Campos 2013-08-22 08:02:01 PDT
Patch doesn't apply because it depends on patch applied to bug #119743
Comment 3 Anders Carlsson 2013-08-23 05:37:18 PDT
Comment on attachment 209352 [details]
Patch

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

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitScriptWorld.cpp:41
> +    DEFINE_STATIC_LOCAL(ScriptWorldMap, map, ());

This can be NeverDestroyed<ScriptWorldMap> map if you include wtf/NeverDestroyed.h
Comment 4 Carlos Garcia Campos 2013-08-23 05:46:13 PDT
(In reply to comment #3)
> (From update of attachment 209352 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=209352&action=review

Thanks for the review. 

> > Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitScriptWorld.cpp:41
> > +    DEFINE_STATIC_LOCAL(ScriptWorldMap, map, ());
> 
> This can be NeverDestroyed<ScriptWorldMap> map if you include wtf/NeverDestroyed.h

Sure, I didn't know NeverDestroyed.
Comment 5 Gustavo Noronha (kov) 2013-08-23 06:00:35 PDT
Comment on attachment 209352 [details]
Patch

The API looks sensible to me, +1.
Comment 6 Martin Robinson 2013-08-23 07:40:20 PDT
Comment on attachment 209352 [details]
Patch

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

Looks good to me as well.

> Source/WebKit2/UIProcess/API/gtk/tests/WebExtensionTest.cpp:154
> +        const char *script;

Nit: asterisk location

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitScriptWorld.cpp:88
> +    return scriptWorlds().get(scriptWorld);

This returns null if the world isn't in the map?
Comment 7 Carlos Garcia Campos 2013-08-24 04:08:26 PDT
(In reply to comment #6)
> (From update of attachment 209352 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=209352&action=review
> 
> Looks good to me as well.

Thanks!

> > Source/WebKit2/UIProcess/API/gtk/tests/WebExtensionTest.cpp:154
> > +        const char *script;
> 
> Nit: asterisk location

Ooops

> > Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitScriptWorld.cpp:88
> > +    return scriptWorlds().get(scriptWorld);
> 
> This returns null if the world isn't in the map?

Yes.
Comment 8 Carlos Garcia Campos 2013-08-24 05:01:06 PDT
Committed r154545: <http://trac.webkit.org/changeset/154545>
Comment 9 WebKit Commit Bot 2013-08-24 12:58:31 PDT
Re-opened since this is blocked by bug 120252
Comment 10 Carlos Garcia Campos 2013-08-26 08:08:26 PDT
Committed r154603: <http://trac.webkit.org/changeset/154603>