window.performance 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 23:43:10 +0000 (23:43 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Oct 2018 23:43:10 +0000 (23:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190636

Reviewed by Ryosuke Niwa.

Source/WebCore:

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

Also note that the attribute is not nullable in the specification:
- https://www.w3.org/TR/navigation-timing/#sec-window.performance-attribute

No new tests, updated existing test.

* page/DOMWindow.cpp:
(WebCore::DOMWindow::performance const):
* page/Performance.cpp:
(WebCore::Performance::Performance):
* page/Performance.h:
* platform/GenericTaskQueue.h:
(WebCore::TaskDispatcher::TaskDispatcher):
(WebCore::TaskDispatcher::postTask):
(WebCore::GenericTaskQueue::GenericTaskQueue):
(WebCore::GenericTaskQueue::isClosed const):
* workers/WorkerGlobalScope.cpp:
(WebCore::WorkerGlobalScope::WorkerGlobalScope):

LayoutTests:

Extend layout test coverage.

* 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@237209 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
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
Source/WebCore/ChangeLog
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/Performance.cpp
Source/WebCore/page/Performance.h
Source/WebCore/platform/GenericTaskQueue.h
Source/WebCore/workers/WorkerGlobalScope.cpp

index 9f6b5ab..a8ac537 100644 (file)
@@ -1,3 +1,17 @@
+2018-10-16  Chris Dumez  <cdumez@apple.com>
+
+        window.performance should not become null after the window loses its browsing context
+        https://bugs.webkit.org/show_bug.cgi?id=190636
+
+        Reviewed by Ryosuke Niwa.
+
+        Extend layout test coverage.
+
+        * 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  Timothy Hatcher  <timothy@apple.com>
 
         Add <meta name="supported-color-schemes"> to control what color schemes the page supports
index 52d5bf8..1d4a1e6 100644 (file)
@@ -30,6 +30,7 @@ PASS w.applicationCache threw exception SecurityError: Blocked a frame with orig
 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.performance 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..
 
@@ -60,6 +61,7 @@ PASS w.applicationCache threw exception SecurityError: Blocked a frame with orig
 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.performance 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 43bb6f1..fe12191 100644 (file)
@@ -45,6 +45,7 @@ function validateWindow(_w)
     shouldThrowErrorName("w.visualViewport", "SecurityError");
     shouldThrowErrorName("w.styleMedia", "SecurityError");
     shouldThrowErrorName("w.navigator", "SecurityError");
+    shouldThrowErrorName("w.performance", "SecurityError");
 
     shouldThrowErrorName("w.foo", "SecurityError");
     shouldThrowErrorName("w.location.foo", "SecurityError");
index 7116e3a..2ed5acc 100644 (file)
@@ -74,6 +74,7 @@ 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.performance is true
 PASS w.foo is undefined.
 PASS w.location.foo is undefined.
 
@@ -148,6 +149,7 @@ 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.performance is true
 PASS w.foo is undefined.
 PASS w.location.foo is undefined.
 PASS successfullyParsed is true
index b167cbe..d535cce 100644 (file)
@@ -137,6 +137,12 @@ function validateWindow(_w)
     }
 
     try {
+    shouldBeTrue("!!w.performance");
+    } catch (e) {
+        debug(e);
+    }
+
+    try {
     shouldBeUndefined("w.foo");
     shouldBeUndefined("w.location.foo");
     } catch (e) {
index 54b97e8..d0f75cc 100644 (file)
@@ -1,3 +1,31 @@
+2018-10-16  Chris Dumez  <cdumez@apple.com>
+
+        window.performance should not become null after the window loses its browsing context
+        https://bugs.webkit.org/show_bug.cgi?id=190636
+
+        Reviewed by Ryosuke Niwa.
+
+        window.performance should not become null after the window loses its browsing context. This
+        WebKit behavior does not match the HTML specification nor the behavior of other browsers.
+
+        Also note that the attribute is not nullable in the specification:
+        - https://www.w3.org/TR/navigation-timing/#sec-window.performance-attribute
+
+        No new tests, updated existing test.
+
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::performance const):
+        * page/Performance.cpp:
+        (WebCore::Performance::Performance):
+        * page/Performance.h:
+        * platform/GenericTaskQueue.h:
+        (WebCore::TaskDispatcher::TaskDispatcher):
+        (WebCore::TaskDispatcher::postTask):
+        (WebCore::GenericTaskQueue::GenericTaskQueue):
+        (WebCore::GenericTaskQueue::isClosed const):
+        * workers/WorkerGlobalScope.cpp:
+        (WebCore::WorkerGlobalScope::WorkerGlobalScope):
+
 2018-10-16  Jer Noble  <jer.noble@apple.com>
 
         Refactoring: Convert HTMLMediaElement::scheduleDelayedAction() to individually schedulable & cancelable tasks
index 81d8bbd..ec926b7 100644 (file)
@@ -724,13 +724,9 @@ Navigator* DOMWindow::navigator()
 
 Performance* DOMWindow::performance() const
 {
-    // FIXME: This should not return nullptr when frameless.
-    if (!isCurrentlyDisplayedInFrame())
-        return nullptr;
-
     if (!m_performance) {
-        MonotonicTime timeOrigin = document()->loader() ? document()->loader()->timing().referenceMonotonicTime() : MonotonicTime::now();
-        m_performance = Performance::create(*document(), timeOrigin);
+        MonotonicTime timeOrigin = document() && document()->loader() ? document()->loader()->timing().referenceMonotonicTime() : MonotonicTime::now();
+        m_performance = Performance::create(document(), timeOrigin);
     }
     return m_performance.get();
 }
index 4215d16..80e9dae 100644 (file)
 
 namespace WebCore {
 
-Performance::Performance(ScriptExecutionContext& context, MonotonicTime timeOrigin)
-    : ContextDestructionObserver(&context)
+Performance::Performance(ScriptExecutionContext* context, MonotonicTime timeOrigin)
+    : ContextDestructionObserver(context)
     , m_resourceTimingBufferFullTimer(*this, &Performance::resourceTimingBufferFullTimerFired)
     , m_timeOrigin(timeOrigin)
     , m_performanceTimelineTaskQueue(context)
 {
     ASSERT(m_timeOrigin);
+    ASSERT(context || m_performanceTimelineTaskQueue.isClosed());
 }
 
 Performance::~Performance() = default;
index 24ab94b..a62cb2e 100644 (file)
@@ -54,7 +54,7 @@ class UserTiming;
 
 class Performance final : public RefCounted<Performance>, public ContextDestructionObserver, public EventTargetWithInlineData {
 public:
-    static Ref<Performance> create(ScriptExecutionContext& context, MonotonicTime timeOrigin) { return adoptRef(*new Performance(context, timeOrigin)); }
+    static Ref<Performance> create(ScriptExecutionContext* context, MonotonicTime timeOrigin) { return adoptRef(*new Performance(context, timeOrigin)); }
     ~Performance();
 
     DOMHighResTimeStamp now() const;
@@ -91,7 +91,7 @@ public:
     using RefCounted::deref;
 
 private:
-    Performance(ScriptExecutionContext&, MonotonicTime timeOrigin);
+    Performance(ScriptExecutionContext*, MonotonicTime timeOrigin);
 
     void contextDestroyed() override;
 
index aaa9450..c6b55ea 100644 (file)
@@ -35,18 +35,19 @@ namespace WebCore {
 template <typename T>
 class TaskDispatcher {
 public:
-    TaskDispatcher(T& context)
+    TaskDispatcher(T* context)
         : m_context(context)
     {
     }
 
     void postTask(WTF::Function<void()>&& f)
     {
-        m_context.postTask(WTFMove(f));
+        ASSERT(m_context);
+        m_context->postTask(WTFMove(f));
     }
 
 private:
-    T& m_context;
+    T* m_context;
 };
 
 template<>
@@ -74,7 +75,13 @@ public:
     }
 
     GenericTaskQueue(T& t)
+        : m_dispatcher(&t)
+    {
+    }
+
+    GenericTaskQueue(T* t)
         : m_dispatcher(t)
+        , m_isClosed(!t)
     {
     }
 
@@ -106,7 +113,9 @@ public:
         CanMakeWeakPtr<GenericTaskQueue<T>>::weakPtrFactory().revokeAll();
         m_pendingTasks = 0;
     }
+
     bool hasPendingTasks() const { return m_pendingTasks; }
+    bool isClosed() const { return m_isClosed; }
 
 private:
     TaskDispatcher<T> m_dispatcher;
index 15c6701..ae0546e 100644 (file)
@@ -70,7 +70,7 @@ WorkerGlobalScope::WorkerGlobalScope(const URL& url, Ref<SecurityOrigin>&& origi
     , m_connectionProxy(connectionProxy)
 #endif
     , m_socketProvider(socketProvider)
-    , m_performance(Performance::create(*this, timeOrigin))
+    , m_performance(Performance::create(this, timeOrigin))
     , m_sessionID(sessionID)
 {
 #if !ENABLE(INDEXED_DATABASE)