Bug 184716

Summary: Set RemoteDOMWindow's initial opener
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: BindingsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, esprehn+autocc, ews-watchlist, ggaren, kondapallykalyan, rniwa, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 184515    
Bug Blocks: 184466, 184756    
Attachments:
Description Flags
WIP Patch
none
Patch none

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>