Changeset 245931 in webkit

Timestamp:

May 30, 2019, 5:30:30 PM (7 years ago)

Author:

Alan Coon

Message:

Apply patch. rdar://problem/51264847

Location:

branches/safari-607-branch/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/WebProcessCache.cpp (modified) (2 diffs)
• UIProcess/WebProcessProxy.cpp (modified) (1 diff)

branches/safari-607-branch/Source/WebKit/ChangeLog

@@ +1 @@
+2019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
+
+        Apply patch. rdar://problem/51264847
+
+    2019-05-30  Chris Dumez  <cdumez@apple.com>
+
+            <rdar://problem/50435250> v2: CrashTracer: Crash in WebProcessCache::removeProcess
+
+            Reviewed by Geoff Garen.
+
+            * UIProcess/WebProcessCache.cpp:
+            (WebKit::WebProcessCache::removeProcess):
+            This code change is to avoid crashing if my speculative fix below does not work. If
+            WebProcessCache::removeProcess() gets called with a process whole registrable domain
+            is empty (or null), remove it the slow way from the HashMap instead of trying to look
+            up the bad registrable domain in the HashMap. This ensures we do no crash but still
+            remove the process we're supposed to evict.
+
+            * UIProcess/WebProcessProxy.cpp:
+            (WebKit::WebProcessProxy::canTerminateChildProcess):
+            This is a speculative fix for the crash which aligns the branch with trunk. There is
+            a period of time where a SuspendedPageProxy can exist for a given process but the
+            WebProcessPool is no longer aware of it. This is when we're about to use the
+            SuspendedPageProxy for a load. If WebProcessProxy::maybeShutDown() gets called during
+            this period of time, canTerminateChildProcess() would incorrectly return true and we
+            would cache the WebProcess even though it is about to do a load. The new code relies
+            on m_suspendedPageCount which is a lot more reliable than asking the process pool.
+
 2019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
 

branches/safari-607-branch/Source/WebKit/UIProcess/WebProcessCache.cpp

@@ -192 +192 @@
 
     std::unique_ptr<CachedProcess> cachedProcess;
-    auto it = m_processesPerRegistrableDomain.find(process.registrableDomain());
-    if (it != m_processesPerRegistrableDomain.end() && &it->value->process() == &process) {
-        cachedProcess = WTFMove(it->value);
-        m_processesPerRegistrableDomain.remove(it);
+    auto registrableDomain = process.registrableDomain();
+    ASSERT(!registrableDomain.isEmpty());
+    if (registrableDomain.isEmpty()) {
+        for (auto it = m_processesPerRegistrableDomain.begin(); it != m_processesPerRegistrableDomain.end(); ++it) {
+            if (&it->value->process() == &process) {
+                cachedProcess = WTFMove(it->value);
+                m_processesPerRegistrableDomain.remove(it);
+                break;
+            }
+        }
     } else {
+        auto it = m_processesPerRegistrableDomain.find(registrableDomain);
+        if (it != m_processesPerRegistrableDomain.end() && &it->value->process() == &process) {
+            cachedProcess = WTFMove(it->value);
+            m_processesPerRegistrableDomain.remove(it);
+        }
+    }
+
+    if (!cachedProcess) {
         for (auto& pair : m_pendingAddRequests) {
             if (&pair.value->process() == &process) {
@@ -205 +219 @@
         }
     }
+
     ASSERT(cachedProcess);
     if (!cachedProcess)

branches/safari-607-branch/Source/WebKit/UIProcess/WebProcessProxy.cpp

@@ -1004 +1004 @@
 bool WebProcessProxy::canTerminateChildProcess()
 {
-    if (!m_pageMap.isEmpty() || m_processPool->hasSuspendedPageFor(*this) || !m_provisionalPages.isEmpty() || m_isInProcessCache)
+    if (!m_pageMap.isEmpty() || m_suspendedPageCount || !m_provisionalPages.isEmpty() || m_isInProcessCache)
         return false;