Bug 184716 - Set RemoteDOMWindow's initial opener
Summary: Set RemoteDOMWindow's initial opener
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 184515
Blocks: 184466 184756
  Show dependency treegraph
 
Reported: 2018-04-17 16:56 PDT by Chris Dumez
Modified: 2018-04-18 15:28 PDT (History)
9 users (show)

See Also:


Attachments
WIP Patch (12.36 KB, patch)
2018-04-17 16:57 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (27.31 KB, patch)
2018-04-18 13:17 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2018-04-17 16:56:48 PDT
Set RemoteDOMWindow's initial opener.
Comment 1 Chris Dumez 2018-04-17 16:57:33 PDT
Created attachment 338160 [details]
WIP Patch
Comment 2 Sam Weinig 2018-04-17 19:34:14 PDT
As I mentioned to Chris elsewhere, I think I might consider having RemoteDOMWindow::opener() return a WindowProxyController (and perhaps rename it to WindowProxy or DOMWindowProxy) since a WindowProxyController is just the window proxy part of an AbstractFrame.  I would further change everything (in the IDLs) that returns a DOMWindow to return a WindowProxy instead.
Comment 3 Chris Dumez 2018-04-17 19:56:28 PDT
(In reply to Sam Weinig from comment #2)
> As I mentioned to Chris elsewhere, I think I might consider having
> RemoteDOMWindow::opener() return a WindowProxyController (and perhaps rename
> it to WindowProxy or DOMWindowProxy) since a WindowProxyController is just
> the window proxy part of an AbstractFrame.  I would further change
> everything (in the IDLs) that returns a DOMWindow to return a WindowProxy
> instead.

I think I could return a WindowProxyController instead of an AbstractFrame in this patch, I like the idea and this is a small change.

Updating all the IDLs to use WindowProxy is definitely the way to go, I agree but I think this is out of scope of this patch. I can take care of it in a follow-up.
Comment 4 Chris Dumez 2018-04-18 11:52:42 PDT
(In reply to Sam Weinig from comment #2)
> As I mentioned to Chris elsewhere, I think I might consider having
> RemoteDOMWindow::opener() return a WindowProxyController (and perhaps rename
> it to WindowProxy or DOMWindowProxy) since a WindowProxyController is just
> the window proxy part of an AbstractFrame.  I would further change
> everything (in the IDLs) that returns a DOMWindow to return a WindowProxy
> instead.

Note that forcing the implementation to return a WindowProxyController means we have to null check the frame. Previously, we'd just return a potentially null frame and toJS() would take care of the null check for us.
Comment 5 Chris Dumez 2018-04-18 13:17:24 PDT
Created attachment 338251 [details]
Patch
Comment 6 Sam Weinig 2018-04-18 13:31:05 PDT
Comment on attachment 338251 [details]
Patch

Very nice.
Comment 7 WebKit Commit Bot 2018-04-18 15:27:25 PDT
Comment on attachment 338251 [details]
Patch

Clearing flags on attachment: 338251

Committed r230789: <https://trac.webkit.org/changeset/230789>
Comment 8 WebKit Commit Bot 2018-04-18 15:27:27 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2018-04-18 15:28:31 PDT
<rdar://problem/39543247>