Changeset 220931 in webkit

Timestamp:

Aug 18, 2017 1:53:33 PM (7 years ago)

Author:

Chris Dumez

Message:

REGRESSION (r220601): Crash when closing google doc after switching the order of tabs in safari
​https://bugs.webkit.org/show_bug.cgi?id=175721
<rdar://problem/33928369>

Reviewed by Geoffrey Garen.

Make sure WebProcess::markAllLayersVolatile() does not call WTFMove() multiple times
on the same completion handler. Use a RefCounter to hold on to the completion handler
and make sure the handler gets called when the RefCounter's value becomes 0.

• WebProcess/WebProcess.cpp:

(WebKit::WebProcess::cancelPrepareToSuspend):
(WebKit::WebProcess::markAllLayersVolatile):
(WebKit::WebProcess::cancelMarkAllLayersVolatile):

• WebProcess/WebProcess.h:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• WebProcess/WebProcess.cpp (modified) (2 diffs)
• WebProcess/WebProcess.h (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2017-08-18  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (r220601): Crash when closing google doc after switching the order of tabs in safari
+        https://bugs.webkit.org/show_bug.cgi?id=175721
+        <rdar://problem/33928369>
+
+        Reviewed by Geoffrey Garen.
+
+        Make sure WebProcess::markAllLayersVolatile() does not call WTFMove() multiple times
+        on the same completion handler. Use a RefCounter to hold on to the completion handler
+        and make sure the handler gets called when the RefCounter's value becomes 0.
+
+        * WebProcess/WebProcess.cpp:
+        (WebKit::WebProcess::cancelPrepareToSuspend):
+        (WebKit::WebProcess::markAllLayersVolatile):
+        (WebKit::WebProcess::cancelMarkAllLayersVolatile):
+        * WebProcess/WebProcess.h:
+
 2017-08-18  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebKit/WebProcess/WebProcess.cpp

@@ -1373 +1373 @@
     // If we've already finished cleaning up and sent ProcessReadyToSuspend, we
     // shouldn't send DidCancelProcessSuspension; the UI process strictly expects one or the other.
-    if (!m_pagesMarkingLayersAsVolatile)
+    if (!m_pageMarkingLayersAsVolatileCounter)
         return;
 
@@ -1385 +1385 @@
 {
     RELEASE_LOG(ProcessSuspension, "%p - WebProcess::markAllLayersVolatile()", this);
-    m_pagesMarkingLayersAsVolatile = m_pageMap.size();
-    if (!m_pagesMarkingLayersAsVolatile) {
+    ASSERT(!m_pageMarkingLayersAsVolatileCounter);
+    m_pageMarkingLayersAsVolatileCounter = std::make_unique<PageMarkingLayersAsVolatileCounter>([this, completionHandler = WTFMove(completionHandler)] (RefCounterEvent) {
+        if (m_pageMarkingLayersAsVolatileCounter->value())
+            return;
+
         completionHandler();
-        return;
-    }
-    for (auto& page : m_pageMap.values()) {
-        page->markLayersVolatile([this, completionHandler = WTFMove(completionHandler)] {
-            ASSERT(m_pagesMarkingLayersAsVolatile);
-            if (!--m_pagesMarkingLayersAsVolatile)
-                completionHandler();
-        });
-    }
+        m_pageMarkingLayersAsVolatileCounter = nullptr;
+    });
+    auto token = m_pageMarkingLayersAsVolatileCounter->count();
+    for (auto& page : m_pageMap.values())
+        page->markLayersVolatile([token] { });
 }
 
 void WebProcess::cancelMarkAllLayersVolatile()
 {
-    if (!m_pagesMarkingLayersAsVolatile)
-        return;
-
+    if (!m_pageMarkingLayersAsVolatileCounter)
+        return;
+
+    m_pageMarkingLayersAsVolatileCounter = nullptr;
     for (auto& page : m_pageMap.values())
         page->cancelMarkLayersVolatile();
-    m_pagesMarkingLayersAsVolatile = 0;
 }
 

trunk/Source/WebKit/WebProcess/WebProcess.h

@@ -41 +41 @@
 #include <wtf/HashMap.h>
 #include <wtf/HashSet.h>
+#include <wtf/RefCounter.h>
 #include <wtf/text/AtomicString.h>
 #include <wtf/text/AtomicStringHash.h>
@@ -415 +416 @@
 #endif
 
-    unsigned m_pagesMarkingLayersAsVolatile { 0 };
+    enum PageMarkingLayersAsVolatileCounterType { };
+    using PageMarkingLayersAsVolatileCounter = RefCounter<PageMarkingLayersAsVolatileCounterType>;
+    std::unique_ptr<PageMarkingLayersAsVolatileCounter> m_pageMarkingLayersAsVolatileCounter;
+
     bool m_suppressMemoryPressureHandler { false };
 #if PLATFORM(MAC)