Changeset 245930 in webkit

Timestamp:

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

Author:

Alan Coon

Message:

Apply patch. rdar://problem/50857668

Location:

branches/safari-607-branch/Source/WebKit

Files:

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

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

@@ +1 @@
+2019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
+
+        Apply patch. rdar://problem/50857668
+
+    2019-05-30  Chris Dumez  <cdumez@apple.com>
+
+            TestWebKitAPI.ProcessSwap.UseSessionCookiesAfterProcessSwapInNonDefaultPersistentSession is failing on safari-607-branch
+            <rdar://problem/49980530>
+
+            Reviewed by Brady Eidson.
+
+            On the safari-607-branch banch, we need to send a WebProcess::AddWebsiteDataStore() IPC to the WebContent
+            process before it does any load for a non-default data store. If we fail to do so then the WebContent
+            process will fall back to using the default session for the loads.
+
+            This is what was happening for this API test. The cookie was getting set for session 2 but then it
+            would later try to read the cookie from session 1 (default one) after the process swap, and would
+            thus fail to get the cookie it expected.
+
+            The issue was that we would send the WebProcess::AddWebsiteDataStore() IPC too late, when committing
+            the provisional process (in WebPageProxy::swapToWebProcess()). By this point, the provisional load
+            would have already started in the provisional process and used the wrong session. To address the issue,
+            we now send the WebProcess::AddWebsiteDataStore() IPC as soon as we create the ProvisionalPageProxy.
+
+            * UIProcess/WebProcessProxy.cpp:
+            (WebKit::WebProcessProxy::sendPageDataStore):
+            (WebKit::WebProcessProxy::addProvisionalPageProxy):
+            (WebKit::WebProcessProxy::removeProvisionalPageProxy):
+            (WebKit::WebProcessProxy::addExistingWebPage):
+            (WebKit::WebProcessProxy::removeWebPage):
+            (WebKit::WebProcessProxy::destroyDataStoreIfUnused):
+            (WebKit::WebProcessProxy::hasPageUsingSession const):
+            * UIProcess/WebProcessProxy.h:
+            (WebKit::WebProcessProxy::addProvisionalPageProxy): Deleted.
+            (WebKit::WebProcessProxy::removeProvisionalPageProxy): Deleted.
+
 2019-05-30  Kocsen Chung  <kocsen_chung@apple.com>
 

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

@@ -457 +457 @@
 }
 
-void WebProcessProxy::addExistingWebPage(WebPageProxy& webPage, uint64_t pageID, BeginsUsingDataStore beginsUsingDataStore)
-{
-    ASSERT(!m_pageMap.contains(pageID));
-    ASSERT(!globalPageMap().contains(pageID));
-    ASSERT(!m_isInProcessCache);
-
-    if (beginsUsingDataStore == BeginsUsingDataStore::Yes)
-        m_processPool->pageBeginUsingWebsiteDataStore(webPage);
-
+void WebProcessProxy::sendPageDataStore(WebPageProxy& webPage)
+{
     auto sessionID = webPage.sessionID();
     if (sessionID.isEphemeral())
@@ -471 +464 @@
     else if (sessionID != PAL::SessionID::defaultSessionID())
         send(Messages::WebProcess::AddWebsiteDataStore(webPage.websiteDataStore().parameters()), 0);
+}
+
+void WebProcessProxy::addProvisionalPageProxy(ProvisionalPageProxy& provisionalPage)
+{
+    ASSERT(!m_provisionalPages.contains(&provisionalPage));
+    m_provisionalPages.add(&provisionalPage);
+
+    sendPageDataStore(provisionalPage.page());
+}
+
+void WebProcessProxy::removeProvisionalPageProxy(ProvisionalPageProxy& provisionalPage)
+{
+    ASSERT(m_provisionalPages.contains(&provisionalPage));
+    m_provisionalPages.remove(&provisionalPage);
+
+    destroyDataStoreIfUnused(provisionalPage.page().sessionID());
+}
+
+void WebProcessProxy::addExistingWebPage(WebPageProxy& webPage, uint64_t pageID, BeginsUsingDataStore beginsUsingDataStore)
+{
+    ASSERT(!m_pageMap.contains(pageID));
+    ASSERT(!globalPageMap().contains(pageID));
+    ASSERT(!m_isInProcessCache);
+
+    if (beginsUsingDataStore == BeginsUsingDataStore::Yes)
+        m_processPool->pageBeginUsingWebsiteDataStore(webPage);
+
+    sendPageDataStore(webPage);
 
     m_pageMap.set(pageID, &webPage);
@@ -499 +520 @@
         m_processPool->pageEndUsingWebsiteDataStore(webPage);
 
-    auto sessionID = webPage.sessionID();
+    destroyDataStoreIfUnused(webPage.sessionID());
+
+    updateBackgroundResponsivenessTimer();
+
+    maybeShutDown();
+}
+
+void WebProcessProxy::destroyDataStoreIfUnused(PAL::SessionID sessionID)
+{
     if (sessionID != PAL::SessionID::defaultSessionID() && !hasPageUsingSession(sessionID))
         send(Messages::WebProcess::DestroySession(sessionID), 0);
-
-    updateBackgroundResponsivenessTimer();
-
-    maybeShutDown();
 }
 
@@ -512 +537 @@
     for (auto& page : m_pageMap.values()) {
         if (page->sessionID() == sessionID)
+            return true;
+    }
+    for (auto* provisionalPage : m_provisionalPages) {
+        if (provisionalPage->page().sessionID() == sessionID)
             return true;
     }

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

@@ -133 +133 @@
     void removeWebPage(WebPageProxy&, uint64_t pageID, EndsUsingDataStore);
 
-    void addProvisionalPageProxy(ProvisionalPageProxy& provisionalPage) { ASSERT(!m_provisionalPages.contains(&provisionalPage)); m_provisionalPages.add(&provisionalPage); }
-    void removeProvisionalPageProxy(ProvisionalPageProxy& provisionalPage) { ASSERT(m_provisionalPages.contains(&provisionalPage)); m_provisionalPages.remove(&provisionalPage); }
+    void addProvisionalPageProxy(ProvisionalPageProxy&);
+    void removeProvisionalPageProxy(ProvisionalPageProxy&);
 
     typename WebPageProxyMap::ValuesConstIteratorRange pages() const { return m_pageMap.values(); }
@@ -283 +283 @@
     void didDestroyFrame(uint64_t);
     void didDestroyUserGestureToken(uint64_t);
+
+    void sendPageDataStore(WebPageProxy&);
+    void destroyDataStoreIfUnused(PAL::SessionID);
 
     bool canBeAddedToWebProcessCache() const;