Bug 119743 - [GTK] Expose WebKitFrame in WebKit2GTK+ web extensions API
Summary: [GTK] Expose WebKitFrame in WebKit2GTK+ web extensions API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 103377
  Show dependency treegraph
 
Reported: 2013-08-13 06:30 PDT by Carlos Garcia Campos
Modified: 2013-08-25 12:09 PDT (History)
17 users (show)

See Also:


Attachments
Patch (18.46 KB, patch)
2013-08-13 06:44 PDT, Carlos Garcia Campos
gtk-ews: commit-queue-
Details | Formatted Diff | Diff
Updated patch (25.88 KB, patch)
2013-08-13 08:06 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Another update (32.83 KB, patch)
2013-08-13 09:05 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 Carlos Garcia Campos 2013-08-13 06:30:31 PDT
This is required for the javascript injection and isolated worlds API. See https://lists.webkit.org/pipermail/webkit-gtk/2013-August/001535.html.
Comment 1 Carlos Garcia Campos 2013-08-13 06:44:27 PDT
Created attachment 208624 [details]
Patch
Comment 2 WebKit Commit Bot 2013-08-13 06:46:20 PDT
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
Comment 3 kov's GTK+ EWS bot 2013-08-13 07:44:15 PDT
Comment on attachment 208624 [details]
Patch

Attachment 208624 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1452440
Comment 4 Carlos Garcia Campos 2013-08-13 08:06:40 PDT
Created attachment 208634 [details]
Updated patch

I forgot to git add some new files
Comment 5 Carlos Garcia Campos 2013-08-13 09:05:39 PDT
Created attachment 208638 [details]
Another update

Sorry, I forgot to git add two more files.
Comment 6 Sergio Villar Senin 2013-08-13 09:14:51 PDT
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
Comment 7 Gustavo Noronha (kov) 2013-08-14 16:07:07 PDT
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!
Comment 8 Carlos Garcia Campos 2013-08-22 23:29:09 PDT
(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
Comment 9 Anders Carlsson 2013-08-23 05:51:04 PDT
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!
Comment 10 Carlos Garcia Campos 2013-08-24 01:43:46 PDT
Committed r154540: <http://trac.webkit.org/changeset/154540>
Comment 12 Zan Dobersek 2013-08-25 12:09:38 PDT
(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.