Bug 119743 – [GTK] Expose WebKitFrame in WebKit2GTK+ web extensions API

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Carlos Garcia Campos]

This is required for the javascript injection and isolated worlds API. See https://lists.webkit.org/pipermail/webkit-gtk/2013-August/001535.html.

[Carlos Garcia Campos]

Created attachment 208624 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[kov's GTK+ EWS bot]

Comment on attachment 208624 [details]
Patch

Attachment 208624 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1452440

[Carlos Garcia Campos]

Created attachment 208634 [details]
Updated patch

I forgot to git add some new files

[Carlos Garcia Campos]

Created attachment 208638 [details]
Another update

Sorry, I forgot to git add two more files.

[Sergio Villar Senin]

Comment on attachment 208638 [details]
Another update

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

Changes (informally) look awesome to me

> Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk-sections.txt:1064
> +webkit_frame_get_javascript_global_context

nit: should we have these properly sorted?

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp:305
> +        kWKBundlePageLoaderClientCurrentVersion,

wow this probably deserved a separate fix :), good catch

[Gustavo Noronha (kov)]

Comment on attachment 208638 [details]
Another update

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

LGTM, r=me, just need to get a wk2 owner to approve, I guess.

>> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp:305
>> +        kWKBundlePageLoaderClientCurrentVersion,
> 
> wow this probably deserved a separate fix :), good catch

Indeed, r=me for landing this separately!

[Carlos Garcia Campos]

(In reply to comment #7)
> (From update of attachment 208638 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=208638&action=review
> 
> LGTM, r=me, just need to get a wk2 owner to approve, I guess.
> 
> >> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp:305
> >> +        kWKBundlePageLoaderClientCurrentVersion,
> > 
> > wow this probably deserved a separate fix :), good catch
> 
> Indeed, r=me for landing this separately!

Fixed in http://trac.webkit.org/changeset/154474

[Anders Carlsson]

Comment on attachment 208638 [details]
Another update

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

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp:74
> +    DEFINE_STATIC_LOCAL(WebFrameMap, map, ());

Can use NeverDestroyed here!

[Carlos Garcia Campos]

Committed r154540: <http://trac.webkit.org/changeset/154540>

[Simon Fraser (smfr)]

This has caused 6 unit tests to fail:
http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK2%20%28Tests%29/builds/11911/steps/run-api-tests/logs/stdio

[Zan Dobersek]

(In reply to comment #11)
> This has caused 6 unit tests to fail:
> http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK2%20%28Tests%29/builds/11911/steps/run-api-tests/logs/stdio

Fix is up for review in bug #120268.