WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
184653
Add bindings code for RemoteDOMWindow
https://bugs.webkit.org/show_bug.cgi?id=184653
Summary
Add bindings code for RemoteDOMWindow
Chris Dumez
Reported
2018-04-16 09:13:33 PDT
Add bindings code for RemoteDOMWindow.
Attachments
Patch
(55.69 KB, patch)
2018-04-16 10:48 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews100 for mac-sierra
(526.29 KB, application/zip)
2018-04-16 11:43 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews116 for mac-sierra
(1.45 MB, application/zip)
2018-04-16 11:44 PDT
,
EWS Watchlist
no flags
Details
Patch
(56.99 KB, patch)
2018-04-16 11:49 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(57.58 KB, patch)
2018-04-16 12:06 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(57.68 KB, patch)
2018-04-16 13:27 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(58.52 KB, patch)
2018-04-17 08:52 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2018-04-16 10:48:24 PDT
Created
attachment 338014
[details]
Patch
EWS Watchlist
Comment 2
2018-04-16 11:43:06 PDT
Comment on
attachment 338014
[details]
Patch
Attachment 338014
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/7332946
Number of test failures exceeded the failure limit.
EWS Watchlist
Comment 3
2018-04-16 11:43:07 PDT
Created
attachment 338018
[details]
Archive of layout-test-results from ews100 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6
EWS Watchlist
Comment 4
2018-04-16 11:44:09 PDT
Comment on
attachment 338014
[details]
Patch
Attachment 338014
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/7332714
Number of test failures exceeded the failure limit.
EWS Watchlist
Comment 5
2018-04-16 11:44:11 PDT
Created
attachment 338019
[details]
Archive of layout-test-results from ews116 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-sierra Platform: Mac OS X 10.12.6
Chris Dumez
Comment 6
2018-04-16 11:49:16 PDT
Created
attachment 338021
[details]
Patch
Chris Dumez
Comment 7
2018-04-16 12:06:52 PDT
Created
attachment 338023
[details]
Patch
Chris Dumez
Comment 8
2018-04-16 13:27:15 PDT
Created
attachment 338030
[details]
Patch
Ryosuke Niwa
Comment 9
2018-04-16 21:23:27 PDT
Comment on
attachment 338030
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=338030&action=review
> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:82 > +template <IsRemoteDOMWindow isRemoteDOMWindow>
I think I'd prefer having DOMWindowType / DOMWindowLocation enum with Remote and Local as values.
> Source/WebCore/bindings/js/JSRemoteDOMWindowBase.h:37 > +class WEBCORE_EXPORT JSRemoteDOMWindowBase : public JSDOMGlobalObject {
Why is this class introduced / needed? Please explain it in the change log.
Chris Dumez
Comment 10
2018-04-17 08:46:35 PDT
(In reply to Ryosuke Niwa from
comment #9
)
> Comment on
attachment 338030
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=338030&action=review
> > > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:82 > > +template <IsRemoteDOMWindow isRemoteDOMWindow> > > I think I'd prefer having DOMWindowType / DOMWindowLocation enum with Remote > and Local as values.
OK.
> > > Source/WebCore/bindings/js/JSRemoteDOMWindowBase.h:37 > > +class WEBCORE_EXPORT JSRemoteDOMWindowBase : public JSDOMGlobalObject { > > Why is this class introduced / needed? Please explain it in the change log.
I will add it to the ChangeLog. Basically, JSRemoteDOMWindow needs to be a JSGlobalObject because: 1. a JSProxy's target needs to be a JSGlobalObject currently 2. The 'structure()->setGlobalObject(vm, &window);' call in JSDOMWindowProxy::setWindow(VM&, JSDOMGlobalObject&) which requires a JSGlobalObject Ideally, this wouldn't be the case in the future but this would require some code refactoring. Our DOM global objects normally subclass JSDOMGlobalObject so I decided to subclass JSDOMGlobalObject, which brings some things our bindings code expect. However, subclassing JSDOMGlobalObject directly is problematic because it does not hold the m_wrapped implementation pointer. To address this issue, all our our DOM global objects have a JS*Base base class which subclasses JSDOMGlobalObject and stores the m_wrapped implementation pointer. I followed the same pattern here. Again, ideally, in the future, JSRemoteDOMWindow wouldn't need to be a GlobalObject at all and we would be able to get rid of all this code.
Chris Dumez
Comment 11
2018-04-17 08:52:31 PDT
Created
attachment 338121
[details]
Patch
WebKit Commit Bot
Comment 12
2018-04-17 09:58:51 PDT
The commit-queue encountered the following flaky tests while processing
attachment 338121
[details]
: imported/w3c/web-platform-tests/dom/nodes/Document-constructor.html
bug 184700
(authors:
cdumez@apple.com
and
youennf@gmail.com
) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 13
2018-04-17 09:59:40 PDT
Comment on
attachment 338121
[details]
Patch Clearing flags on attachment: 338121 Committed
r230715
: <
https://trac.webkit.org/changeset/230715
>
WebKit Commit Bot
Comment 14
2018-04-17 09:59:42 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 15
2018-04-17 10:00:27 PDT
<
rdar://problem/39493862
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug