Bug 103377 – [GTK] Add WebKit2 API for isolated worlds

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Zan Dobersek]

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.

[Carlos Garcia Campos]

Created attachment 209352 [details]
Patch

[Carlos Garcia Campos]

Patch doesn't apply because it depends on patch applied to bug #119743

[Anders Carlsson]

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

[Carlos Garcia Campos]

(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.

[Gustavo Noronha (kov)]

Comment on attachment 209352 [details]
Patch

The API looks sensible to me, +1.

[Martin Robinson]

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?

[Carlos Garcia Campos]

(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.

[Carlos Garcia Campos]

Committed r154545: <http://trac.webkit.org/changeset/154545>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 120252

[Carlos Garcia Campos]

Committed r154603: <http://trac.webkit.org/changeset/154603>