A Document / Window should lose its browsing context as soon as its iframe is removed...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Oct 2018 00:19:46 +0000 (00:19 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Oct 2018 00:19:46 +0000 (00:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190282

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

Rebaseline several WPT tests that are now passing. I have verified that those tests are also passing in
Firefox and Chrome.

* web-platform-tests/html/browsers/windows/nested-browsing-contexts/window-parent-null-expected.txt:
* web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe-synchronously-discard-expected.txt:

Source/WebCore:

A Document / Window should lose its browsing context (aka Frame) as soon as its iframe is removed from
the document. In WebKit, a Document / Window's Frame was only getting nulled out when the frame gets
destroyed, which happens later usually after a GC happens.

Specification:
- https://html.spec.whatwg.org/#the-iframe-element
"""
When an iframe element is removed from a document, the user agent must discard the element's nested browsing
context, if it is not null, and then set the element's nested browsing context to null.
"""

This was not consistent with the specification or other browsers (tested Chrome and Firefox) so this
patch is aligning our behavior.

In a follow-up, I am planning to look into making the Window not be a FrameDestructionObserver, and instead
get its frame from the Document. This should make the code simpler.

No new tests, rebaselined existing tests.

* Modules/mediastream/MediaDevices.cpp:
(WebCore::MediaDevices::getUserMedia const):
* Modules/mediastream/MediaDevices.h:
Update getUserMedia() to reject a the Promise with an InvalidStateError when calling after the
document has been detached, instead of throwing an InvalidStateError. This behavior is as per
specification:
- https://w3c.github.io/mediacapture-main/#dom-mediadevices-getusermedia (Step 4)
I needed to make this change to keep one of our layout tests passing.

* dom/Document.cpp:
(WebCore::Document::attachToCachedFrame):
(WebCore::Document::detachFromFrame):
* dom/Document.h:
* page/DOMWindow.cpp:
(WebCore::DOMWindow::didSecureTransitionTo):
(WebCore::DOMWindow::willDetachDocumentFromFrame):
(WebCore::DOMWindow::setStatus):
(WebCore::DOMWindow::detachFromFrame):
(WebCore::DOMWindow::attachToFrame):
* page/DOMWindow.h:
* page/DOMWindowProperty.cpp:
(WebCore::DOMWindowProperty::disconnectFrameForDocumentSuspension):
(WebCore::DOMWindowProperty::willDestroyGlobalObjectInCachedFrame):
(WebCore::DOMWindowProperty::willDestroyGlobalObjectInFrame):
* page/Frame.cpp:
(WebCore::Frame::disconnectOwnerElement):

* platform/mock/MockRealtimeVideoSource.cpp:
(WebCore::MockRealtimeVideoSource::drawText):
Calling drawText() with a null String hits an assertion in debug. This was triggered by one of
our layout tests so I made sure we only call drawText when the String is not null.

LayoutTests:

Update existing layout test to reflect behavior change.

* fast/dom/Window/BarInfo-after-frame-removed.html:
* fast/dom/Window/dom-access-from-closure-iframe-expected.txt:
* fast/dom/Window/dom-access-from-closure-window-expected.txt:
* fast/dom/Window/dom-access-from-closure-window-with-gc-expected.txt:
* fast/dom/Window/resources/dom-access-from-closure-iframe-child.html:
* fast/dom/Window/resources/dom-access-from-closure-window-child.html:
* fast/events/resources/before-unload-return-string-conversion-frame.html:
* fast/parser/resources/set-parent-to-javascript-url.html:
* http/tests/media/media-stream/disconnected-frame.html:
* http/tests/security/contentSecurityPolicy/resources/checkDidSameOriginChildWindowLoad.js:
(checkDidLoad):
* http/tests/security/named-window-property-from-same-origin-inactive-document-expected.txt:
* http/tests/security/named-window-property-from-same-origin-inactive-document.html:
* http/tests/security/xss-DENIED-contentWindow-eval-expected.txt:
* http/tests/security/xss-DENIED-named-window-property-from-cross-origin-inactive-document-expected.txt:
* http/tests/security/xss-DENIED-named-window-property-from-cross-origin-inactive-document.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236862 268f45cc-cd09-0410-ab3c-d52691b4dbfc

29 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/Window/BarInfo-after-frame-removed.html
LayoutTests/fast/dom/Window/dom-access-from-closure-iframe-expected.txt
LayoutTests/fast/dom/Window/dom-access-from-closure-window-expected.txt
LayoutTests/fast/dom/Window/dom-access-from-closure-window-with-gc-expected.txt
LayoutTests/fast/dom/Window/resources/dom-access-from-closure-iframe-child.html
LayoutTests/fast/dom/Window/resources/dom-access-from-closure-window-child.html
LayoutTests/fast/events/resources/before-unload-return-string-conversion-frame.html
LayoutTests/fast/parser/resources/set-parent-to-javascript-url.html
LayoutTests/http/tests/media/media-stream/disconnected-frame.html
LayoutTests/http/tests/security/contentSecurityPolicy/resources/checkDidSameOriginChildWindowLoad.js
LayoutTests/http/tests/security/named-window-property-from-same-origin-inactive-document-expected.txt
LayoutTests/http/tests/security/named-window-property-from-same-origin-inactive-document.html
LayoutTests/http/tests/security/xss-DENIED-contentWindow-eval-expected.txt
LayoutTests/http/tests/security/xss-DENIED-named-window-property-from-cross-origin-inactive-document-expected.txt
LayoutTests/http/tests/security/xss-DENIED-named-window-property-from-cross-origin-inactive-document.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/html/browsers/windows/nested-browsing-contexts/window-parent-null-expected.txt
LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe-synchronously-discard-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/MediaDevices.cpp
Source/WebCore/Modules/mediastream/MediaDevices.h
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/DOMWindow.h
Source/WebCore/page/DOMWindowProperty.cpp
Source/WebCore/page/Frame.cpp
Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp

index 5c259c4..59de6c5 100644 (file)
@@ -1,3 +1,29 @@
+2018-10-04  Chris Dumez  <cdumez@apple.com>
+
+        A Document / Window should lose its browsing context as soon as its iframe is removed from the document
+        https://bugs.webkit.org/show_bug.cgi?id=190282
+
+        Reviewed by Ryosuke Niwa.
+
+        Update existing layout test to reflect behavior change.
+
+        * fast/dom/Window/BarInfo-after-frame-removed.html:
+        * fast/dom/Window/dom-access-from-closure-iframe-expected.txt:
+        * fast/dom/Window/dom-access-from-closure-window-expected.txt:
+        * fast/dom/Window/dom-access-from-closure-window-with-gc-expected.txt:
+        * fast/dom/Window/resources/dom-access-from-closure-iframe-child.html:
+        * fast/dom/Window/resources/dom-access-from-closure-window-child.html:
+        * fast/events/resources/before-unload-return-string-conversion-frame.html:
+        * fast/parser/resources/set-parent-to-javascript-url.html:
+        * http/tests/media/media-stream/disconnected-frame.html:
+        * http/tests/security/contentSecurityPolicy/resources/checkDidSameOriginChildWindowLoad.js:
+        (checkDidLoad):
+        * http/tests/security/named-window-property-from-same-origin-inactive-document-expected.txt:
+        * http/tests/security/named-window-property-from-same-origin-inactive-document.html:
+        * http/tests/security/xss-DENIED-contentWindow-eval-expected.txt:
+        * http/tests/security/xss-DENIED-named-window-property-from-cross-origin-inactive-document-expected.txt:
+        * http/tests/security/xss-DENIED-named-window-property-from-cross-origin-inactive-document.html:
+
 2018-10-04  Ross Kirsling  <ross.kirsling@sony.com>
 
         Unreviewed test gardening for WinCairo (and one cross-platform test). 
 2018-10-04  Ross Kirsling  <ross.kirsling@sony.com>
 
         Unreviewed test gardening for WinCairo (and one cross-platform test). 
index 4f6773b..dc829fd 100644 (file)
@@ -15,7 +15,9 @@
         var frame = document.getElementById("frame");
         var childWindow = frame.contentWindow;
         frame.parentNode.removeChild(frame);
         var frame = document.getElementById("frame");
         var childWindow = frame.contentWindow;
         frame.parentNode.removeChild(frame);
-        childWindow.toolbar.visible;
+        try {
+            childWindow.toolbar.visible;
+        } catch (e) { }
         
         document.getElementById("console").firstChild.data = "TEST RAN";
     }
         
         document.getElementById("console").firstChild.data = "TEST RAN";
     }
index 0f38dd7..ff920c6 100644 (file)
@@ -1,5 +1,5 @@
 document.URL: LayoutTests/fast/dom/Window/resources/dom-access-from-closure-iframe-child.html
 window.document.URL: LayoutTests/fast/dom/Window/resources/notify-parent-done.html
 document.URL: LayoutTests/fast/dom/Window/resources/dom-access-from-closure-iframe-child.html
 window.document.URL: LayoutTests/fast/dom/Window/resources/notify-parent-done.html
-name: child
+name: 
 window.name: child
 
 window.name: child
 
index e70c0db..861f28f 100644 (file)
@@ -1,5 +1,5 @@
 document.URL: LayoutTests/fast/dom/Window/resources/dom-access-from-closure-window-child.html
 window.document.URL: LayoutTests/fast/dom/Window/resources/notify-opener-done.html
 document.URL: LayoutTests/fast/dom/Window/resources/dom-access-from-closure-window-child.html
 window.document.URL: LayoutTests/fast/dom/Window/resources/notify-opener-done.html
-name: child
+name: 
 window.name: child
 
 window.name: child
 
index e70c0db..861f28f 100644 (file)
@@ -1,5 +1,5 @@
 document.URL: LayoutTests/fast/dom/Window/resources/dom-access-from-closure-window-child.html
 window.document.URL: LayoutTests/fast/dom/Window/resources/notify-opener-done.html
 document.URL: LayoutTests/fast/dom/Window/resources/dom-access-from-closure-window-child.html
 window.document.URL: LayoutTests/fast/dom/Window/resources/notify-opener-done.html
-name: child
+name: 
 window.name: child
 
 window.name: child
 
index 4550d12..292774c 100644 (file)
@@ -1,4 +1,6 @@
 <script>
 <script>
+    const parent = window.parent; // Save parent as the window will be detached when accessFrame() is called.
+
     parent.accessFrame = function()
     {
         function normalizeURL(url)
     parent.accessFrame = function()
     {
         function normalizeURL(url)
index 3691095..3b9b234 100644 (file)
@@ -1,4 +1,6 @@
 <script>
 <script>
+    const opener = window.opener; // Save opener as the window will be detached when accessFrame() is called.
+
     opener.accessFrame = function()
     {
         function normalizeURL(url)
     opener.accessFrame = function()
     {
         function normalizeURL(url)
index 7387fbe..bd16de4 100644 (file)
@@ -14,9 +14,7 @@ window.runTest = function() {
         parent.shouldBeTrue("event.defaultPrevented");
         parent.shouldBeEqualToString("event.returnValue", "PASS");
         parent.shouldBeTrue("toStringCalled");
         parent.shouldBeTrue("event.defaultPrevented");
         parent.shouldBeEqualToString("event.returnValue", "PASS");
         parent.shouldBeTrue("toStringCalled");
-        parent.setTimeout(function() {
-            parent.finishJSTest();
-        }, 0);
+        parent.finishJSTest();
     }
 
     window.addEventListener("beforeunload", listener);
     }
 
     window.addEventListener("beforeunload", listener);
index a64b9a0..71d78bf 100644 (file)
@@ -1,4 +1,5 @@
 <script>
 <script>
+const parent = window.parent;
 alert(1);
 parent.document.getElementsByTagName('iframe')[0].src = "javascript:alert(2),'PASS<script>alert(3)<\/script>'";
 alert(4);
 alert(1);
 parent.document.getElementsByTagName('iframe')[0].src = "javascript:alert(2),'PASS<script>alert(3)<\/script>'";
 alert(4);
index 0f972c9..6db4b4e 100644 (file)
@@ -12,27 +12,29 @@ if (window.testRunner)
     testRunner.setUserMediaPermission(true);
 
 function onIframeLoaded() {
     testRunner.setUserMediaPermission(true);
 
 function onIframeLoaded() {
-    iframeNavigator = iframe.contentWindow.navigator;
+    iframeMediaDevices = iframe.contentWindow.navigator.mediaDevices;
     iframe.remove();
     onIframeUnloaded();
 }
 
 function onIframeUnloaded() {
     iframe.remove();
     onIframeUnloaded();
 }
 
 function onIframeUnloaded() {
+    handle = setTimeout(function() {
+        testFailed('Timeout: promise resolve and reject functions not called.');
+        finishJSTest();
+    }, 100);
+
     var options = {audio: true, video: true};
     var options = {audio: true, video: true};
-    iframeNavigator.mediaDevices.getUserMedia(options)
+    iframeMediaDevices.getUserMedia(options)
         .then(stream => {
             testFailed('Promise resolved unexpectedly.');
         .then(stream => {
             testFailed('Promise resolved unexpectedly.');
+            clearTimeout(handle);
             finishJSTest();
         })
         .catch(err => {
             testPassed('Promise rejected as expected.');
             finishJSTest();
         })
         .catch(err => {
             testPassed('Promise rejected as expected.');
+            clearTimeout(handle);
             finishJSTest();
         });
             finishJSTest();
         });
-
-    setTimeout(function() {
-        testFailed('Timeout: promise resolve and reject functions not called.');
-        finishJSTest();
-    }, 100);
 }
 
 var iframe = document.createElement('iframe');
 }
 
 var iframe = document.createElement('iframe');
index 91860b2..1e6847e 100644 (file)
@@ -8,12 +8,5 @@ function checkDidSameOriginChildWindowLoadAndNotifyDone(childWindow)
 
 function checkDidSameOriginChildWindowLoad(childWindow, callback)
 {
 
 function checkDidSameOriginChildWindowLoad(childWindow, callback)
 {
-    function checkDidLoad() {
-        if (childWindow.document.location.origin !== document.location.origin)
-            return;
-        // Child window did load
-        window.clearInterval(intervalID);
-        callback()
-    }
-    intervalID = window.setInterval(checkDidLoad, 10);
+    childWindow.onload = callback;
 }
 }
index 0718ad5..ab20b97 100644 (file)
@@ -5,10 +5,10 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 Lookup named element whose name corresponds to an element in the initial about:blank document:
 PASS frame.contentDocument.getElementById('A') is not elementAInInactiveDocument
 
 Lookup named element whose name corresponds to an element in the initial about:blank document:
 PASS frame.contentDocument.getElementById('A') is not elementAInInactiveDocument
-PASS elementAInActiveDocumentFunction() is frame.contentDocument.getElementById('A')
+PASS elementAInDetachedWindowFunction() threw exception ReferenceError: Can't find variable: A.
 
 Lookup named element whose name does not correspond to an element in the initial about:blank document:
 
 Lookup named element whose name does not correspond to an element in the initial about:blank document:
-PASS elementBInActiveDocumentFunction() is frame.contentDocument.getElementById('B')
+PASS elementBInDetachedWindowFunction() threw exception ReferenceError: Can't find variable: B.
 PASS successfullyParsed is true
 
 TEST COMPLETE
 PASS successfullyParsed is true
 
 TEST COMPLETE
index e7e4b61..e366f6d 100644 (file)
@@ -17,17 +17,17 @@ var elementAInInactiveDocument = frameDocument.createElement("div");
 elementAInInactiveDocument.id = "A";
 frameDocument.body.appendChild(elementAInInactiveDocument);
 
 elementAInInactiveDocument.id = "A";
 frameDocument.body.appendChild(elementAInInactiveDocument);
 
-var elementAInActiveDocumentFunction = frame.contentWindow.Function("return A;");
-var elementBInActiveDocumentFunction = frame.contentWindow.Function("return B;");
+var elementAInDetachedWindowFunction = frame.contentWindow.Function("return A;");
+var elementBInDetachedWindowFunction = frame.contentWindow.Function("return B;");
 
 frame.onload = function ()
 {
     debug("Lookup named element whose name corresponds to an element in the initial about:blank document:");
     shouldNotBe("frame.contentDocument.getElementById('A')", "elementAInInactiveDocument");
 
 frame.onload = function ()
 {
     debug("Lookup named element whose name corresponds to an element in the initial about:blank document:");
     shouldNotBe("frame.contentDocument.getElementById('A')", "elementAInInactiveDocument");
-    shouldBe("elementAInActiveDocumentFunction()", "frame.contentDocument.getElementById('A')");
+    shouldThrowErrorName("elementAInDetachedWindowFunction()", "ReferenceError");
 
     debug("<br>Lookup named element whose name does not correspond to an element in the initial about:blank document:");
 
     debug("<br>Lookup named element whose name does not correspond to an element in the initial about:blank document:");
-    shouldBe("elementBInActiveDocumentFunction()", "frame.contentDocument.getElementById('B')");
+    shouldThrowErrorName("elementBInDetachedWindowFunction()", "ReferenceError");
 
     finishJSTest();
 }
 
     finishJSTest();
 }
index f0ee8d2..7ffa04d 100644 (file)
@@ -1,2 +1 @@
-CONSOLE MESSAGE: line 1: TypeError: Type error
 This test passes if alert() is not called. 
 This test passes if alert() is not called. 
index 895f9bf..e4fa94d 100644 (file)
@@ -4,10 +4,10 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 Lookup named element whose name corresponds to an element in the initial about:blank document:
 
 
 Lookup named element whose name corresponds to an element in the initial about:blank document:
-PASS elementAInActiveDocumentFunction() threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
+PASS elementAInDetachedWindowFunction() threw exception ReferenceError: Can't find variable: A.
 
 Lookup named element whose name does not correspond to an element in the initial about:blank document:
 
 Lookup named element whose name does not correspond to an element in the initial about:blank document:
-PASS elementBInActiveDocumentFunction() threw exception SecurityError: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a cross-origin frame. Protocols, domains, and ports must match..
+PASS elementBInDetachedWindowFunction() threw exception ReferenceError: Can't find variable: B.
 PASS successfullyParsed is true
 
 TEST COMPLETE
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 35e4544..893aca7 100644 (file)
@@ -17,16 +17,16 @@ var elementAInInactiveDocument = frameDocument.createElement("div");
 elementAInInactiveDocument.id = "A";
 frameDocument.body.appendChild(elementAInInactiveDocument);
 
 elementAInInactiveDocument.id = "A";
 frameDocument.body.appendChild(elementAInInactiveDocument);
 
-var elementAInActiveDocumentFunction = frame.contentWindow.Function("return A;");
-var elementBInActiveDocumentFunction = frame.contentWindow.Function("return B;");
+var elementAInDetachedWindowFunction = frame.contentWindow.Function("return A;");
+var elementBInDetachedWindowFunction = frame.contentWindow.Function("return B;");
 
 frame.onload = function ()
 {
     debug("Lookup named element whose name corresponds to an element in the initial about:blank document:")
 
 frame.onload = function ()
 {
     debug("Lookup named element whose name corresponds to an element in the initial about:blank document:")
-    shouldThrowErrorName("elementAInActiveDocumentFunction()", 'SecurityError');
+    shouldThrowErrorName("elementAInDetachedWindowFunction()", 'ReferenceError');
 
     debug("<br>Lookup named element whose name does not correspond to an element in the initial about:blank document:");
 
     debug("<br>Lookup named element whose name does not correspond to an element in the initial about:blank document:");
-    shouldThrowErrorName("elementBInActiveDocumentFunction()", 'SecurityError');
+    shouldThrowErrorName("elementBInDetachedWindowFunction()", 'ReferenceError');
 
     finishJSTest();
 }
 
     finishJSTest();
 }
index 8402178..8b31bcb 100644 (file)
@@ -1,3 +1,16 @@
+2018-10-04  Chris Dumez  <cdumez@apple.com>
+
+        A Document / Window should lose its browsing context as soon as its iframe is removed from the document
+        https://bugs.webkit.org/show_bug.cgi?id=190282
+
+        Reviewed by Ryosuke Niwa.
+
+        Rebaseline several WPT tests that are now passing. I have verified that those tests are also passing in
+        Firefox and Chrome.
+
+        * web-platform-tests/html/browsers/windows/nested-browsing-contexts/window-parent-null-expected.txt:
+        * web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe-synchronously-discard-expected.txt:
+
 2018-10-04  YUHAN WU  <yuhan_wu@apple.com>
 
         runtime flag and IDL for MediaRecorder
 2018-10-04  YUHAN WU  <yuhan_wu@apple.com>
 
         runtime flag and IDL for MediaRecorder
index 15e9551..3f7a8fa 100644 (file)
@@ -1,4 +1,4 @@
 
 
-FAIL `window.parent` is null when browsing context container element removed assert_equals: expected null but got object "[object Window]"
-FAIL `window.parent` null when parent browsing context container removed assert_equals: expected null but got object "[object Window]"
-
+PASS `window.parent` is null when browsing context container element removed 
+PASS `window.parent` null when parent browsing context container removed 
index c37567d..31d5ee4 100644 (file)
@@ -1,3 +1,61 @@
+2018-10-04  Chris Dumez  <cdumez@apple.com>
+
+        A Document / Window should lose its browsing context as soon as its iframe is removed from the document
+        https://bugs.webkit.org/show_bug.cgi?id=190282
+
+        Reviewed by Ryosuke Niwa.
+
+        A Document / Window should lose its browsing context (aka Frame) as soon as its iframe is removed from
+        the document. In WebKit, a Document / Window's Frame was only getting nulled out when the frame gets
+        destroyed, which happens later usually after a GC happens.
+
+        Specification:
+        - https://html.spec.whatwg.org/#the-iframe-element
+        """
+        When an iframe element is removed from a document, the user agent must discard the element's nested browsing
+        context, if it is not null, and then set the element's nested browsing context to null.
+        """
+
+        This was not consistent with the specification or other browsers (tested Chrome and Firefox) so this
+        patch is aligning our behavior.
+
+        In a follow-up, I am planning to look into making the Window not be a FrameDestructionObserver, and instead
+        get its frame from the Document. This should make the code simpler.
+
+        No new tests, rebaselined existing tests.
+
+        * Modules/mediastream/MediaDevices.cpp:
+        (WebCore::MediaDevices::getUserMedia const):
+        * Modules/mediastream/MediaDevices.h:
+        Update getUserMedia() to reject a the Promise with an InvalidStateError when calling after the
+        document has been detached, instead of throwing an InvalidStateError. This behavior is as per
+        specification:
+        - https://w3c.github.io/mediacapture-main/#dom-mediadevices-getusermedia (Step 4)
+        I needed to make this change to keep one of our layout tests passing.
+
+        * dom/Document.cpp:
+        (WebCore::Document::attachToCachedFrame):
+        (WebCore::Document::detachFromFrame):
+        * dom/Document.h:
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::didSecureTransitionTo):
+        (WebCore::DOMWindow::willDetachDocumentFromFrame):
+        (WebCore::DOMWindow::setStatus):
+        (WebCore::DOMWindow::detachFromFrame):
+        (WebCore::DOMWindow::attachToFrame):
+        * page/DOMWindow.h:
+        * page/DOMWindowProperty.cpp:
+        (WebCore::DOMWindowProperty::disconnectFrameForDocumentSuspension):
+        (WebCore::DOMWindowProperty::willDestroyGlobalObjectInCachedFrame):
+        (WebCore::DOMWindowProperty::willDestroyGlobalObjectInFrame):
+        * page/Frame.cpp:
+        (WebCore::Frame::disconnectOwnerElement):
+
+        * platform/mock/MockRealtimeVideoSource.cpp:
+        (WebCore::MockRealtimeVideoSource::drawText):
+        Calling drawText() with a null String hits an assertion in debug. This was triggered by one of
+        our layout tests so I made sure we only call drawText when the String is not null.
+
 2018-10-04  Jeremy Jones  <jeremyj@apple.com>
 
         Unify implementation in VideoFullscreenInterfaceAVKit
 2018-10-04  Jeremy Jones  <jeremyj@apple.com>
 
         Unify implementation in VideoFullscreenInterfaceAVKit
index 690ff36..91c51ad 100644 (file)
@@ -96,11 +96,13 @@ static MediaConstraints createMediaConstraints(const Variant<bool, MediaTrackCon
     );
 }
 
     );
 }
 
-ExceptionOr<void> MediaDevices::getUserMedia(const StreamConstraints& constraints, Promise&& promise) const
+void MediaDevices::getUserMedia(const StreamConstraints& constraints, Promise&& promise) const
 {
     auto* document = this->document();
 {
     auto* document = this->document();
-    if (!document)
-        return Exception { InvalidStateError };
+    if (!document) {
+        promise.reject(Exception { InvalidStateError });
+        return;
+    }
 
     auto audioConstraints = createMediaConstraints(constraints.audio);
     auto videoConstraints = createMediaConstraints(constraints.video);
 
     auto audioConstraints = createMediaConstraints(constraints.audio);
     auto videoConstraints = createMediaConstraints(constraints.video);
@@ -111,7 +113,7 @@ ExceptionOr<void> MediaDevices::getUserMedia(const StreamConstraints& constraint
     if (request)
         request->start();
 
     if (request)
         request->start();
 
-    return { };
+    return;
 }
 
 ExceptionOr<void> MediaDevices::getDisplayMedia(const StreamConstraints& constraints, Promise&& promise) const
 }
 
 ExceptionOr<void> MediaDevices::getDisplayMedia(const StreamConstraints& constraints, Promise&& promise) const
index 7b2c7ca..7cda1eb 100644 (file)
@@ -74,7 +74,7 @@ public:
         Variant<bool, MediaTrackConstraints> video;
         Variant<bool, MediaTrackConstraints> audio;
     };
         Variant<bool, MediaTrackConstraints> video;
         Variant<bool, MediaTrackConstraints> audio;
     };
-    ExceptionOr<void> getUserMedia(const StreamConstraints&, Promise&&) const;
+    void getUserMedia(const StreamConstraints&, Promise&&) const;
     ExceptionOr<void> getDisplayMedia(const StreamConstraints&, Promise&&) const;
     void enumerateDevices(EnumerateDevicesPromise&&) const;
     MediaTrackSupportedConstraints getSupportedConstraints();
     ExceptionOr<void> getDisplayMedia(const StreamConstraints&, Promise&&) const;
     void enumerateDevices(EnumerateDevicesPromise&&) const;
     MediaTrackSupportedConstraints getSupportedConstraints();
index 5456a7c..75a82ce 100644 (file)
@@ -2341,6 +2341,8 @@ void Document::attachToCachedFrame(CachedFrameBase& cachedFrame)
     ASSERT(cachedFrame.view());
     ASSERT(m_pageCacheState == Document::InPageCache);
     observeFrame(&cachedFrame.view()->frame());
     ASSERT(cachedFrame.view());
     ASSERT(m_pageCacheState == Document::InPageCache);
     observeFrame(&cachedFrame.view()->frame());
+    if (auto* window = domWindow())
+        window->attachToFrame(cachedFrame.view()->frame());
 }
 
 void Document::detachFromCachedFrame(CachedFrameBase& cachedFrame)
 }
 
 void Document::detachFromCachedFrame(CachedFrameBase& cachedFrame)
@@ -8245,4 +8247,12 @@ bool Document::registerCSSProperty(CSSRegisteredCustomProperty&& prop)
     return m_CSSRegisteredPropertySet.add(prop.name, std::make_unique<CSSRegisteredCustomProperty>(WTFMove(prop))).isNewEntry;
 }
 
     return m_CSSRegisteredPropertySet.add(prop.name, std::make_unique<CSSRegisteredCustomProperty>(WTFMove(prop))).isNewEntry;
 }
 
+void Document::detachFromFrame()
+{
+    if (auto* window = domWindow())
+        window->willDetachDocumentFromFrame();
+
+    observeFrame(nullptr);
+}
+
 } // namespace WebCore
 } // namespace WebCore
index b442d49..d8b1ad9 100644 (file)
@@ -1505,6 +1505,8 @@ public:
     void setAsRunningUserScripts() { m_isRunningUserScripts = true; }
     bool isRunningUserScripts() const { return m_isRunningUserScripts; }
 
     void setAsRunningUserScripts() { m_isRunningUserScripts = true; }
     bool isRunningUserScripts() const { return m_isRunningUserScripts; }
 
+    void detachFromFrame();
+
 protected:
     enum ConstructionFlags { Synthesized = 1, NonRenderedPlaceholder = 1 << 1 };
     Document(Frame*, const URL&, unsigned = DefaultDocumentClass, unsigned constructionFlags = 0);
 protected:
     enum ConstructionFlags { Synthesized = 1, NonRenderedPlaceholder = 1 << 1 };
     Document(Frame*, const URL&, unsigned = DefaultDocumentClass, unsigned constructionFlags = 0);
@@ -1522,8 +1524,6 @@ private:
 
     bool shouldInheritContentSecurityPolicyFromOwner() const;
 
 
     bool shouldInheritContentSecurityPolicyFromOwner() const;
 
-    void detachFromFrame() { observeFrame(nullptr); }
-
     void updateTitleElement(Element& changingTitleElement);
     void frameDestroyed() final;
 
     void updateTitleElement(Element& changingTitleElement);
     void frameDestroyed() final;
 
index 7bde1cb..f6aa23c 100644 (file)
@@ -414,6 +414,7 @@ DOMWindow::DOMWindow(Document& document)
 void DOMWindow::didSecureTransitionTo(Document& document)
 {
     observeContext(&document);
 void DOMWindow::didSecureTransitionTo(Document& document)
 {
     observeContext(&document);
+    observeFrame(document.frame());
 }
 
 DOMWindow::~DOMWindow()
 }
 
 DOMWindow::~DOMWindow()
@@ -511,6 +512,8 @@ void DOMWindow::willDetachDocumentFromFrame()
 
     if (m_performance)
         m_performance->clearResourceTimings();
 
     if (m_performance)
         m_performance->clearResourceTimings();
+
+    detachFromFrame();
 }
 
 #if ENABLE(GAMEPAD)
 }
 
 #if ENABLE(GAMEPAD)
@@ -1402,7 +1405,17 @@ void DOMWindow::setStatus(const String& string)
 
     ASSERT(m_frame->document()); // Client calls shouldn't be made when the frame is in inconsistent state.
     page->chrome().setStatusbarText(*m_frame, m_status);
 
     ASSERT(m_frame->document()); // Client calls shouldn't be made when the frame is in inconsistent state.
     page->chrome().setStatusbarText(*m_frame, m_status);
-} 
+}
+
+void DOMWindow::detachFromFrame()
+{
+    observeFrame(nullptr);
+}
+
+void DOMWindow::attachToFrame(Frame& frame)
+{
+    observeFrame(&frame);
+}
     
 void DOMWindow::setDefaultStatus(const String& string) 
 {
     
 void DOMWindow::setDefaultStatus(const String& string) 
 {
index e2f49d1..3ed4703 100644 (file)
@@ -333,6 +333,9 @@ public:
     void willDetachDocumentFromFrame();
     void willDestroyCachedFrame();
 
     void willDetachDocumentFromFrame();
     void willDestroyCachedFrame();
 
+    void attachToFrame(Frame&);
+    void detachFromFrame();
+
     void enableSuddenTermination();
     void disableSuddenTermination();
 
     void enableSuddenTermination();
     void disableSuddenTermination();
 
index 55096b6..2c218ed 100644 (file)
@@ -57,9 +57,6 @@ DOMWindowProperty::~DOMWindowProperty()
 
 void DOMWindowProperty::disconnectFrameForDocumentSuspension()
 {
 
 void DOMWindowProperty::disconnectFrameForDocumentSuspension()
 {
-    // If this property is being disconnected from its Frame to enter the PageCache, it must have
-    // been created with a Frame in the first place.
-    ASSERT(m_frame);
     ASSERT(m_associatedDOMWindow);
 
     m_frame = nullptr;
     ASSERT(m_associatedDOMWindow);
 
     m_frame = nullptr;
@@ -93,8 +90,6 @@ void DOMWindowProperty::willDestroyGlobalObjectInCachedFrame()
 
 void DOMWindowProperty::willDestroyGlobalObjectInFrame()
 {
 
 void DOMWindowProperty::willDestroyGlobalObjectInFrame()
 {
-    // If the property is getting this callback it must have been created with a Frame/DOMWindow and it should still have them.
-    ASSERT(m_frame);
     ASSERT(m_associatedDOMWindow);
 
     // DOMWindowProperty lifetime isn't tied directly to the DOMWindow itself so it is important that it unregister
     ASSERT(m_associatedDOMWindow);
 
     // DOMWindowProperty lifetime isn't tied directly to the DOMWindow itself so it is important that it unregister
@@ -107,9 +102,7 @@ void DOMWindowProperty::willDestroyGlobalObjectInFrame()
 
 void DOMWindowProperty::willDetachGlobalObjectFromFrame()
 {
 
 void DOMWindowProperty::willDetachGlobalObjectFromFrame()
 {
-    // If the property is getting this callback it must have been created with a Frame/DOMWindow and it should still have them.
-    ASSERT(m_frame);
-    ASSERT(m_associatedDOMWindow);
+    m_frame = nullptr;
 }
 
 }
 }
 
 }
index 193ec77..cbe45e3 100644 (file)
@@ -831,6 +831,9 @@ void Frame::disconnectOwnerElement()
             m_page->decrementSubframeCount();
     }
     m_ownerElement = nullptr;
             m_page->decrementSubframeCount();
     }
     m_ownerElement = nullptr;
+
+    if (auto* document = this->document())
+        document->detachFromFrame();
 }
 
 String Frame::displayStringModifiedByEncoding(const String& str) const
 }
 
 String Frame::displayStringModifiedByEncoding(const String& str) const
index 5d3fb91..8681f66 100644 (file)
@@ -372,7 +372,7 @@ void MockRealtimeVideoSource::drawText(GraphicsContext& context)
         string = String::format("Camera: %s", camera);
         statsLocation.move(0, m_statsFontSize);
         context.drawText(statsFont, TextRun((StringView(string))), statsLocation);
         string = String::format("Camera: %s", camera);
         statsLocation.move(0, m_statsFontSize);
         context.drawText(statsFont, TextRun((StringView(string))), statsLocation);
-    } else {
+    } else if (!name().isNull()) {
         statsLocation.move(0, m_statsFontSize);
         context.drawText(statsFont, TextRun { name() }, statsLocation);
     }
         statsLocation.move(0, m_statsFontSize);
         context.drawText(statsFont, TextRun { name() }, statsLocation);
     }