Changeset 212698 in webkit

Timestamp:

Feb 20, 2017 9:02:32 PM (7 years ago)

Author:

commit-queue@webkit.org

Message:

ASSERTION FAILED: m_normalWorld->hasOneRef() under WorkerThread::stop
​https://bugs.webkit.org/show_bug.cgi?id=168356
<rdar://problem/30592486>

Patch by Joseph Pecoraro <Joseph Pecoraro> on 2017-02-20
Reviewed by Ryosuke Niwa.

Source/WebCore:

• page/DOMWindow.cpp:

(WebCore::DOMWindow::removeAllEventListeners):
Remove Performance object EventListeners.

• workers/WorkerGlobalScope.cpp:

(WebCore::WorkerGlobalScope::~WorkerGlobalScope):
(WebCore::WorkerGlobalScope::removeAllEventListeners):
(WebCore::WorkerGlobalScope::performance):

• workers/WorkerGlobalScope.h:

Remove Performance object EventListeners.
Also clear Performance early in destruction since its ContextDestructionObserver
destruction makes checks about the WorkerThread.

LayoutTests:

• TestExpectations:

Unskip tests now that they no longer trigger assertions.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/DOMWindow.cpp (modified) (1 diff)
• Source/WebCore/workers/WorkerGlobalScope.cpp (modified) (2 diffs)
• Source/WebCore/workers/WorkerGlobalScope.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-02-20  Joseph Pecoraro  <pecoraro@apple.com>
+
+        ASSERTION FAILED: m_normalWorld->hasOneRef() under WorkerThread::stop
+        https://bugs.webkit.org/show_bug.cgi?id=168356
+        <rdar://problem/30592486>
+
+        Reviewed by Ryosuke Niwa.
+
+        * TestExpectations:
+        Unskip tests now that they no longer trigger assertions.
+
 2017-02-20  Jer Noble  <jer.noble@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -311 +311 @@
 imported/w3c/web-platform-tests/XMLHttpRequest/responsexml-document-properties.htm [ Failure ]
 
-# Failing with dynamic URLs.
+webkit.org/b/161088 imported/w3c/web-platform-tests/resource-timing/resource-timing.html [ Pass Failure ]
 webkit.org/b/168357 imported/w3c/web-platform-tests/resource-timing/rt-initiatorType-media.html [ Failure ]
-
 webkit.org/b/168448 imported/w3c/web-platform-tests/resource-timing/rt-shared-resource-in-frames.html [ Pass Failure ]
 webkit.org/b/168448 imported/w3c/web-platform-tests/resource-timing/rt-shared-resource-in-workers.html [ Pass Failure ]
@@ -980 +979 @@
 webkit.org/b/160017 js/regress-139548.html [ Slow ]
 
-webkit.org/b/161088 imported/w3c/web-platform-tests/resource-timing/resource-timing.html [ Pass Failure ]
-
-webkit.org/b/168356 [ Debug ] imported/w3c/web-platform-tests/resource-timing [ Skip ]
-
 # Only iOS has implemented lettepress.
 fast/text/letterpress-different.html [ ImageOnlyFailure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-02-20  Joseph Pecoraro  <pecoraro@apple.com>
+
+        ASSERTION FAILED: m_normalWorld->hasOneRef() under WorkerThread::stop
+        https://bugs.webkit.org/show_bug.cgi?id=168356
+        <rdar://problem/30592486>
+
+        Reviewed by Ryosuke Niwa.
+
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::removeAllEventListeners):
+        Remove Performance object EventListeners.
+
+        * workers/WorkerGlobalScope.cpp:
+        (WebCore::WorkerGlobalScope::~WorkerGlobalScope):
+        (WebCore::WorkerGlobalScope::removeAllEventListeners):
+        (WebCore::WorkerGlobalScope::performance):
+        * workers/WorkerGlobalScope.h:
+        Remove Performance object EventListeners.
+        Also clear Performance early in destruction since its ContextDestructionObserver
+        destruction makes checks about the WorkerThread.
+
 2017-02-20  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebCore/page/DOMWindow.cpp

@@ -2028 +2028 @@
 #endif
 
+#if ENABLE(WEB_TIMING)
+    if (m_performance)
+        m_performance->removeAllEventListeners();
+#endif
+
     removeAllUnloadEventListeners(this);
     removeAllBeforeUnloadEventListeners(this);

trunk/Source/WebCore/workers/WorkerGlobalScope.cpp

@@ -98 +98 @@
     ASSERT(currentThread() == thread().threadID());
 
+#if ENABLE(WEB_TIMING)
+    m_performance = nullptr;
+#endif
+
     // Notify proxy that we are going away. This can free the WorkerThread object, so do not access it after this.
     thread().workerReportingProxy().workerGlobalScopeDestroyed();
+}
+
+void WorkerGlobalScope::removeAllEventListeners()
+{
+    EventTarget::removeAllEventListeners();
+
+#if ENABLE(WEB_TIMING)
+    m_performance->removeAllEventListeners();
+#endif
 }
 
@@ -359 +372 @@
 Performance& WorkerGlobalScope::performance() const
 {
-    return m_performance;
+    return *m_performance;
 }
 

trunk/Source/WebCore/workers/WorkerGlobalScope.h

@@ -105 +105 @@
 #endif
 
+    void removeAllEventListeners() final;
+
 protected:
     WorkerGlobalScope(const URL&, const String& identifier, const String& userAgent, WorkerThread&, bool shouldBypassMainWorldContentSecurityPolicy, Ref<SecurityOrigin>&& topOrigin, MonotonicTime timeOrigin, IDBClient::IDBConnectionProxy*, SocketProvider*);
@@ -178 +180 @@
 
 #if ENABLE(WEB_TIMING)
-    Ref<Performance> m_performance;
+    RefPtr<Performance> m_performance;
 #endif