window.navigator should not become null after the window loses its browsing context
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Oct 2018 16:24:12 +0000 (16:24 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Oct 2018 16:24:12 +0000 (16:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190595

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

Rebaseline test which is not failing differently. The last check of this test is checking that
navigator.serviceWorker returns null after the frame has been detached. The test has been written
this way because this is how Chromium behaves. However, Firefox keeps returning the
ServiceWorkerContainer, as we do. Also, the specification indicates the the attribute cannot
return null (since the attribute is not nullable):
- https://w3c.github.io/ServiceWorker/#navigator-serviceworker

* web-platform-tests/service-workers/service-worker/detached-context.https-expected.txt:

Source/WebCore:

window.navigator should not become null after the window loses its browsing context.
This does not match the HTML specification or the behavior of other browsers.

No new tests, updated existing tests.

* Modules/geolocation/NavigatorGeolocation.cpp:
(WebCore::NavigatorGeolocation::geolocation const):
* page/DOMWindow.cpp:
(WebCore::DOMWindow::navigator):
* page/Navigator.cpp:
(WebCore::Navigator::Navigator):
* page/Navigator.h:
* page/NavigatorBase.cpp:
(WebCore::NavigatorBase::NavigatorBase):
* page/NavigatorBase.h:
* page/WorkerNavigator.cpp:
(WebCore::WorkerNavigator::WorkerNavigator):
* workers/service/ServiceWorkerContainer.cpp:
(WebCore::ServiceWorkerContainer::ServiceWorkerContainer):
* workers/service/ServiceWorkerContainer.h:

LayoutTests:

Extend layout test coverage.

* fast/frames/detached-frame-property-expected.txt:
* fast/frames/detached-frame-property.html:
* http/tests/dom/cross-origin-detached-window-properties-expected.txt:
* http/tests/dom/cross-origin-detached-window-properties.html:
* http/tests/dom/same-origin-detached-window-properties-expected.txt:
* http/tests/dom/same-origin-detached-window-properties.html:

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

19 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/frames/detached-frame-property-expected.txt
LayoutTests/fast/frames/detached-frame-property.html
LayoutTests/http/tests/dom/cross-origin-detached-window-properties-expected.txt
LayoutTests/http/tests/dom/cross-origin-detached-window-properties.html
LayoutTests/http/tests/dom/same-origin-detached-window-properties-expected.txt
LayoutTests/http/tests/dom/same-origin-detached-window-properties.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/detached-context.https-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/Modules/geolocation/NavigatorGeolocation.cpp
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/Navigator.cpp
Source/WebCore/page/Navigator.h
Source/WebCore/page/NavigatorBase.cpp
Source/WebCore/page/NavigatorBase.h
Source/WebCore/page/WorkerNavigator.cpp
Source/WebCore/workers/service/ServiceWorkerContainer.cpp
Source/WebCore/workers/service/ServiceWorkerContainer.h

index 97b5ee9..52f19c7 100644 (file)
@@ -1,3 +1,19 @@
+2018-10-16  Chris Dumez  <cdumez@apple.com>
+
+        window.navigator should not become null after the window loses its browsing context
+        https://bugs.webkit.org/show_bug.cgi?id=190595
+
+        Reviewed by Ryosuke Niwa.
+
+        Extend layout test coverage.
+
+        * fast/frames/detached-frame-property-expected.txt:
+        * fast/frames/detached-frame-property.html:
+        * http/tests/dom/cross-origin-detached-window-properties-expected.txt:
+        * http/tests/dom/cross-origin-detached-window-properties.html:
+        * http/tests/dom/same-origin-detached-window-properties-expected.txt:
+        * http/tests/dom/same-origin-detached-window-properties.html:
+
 2018-10-16  Charlie Turner  <cturner@igalia.com>
 
         [EME] Multiple ClearKey tests crashing in gst_qtdemux_request_protection_context
index 02ded49..ff243f8 100644 (file)
@@ -9,6 +9,7 @@ PASS !!detachedWindow.locationbar is true
 PASS !!detachedWindow.history is true
 PASS !!detachedWindow.screen is true
 PASS !!detachedWindow.location is true
+PASS !!detachedWindow.navigator is true
 PASS detachedWindow.closed is true
 PASS detachedWindow.top is null
 PASS detachedWindow.opener is null
@@ -17,7 +18,6 @@ PASS detachedWindow.frameElement is null
 PASS detachedWindow.window is null
 PASS detachedWindow.frames is null
 PASS detachedWindow.self is null
-PASS !detachedWindow.navigator is true
 PASS !detachedWindow.localStorage is true
 PASS !!detachedWindow.document is true
 PASS !!detachedWindow.XMLHttpRequest is true
index 2530b50..e696e34 100644 (file)
@@ -13,6 +13,7 @@ onload = function()
     shouldBeTrue("!!detachedWindow.history");
     shouldBeTrue("!!detachedWindow.screen");
     shouldBeTrue("!!detachedWindow.location");
+    shouldBeTrue("!!detachedWindow.navigator");
     shouldBeTrue("detachedWindow.closed");
     shouldBeNull("detachedWindow.top");
     shouldBeNull("detachedWindow.opener");
@@ -24,7 +25,6 @@ onload = function()
     shouldBeNull("detachedWindow.self");
 
     // Chrome returns undefined but Firefox has a valid object.
-    shouldBeTrue("!detachedWindow.navigator");
     shouldBeTrue("!detachedWindow.localStorage");
     shouldBeTrue("!!detachedWindow.document");
     shouldBeTrue("!!detachedWindow.XMLHttpRequest");
index cffc61f..52d5bf8 100644 (file)
@@ -29,6 +29,7 @@ PASS w.toolbar threw exception SecurityError: Blocked a frame with origin "http:
 PASS w.applicationCache 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 w.visualViewport 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 w.styleMedia 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 w.navigator 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 w.foo 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 w.location.foo 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..
 
@@ -58,6 +59,7 @@ PASS w.toolbar threw exception SecurityError: Blocked a frame with origin "http:
 PASS w.applicationCache 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 w.visualViewport 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 w.styleMedia 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 w.navigator 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 w.foo 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 w.location.foo 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 successfullyParsed is true
index fd1a22a..43bb6f1 100644 (file)
@@ -44,6 +44,7 @@ function validateWindow(_w)
     shouldThrowErrorName("w.applicationCache", "SecurityError");
     shouldThrowErrorName("w.visualViewport", "SecurityError");
     shouldThrowErrorName("w.styleMedia", "SecurityError");
+    shouldThrowErrorName("w.navigator", "SecurityError");
 
     shouldThrowErrorName("w.foo", "SecurityError");
     shouldThrowErrorName("w.location.foo", "SecurityError");
index c838207..7116e3a 100644 (file)
@@ -63,6 +63,17 @@ PASS w.visualViewport.scale is 1
 PASS !!w.styleMedia is true
 PASS w.styleMedia.type is ""
 PASS w.styleMedia.matchMedium('foo') is false
+PASS !!w.navigator is true
+PASS w.navigator.appCodeName is "Mozilla"
+PASS w.navigator.appName is "Netscape"
+PASS w.navigator.appVersion is ""
+PASS w.navigator.cookieEnabled is false
+PASS w.navigator.javaEnabled() is false
+PASS w.navigator.product is "Gecko"
+PASS w.navigator.userAgent is ""
+PASS w.navigator.plugins.length is 0
+PASS w.navigator.mimeTypes.length is 0
+PASS !!w.navigator.geolocation is true
 PASS w.foo is undefined.
 PASS w.location.foo is undefined.
 
@@ -126,6 +137,17 @@ PASS w.visualViewport.scale is 1
 PASS !!w.styleMedia is true
 PASS w.styleMedia.type is ""
 PASS w.styleMedia.matchMedium('foo') is false
+PASS !!w.navigator is true
+PASS w.navigator.appCodeName is "Mozilla"
+PASS w.navigator.appName is "Netscape"
+PASS w.navigator.appVersion is ""
+PASS w.navigator.cookieEnabled is false
+PASS w.navigator.javaEnabled() is false
+PASS w.navigator.product is "Gecko"
+PASS w.navigator.userAgent is ""
+PASS w.navigator.plugins.length is 0
+PASS w.navigator.mimeTypes.length is 0
+PASS !!w.navigator.geolocation is true
 PASS w.foo is undefined.
 PASS w.location.foo is undefined.
 PASS successfullyParsed is true
index 68c1a2a..b167cbe 100644 (file)
@@ -119,6 +119,24 @@ function validateWindow(_w)
     }
 
     try {
+    shouldBeTrue("!!w.navigator");
+    if (w.navigator) {
+        shouldBeEqualToString("w.navigator.appCodeName", "Mozilla");
+        shouldBeEqualToString("w.navigator.appName", "Netscape");
+        shouldBeEqualToString("w.navigator.appVersion", "");
+        shouldBeFalse("w.navigator.cookieEnabled");
+        shouldBeFalse("w.navigator.javaEnabled()");
+        shouldBeEqualToString("w.navigator.product", "Gecko");
+        shouldBeEqualToString("w.navigator.userAgent", "");
+        shouldBe("w.navigator.plugins.length", "0");
+        shouldBe("w.navigator.mimeTypes.length", "0");
+        shouldBeTrue("!!w.navigator.geolocation");
+    }
+    } catch (e) {
+        debug(e);
+    }
+
+    try {
     shouldBeUndefined("w.foo");
     shouldBeUndefined("w.location.foo");
     } catch (e) {
index 43be66d..ab7f46d 100644 (file)
@@ -1,3 +1,19 @@
+2018-10-16  Chris Dumez  <cdumez@apple.com>
+
+        window.navigator should not become null after the window loses its browsing context
+        https://bugs.webkit.org/show_bug.cgi?id=190595
+
+        Reviewed by Ryosuke Niwa.
+
+        Rebaseline test which is not failing differently. The last check of this test is checking that
+        navigator.serviceWorker returns null after the frame has been detached. The test has been written
+        this way because this is how Chromium behaves. However, Firefox keeps returning the
+        ServiceWorkerContainer, as we do. Also, the specification indicates the the attribute cannot
+        return null (since the attribute is not nullable):
+        - https://w3c.github.io/ServiceWorker/#navigator-serviceworker
+
+        * web-platform-tests/service-workers/service-worker/detached-context.https-expected.txt:
+
 2018-10-16  Charlie Turner  <cturner@igalia.com>
 
         [EME] Add some ClearKey baselines for passing tests
index bde430c..d803f9f 100644 (file)
@@ -4,5 +4,5 @@ PASS accessing a ServiceWorkerRegistration from a removed iframe
 PASS accessing a ServiceWorker object from a removed iframe 
 PASS accessing navigator.serviceWorker on a detached iframe 
 FAIL accessing navigator on a removed frame assert_throws: function "() => get_navigator()" did not throw
-FAIL accessing navigator.serviceWorker on a removed about:blank frame null is not an object (evaluating 'get_navigator().serviceWorker')
+FAIL accessing navigator.serviceWorker on a removed about:blank frame assert_equals: expected null but got object "[object ServiceWorkerContainer]"
 
index 5f53e7c..eccd518 100644 (file)
@@ -1,3 +1,31 @@
+2018-10-16  Chris Dumez  <cdumez@apple.com>
+
+        window.navigator should not become null after the window loses its browsing context
+        https://bugs.webkit.org/show_bug.cgi?id=190595
+
+        Reviewed by Ryosuke Niwa.
+
+        window.navigator should not become null after the window loses its browsing context.
+        This does not match the HTML specification or the behavior of other browsers.
+
+        No new tests, updated existing tests.
+
+        * Modules/geolocation/NavigatorGeolocation.cpp:
+        (WebCore::NavigatorGeolocation::geolocation const):
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::navigator):
+        * page/Navigator.cpp:
+        (WebCore::Navigator::Navigator):
+        * page/Navigator.h:
+        * page/NavigatorBase.cpp:
+        (WebCore::NavigatorBase::NavigatorBase):
+        * page/NavigatorBase.h:
+        * page/WorkerNavigator.cpp:
+        (WebCore::WorkerNavigator::WorkerNavigator):
+        * workers/service/ServiceWorkerContainer.cpp:
+        (WebCore::ServiceWorkerContainer::ServiceWorkerContainer):
+        * workers/service/ServiceWorkerContainer.h:
+
 2018-10-16  Alex Christensen  <achristensen@webkit.org>
 
         Replace HistoryItem* with HistoryItem& where possible
index 64df735..8ab4cb9 100644 (file)
@@ -26,6 +26,7 @@
 
 #include "NavigatorGeolocation.h"
 
+#include "DOMWindow.h"
 #include "Document.h"
 #include "Frame.h"
 #include "Geolocation.h"
@@ -71,8 +72,8 @@ Geolocation* NavigatorGeolocation::geolocation(Navigator& navigator)
 
 Geolocation* NavigatorGeolocation::geolocation() const
 {
-    if (!m_geolocation && frame())
-        m_geolocation = Geolocation::create(frame()->document());
+    if (!m_geolocation)
+        m_geolocation = Geolocation::create(window() ? window()->document() : nullptr);
     return m_geolocation.get();
 }
 
index 3bab48d..81d8bbd 100644 (file)
@@ -716,14 +716,8 @@ DOMApplicationCache* DOMWindow::applicationCache()
 
 Navigator* DOMWindow::navigator()
 {
-    // FIXME: This should not return nullptr when frameless.
-    if (!isCurrentlyDisplayedInFrame())
-        return nullptr;
-
-    if (!m_navigator) {
-        ASSERT(scriptExecutionContext());
-        m_navigator = Navigator::create(*scriptExecutionContext(), *this);
-    }
+    if (!m_navigator)
+        m_navigator = Navigator::create(scriptExecutionContext(), *this);
 
     return m_navigator.get();
 }
index 08dbd8b..ba194f5 100644 (file)
@@ -49,7 +49,7 @@
 namespace WebCore {
 using namespace WTF;
 
-Navigator::Navigator(ScriptExecutionContext& context, DOMWindow& window)
+Navigator::Navigator(ScriptExecutionContext* context, DOMWindow& window)
     : NavigatorBase(context)
     , DOMWindowProperty(&window)
 {
index dd4d3c1..1e0a8af 100644 (file)
@@ -33,7 +33,7 @@ class DOMPluginArray;
 
 class Navigator final : public NavigatorBase, public ScriptWrappable, public DOMWindowProperty, public Supplementable<Navigator> {
 public:
-    static Ref<Navigator> create(ScriptExecutionContext& context, DOMWindow& window) { return adoptRef(*new Navigator(context, window)); }
+    static Ref<Navigator> create(ScriptExecutionContext* context, DOMWindow& window) { return adoptRef(*new Navigator(context, window)); }
     virtual ~Navigator();
 
     String appVersion() const;
@@ -53,7 +53,7 @@ public:
     void getStorageUpdates();
 
 private:
-    explicit Navigator(ScriptExecutionContext&, DOMWindow&);
+    explicit Navigator(ScriptExecutionContext*, DOMWindow&);
 
     mutable RefPtr<DOMPluginArray> m_plugins;
     mutable RefPtr<DOMMimeTypeArray> m_mimeTypes;
index 40e2ebd..afbf85c 100644 (file)
@@ -75,7 +75,7 @@
 
 namespace WebCore {
 
-NavigatorBase::NavigatorBase(ScriptExecutionContext& context)
+NavigatorBase::NavigatorBase(ScriptExecutionContext* context)
 #if ENABLE(SERVICE_WORKER)
     : m_serviceWorkerContainer(makeUniqueRef<ServiceWorkerContainer>(context, *this))
 #endif
index 7015c38..d42b393 100644 (file)
@@ -57,7 +57,7 @@ public:
     static Vector<String> languages();
 
 protected:
-    explicit NavigatorBase(ScriptExecutionContext&);
+    explicit NavigatorBase(ScriptExecutionContext*);
 
 #if ENABLE(SERVICE_WORKER)
 public:
index b4964c5..55de004 100644 (file)
@@ -30,7 +30,7 @@
 namespace WebCore {
 
 WorkerNavigator::WorkerNavigator(ScriptExecutionContext& context, const String& userAgent, bool isOnline)
-    : NavigatorBase(context)
+    : NavigatorBase(&context)
     , m_userAgent(userAgent)
     , m_isOnline(isOnline)
 {
index 40d40da..3b3a966 100644 (file)
@@ -57,8 +57,8 @@
 
 namespace WebCore {
 
-ServiceWorkerContainer::ServiceWorkerContainer(ScriptExecutionContext& context, NavigatorBase& navigator)
-    : ActiveDOMObject(&context)
+ServiceWorkerContainer::ServiceWorkerContainer(ScriptExecutionContext* context, NavigatorBase& navigator)
+    : ActiveDOMObject(context)
     , m_navigator(navigator)
 {
     suspendIfNeeded();
index 7d1476c..9b77454 100644 (file)
@@ -52,7 +52,7 @@ class ServiceWorkerContainer final : public EventTargetWithInlineData, public Ac
     WTF_MAKE_NONCOPYABLE(ServiceWorkerContainer);
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    ServiceWorkerContainer(ScriptExecutionContext&, NavigatorBase&);
+    ServiceWorkerContainer(ScriptExecutionContext*, NavigatorBase&);
     ~ServiceWorkerContainer();
 
     ServiceWorker* controller() const;