Bug 184653

Created attachment 338014 [details] Patch

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.

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

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.

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

Created attachment 338021 [details] Patch

Created attachment 338023 [details] Patch

Created attachment 338030 [details] Patch

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.

(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.

Created attachment 338121 [details] Patch

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.

Comment on attachment 338121 [details] Patch Clearing flags on attachment: 338121 Committed r230715: <https://trac.webkit.org/changeset/230715>

All reviewed patches have been landed. Closing bug.

<rdar://problem/39493862>