Bug 73379 - [chromium] add setOpener method to WebFrame
Summary: [chromium] add setOpener method to WebFrame
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Karl Koscher
URL:
Keywords:
Depends on:
Blocks: 73337
  Show dependency treegraph
 
Reported: 2011-11-29 17:28 PST by Karl Koscher
Modified: 2011-12-02 16:54 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.85 KB, patch)
2011-11-29 17:28 PST, Karl Koscher
fishd: review-
Details | Formatted Diff | Diff
Patch update (2.10 KB, patch)
2011-12-01 13:43 PST, Karl Koscher
fishd: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Same patch, now with the missing ChangeLog (2.82 KB, patch)
2011-12-02 10:57 PST, Karl Koscher
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Karl Koscher 2011-11-29 17:28:35 PST
Created attachment 117080 [details]
Patch

To get cross-process postMessage working in Chromium (bug 73337), we need to be able to give frames the ability to send messages to their openers. This requires a chain of opener proxies. Chromium will create new WebViews for the proxies but needs a way to point a frame to its proxy opener. Adding a setOpener to WebFrame allows Chromium to do so.
Comment 1 WebKit Review Bot 2011-11-29 17:30:19 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 2 Darin Fisher (:fishd, Google) 2011-11-29 21:38:53 PST
Comment on attachment 117080 [details]
Patch

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

> Source/WebKit/chromium/public/WebFrame.h:221
> +    virtual void setOpener(const WebFrame*) = 0;

nit: please preserve two new lines above section header

note: we already have opener() and clearOpener().  it seems like setOpener() should be
listed next to those.  also, perhaps we should delete clearOpener() in favor of setOpener(0).
you could at least implement clearOpener() as an inline call to setOpener(0).

> Source/WebKit/chromium/src/WebFrameImpl.cpp:768
> +        static_cast<const WebFrameImpl *>(frame)->m_frame : 0);

nit: no space after "WebFrameImpl"
Comment 3 Karl Koscher 2011-12-01 13:43:08 PST
Created attachment 117478 [details]
Patch update

(In reply to comment #2)
> (From update of attachment 117080 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117080&action=review
> 
> > Source/WebKit/chromium/public/WebFrame.h:221
> > +    virtual void setOpener(const WebFrame*) = 0;
> 
> nit: please preserve two new lines above section header

Fixed.
 
> note: we already have opener() and clearOpener().  it seems like setOpener() should be
> listed next to those.  also, perhaps we should delete clearOpener() in favor of setOpener(0).
> you could at least implement clearOpener() as an inline call to setOpener(0).

I have no idea why I put setOpener there. I moved it and made clearOpener an inline call to setOpener(0).

> > Source/WebKit/chromium/src/WebFrameImpl.cpp:768
> > +        static_cast<const WebFrameImpl *>(frame)->m_frame : 0);
> 
> nit: no space after "WebFrameImpl"

Oops. Old habits. :) Fixed.
Comment 4 WebKit Review Bot 2011-12-01 23:18:09 PST
Comment on attachment 117478 [details]
Patch update

Rejecting attachment 117478 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
-queue/Source/WebKit/chromium/tools/xdisplaycheck --revision 112463 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
46>At revision 112463.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/10693949
Comment 5 Karl Koscher 2011-12-02 10:57:26 PST
Created attachment 117655 [details]
Same patch, now with the missing ChangeLog

webkit-patch was pulling in unrelated changes, so I created this patch manually and forgot to update ChangeLog. This patch is the same as the previous one, just with a ChangeLog entry.
Comment 6 WebKit Review Bot 2011-12-02 14:52:01 PST
Comment on attachment 117655 [details]
Same patch, now with the missing ChangeLog

Rejecting attachment 117655 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

Last 500 characters of output:
line 32, in <module>
    from webkitpy.common.checkout.changelog import ChangeLog, parse_bug_id_from_changelog
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/checkout/changelog.py", line 36, in <module>
    from webkitpy.common.config.committers import CommitterList
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/config/committers.py", line 135
    Contributor("Pierre Rossi", "pierre.rossi@gmail.com", "elproxy"),
              ^
SyntaxError: invalid syntax

Full output: http://queues.webkit.org/results/10725551
Comment 7 WebKit Review Bot 2011-12-02 16:54:02 PST
Comment on attachment 117655 [details]
Same patch, now with the missing ChangeLog

Clearing flags on attachment: 117655

Committed r101879: <http://trac.webkit.org/changeset/101879>
Comment 8 WebKit Review Bot 2011-12-02 16:54:07 PST
All reviewed patches have been landed.  Closing bug.