Diff
Modified: trunk/Source/WebKit/ChangeLog (238827 => 238828)
--- trunk/Source/WebKit/ChangeLog 2018-12-04 00:08:37 UTC (rev 238827)
+++ trunk/Source/WebKit/ChangeLog 2018-12-04 00:08:42 UTC (rev 238828)
@@ -1,5 +1,36 @@
2018-12-03 Chris Dumez <[email protected]>
+ Regression(PSON) Google OAuth is broken in private sessions
+ https://bugs.webkit.org/show_bug.cgi?id=192337
+ <rdar://problem/46353558>
+
+ Reviewed by Alex Christensen.
+
+ In WebPageProxy::swapToWebProcess(), we would call removeWebPage() on the old WebProcessProxy and then
+ addExistingWebPage() on the new WebProcessProxy, as you would expect in case of process swap.
+
+ The issue is that WebProcessProxy::removeWebPage() calls WebProcessPool::pageEndUsingWebsiteDataStore()
+ which would cause the session to get destroyed assuming this was the last page using it. We would
+ therefore lose session cookies after a process-swap in private session.
+
+ To address the issue, a parameter to WebProcessPool::pageEndUsingWebsiteDataStore() and
+ WebProcessPool::pageBeginUsingWebsiteDataStore() to control if we want to tell the WebProcessPool
+ about the page beginning / ending its use of the session. In the case of a process-swap, we make
+ sure the process pool is not notified.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::reattachToWebProcess):
+ (WebKit::WebPageProxy::swapToWebProcess):
+ (WebKit::WebPageProxy::finishAttachingToWebProcess):
+ (WebKit::WebPageProxy::close):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::createWebPage):
+ (WebKit::WebProcessProxy::addExistingWebPage):
+ (WebKit::WebProcessProxy::removeWebPage):
+ * UIProcess/WebProcessProxy.h:
+
+2018-12-03 Chris Dumez <[email protected]>
+
[PSON] Request by the client to process-swap is ignored if the window has an opener
https://bugs.webkit.org/show_bug.cgi?id=192267
<rdar://problem/46386886>
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (238827 => 238828)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-12-04 00:08:37 UTC (rev 238827)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-12-04 00:08:42 UTC (rev 238828)
@@ -727,7 +727,7 @@
ASSERT(!isValid());
RELEASE_LOG_IF_ALLOWED(Process, "%p WebPageProxy::reattachToWebProcess\n", this);
- m_process->removeWebPage(*this, m_pageID);
+ m_process->removeWebPage(*this, m_pageID, WebProcessProxy::EndsUsingDataStore::Yes);
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
auto& processPool = m_process->processPool();
@@ -734,6 +734,9 @@
m_process = processPool.createNewWebProcessRespectingProcessCountLimit(m_websiteDataStore.get());
m_isValid = true;
+ m_process->addExistingWebPage(*this, m_pageID, WebProcessProxy::BeginsUsingDataStore::Yes);
+ m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID, *this);
+
finishAttachingToWebProcess();
}
@@ -779,7 +782,7 @@
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
bool didSuspendPreviousPage = suspendCurrentPageIfPossible(navigation, mainFrameIDInPreviousProcess, processSwapRequestedByClient);
- m_process->removeWebPage(*this, m_pageID);
+ m_process->removeWebPage(*this, m_pageID, WebProcessProxy::EndsUsingDataStore::No);
m_process = WTFMove(process);
m_isValid = true;
@@ -795,6 +798,9 @@
m_process->frameCreated(destinationSuspendedPage->mainFrameID(), *m_mainFrame);
}
+ m_process->addExistingWebPage(*this, m_pageID, WebProcessProxy::BeginsUsingDataStore::No);
+ m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID, *this);
+
finishAttachingToWebProcess(didSuspendPreviousPage ? ShouldDelayAttachingDrawingArea::Yes : ShouldDelayAttachingDrawingArea::No);
completionHandler(didSuspendPreviousPage);
};
@@ -814,9 +820,6 @@
processDidFinishLaunching();
}
- m_process->addExistingWebPage(*this, m_pageID);
- m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID, *this);
-
updateActivityState();
updateThrottleState();
@@ -996,7 +999,7 @@
m_process->processPool().removeAllSuspendedPagesForPage(*this);
m_process->send(Messages::WebPage::Close(), m_pageID);
- m_process->removeWebPage(*this, m_pageID);
+ m_process->removeWebPage(*this, m_pageID, WebProcessProxy::EndsUsingDataStore::Yes);
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_pageID);
m_process->processPool().supplement<WebNotificationManagerProxy>()->clearNotifications(this);
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (238827 => 238828)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2018-12-04 00:08:37 UTC (rev 238827)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2018-12-04 00:08:42 UTC (rev 238828)
@@ -428,17 +428,18 @@
uint64_t pageID = generatePageID();
Ref<WebPageProxy> webPage = WebPageProxy::create(pageClient, *this, pageID, WTFMove(pageConfiguration));
- addExistingWebPage(webPage.get(), pageID);
+ addExistingWebPage(webPage.get(), pageID, BeginsUsingDataStore::Yes);
return webPage;
}
-void WebProcessProxy::addExistingWebPage(WebPageProxy& webPage, uint64_t pageID)
+void WebProcessProxy::addExistingWebPage(WebPageProxy& webPage, uint64_t pageID, BeginsUsingDataStore beginsUsingDataStore)
{
ASSERT(!m_pageMap.contains(pageID));
ASSERT(!globalPageMap().contains(pageID));
- m_processPool->pageBeginUsingWebsiteDataStore(webPage);
+ if (beginsUsingDataStore == BeginsUsingDataStore::Yes)
+ m_processPool->pageBeginUsingWebsiteDataStore(webPage);
m_pageMap.set(pageID, &webPage);
globalPageMap().set(pageID, &webPage);
@@ -457,7 +458,7 @@
send(Messages::WebProcess::MarkIsNoLongerPrewarmed(), 0);
}
-void WebProcessProxy::removeWebPage(WebPageProxy& webPage, uint64_t pageID)
+void WebProcessProxy::removeWebPage(WebPageProxy& webPage, uint64_t pageID, EndsUsingDataStore endsUsingDataStore)
{
auto* removedPage = m_pageMap.take(pageID);
ASSERT_UNUSED(removedPage, removedPage == &webPage);
@@ -464,7 +465,8 @@
removedPage = globalPageMap().take(pageID);
ASSERT_UNUSED(removedPage, removedPage == &webPage);
- m_processPool->pageEndUsingWebsiteDataStore(webPage);
+ if (endsUsingDataStore == EndsUsingDataStore::Yes)
+ m_processPool->pageEndUsingWebsiteDataStore(webPage);
updateBackgroundResponsivenessTimer();
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (238827 => 238828)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2018-12-04 00:08:37 UTC (rev 238827)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2018-12-04 00:08:42 UTC (rev 238828)
@@ -117,9 +117,13 @@
static WebProcessProxy* processForIdentifier(WebCore::ProcessIdentifier);
static WebPageProxy* webPage(uint64_t pageID);
Ref<WebPageProxy> createWebPage(PageClient&, Ref<API::PageConfiguration>&&);
- void addExistingWebPage(WebPageProxy&, uint64_t pageID);
- void removeWebPage(WebPageProxy&, uint64_t pageID);
+ enum class BeginsUsingDataStore : bool { No, Yes };
+ void addExistingWebPage(WebPageProxy&, uint64_t pageID, BeginsUsingDataStore);
+
+ enum class EndsUsingDataStore : bool { No, Yes };
+ void removeWebPage(WebPageProxy&, uint64_t pageID, EndsUsingDataStore);
+
typename WebPageProxyMap::ValuesConstIteratorRange pages() const { return m_pageMap.values(); }
unsigned pageCount() const { return m_pageMap.size(); }
unsigned visiblePageCount() const { return m_visiblePageCounter.value(); }
Modified: trunk/Tools/ChangeLog (238827 => 238828)
--- trunk/Tools/ChangeLog 2018-12-04 00:08:37 UTC (rev 238827)
+++ trunk/Tools/ChangeLog 2018-12-04 00:08:42 UTC (rev 238828)
@@ -1,5 +1,20 @@
2018-12-03 Chris Dumez <[email protected]>
+ Regression(PSON) Google OAuth is broken in private sessions
+ https://bugs.webkit.org/show_bug.cgi?id=192337
+ <rdar://problem/46353558>
+
+ Reviewed by Alex Christensen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKitCocoa/GetSessionCookie.html: Added.
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+ * TestWebKitAPI/Tests/WebKitCocoa/SetSessionCookie.html: Added.
+
+2018-12-03 Chris Dumez <[email protected]>
+
[PSON] Request by the client to process-swap is ignored if the window has an opener
https://bugs.webkit.org/show_bug.cgi?id=192267
<rdar://problem/46386886>
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (238827 => 238828)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2018-12-04 00:08:37 UTC (rev 238827)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2018-12-04 00:08:42 UTC (rev 238828)
@@ -173,6 +173,8 @@
46397B951DC2C850009A78AE /* DOMNode.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46397B941DC2C850009A78AE /* DOMNode.mm */; };
4647B1261EBA3B850041D7EF /* ProcessDidTerminate.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4647B1251EBA3B730041D7EF /* ProcessDidTerminate.cpp */; };
466C3843210637DE006A88DE /* notify-resourceLoadObserver.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 466C3842210637CE006A88DE /* notify-resourceLoadObserver.html */; };
+ 467C565321B5ED130057516D /* GetSessionCookie.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 467C565121B5ECDF0057516D /* GetSessionCookie.html */; };
+ 467C565421B5ED130057516D /* SetSessionCookie.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 467C565221B5ECDF0057516D /* SetSessionCookie.html */; };
46A911592108E6780078D40D /* CustomUserAgent.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46A911582108E66B0078D40D /* CustomUserAgent.mm */; };
46AE5A3720F9066D00E0873E /* SimpleServiceWorkerRegistrations-3.sqlite3 in Copy Resources */ = {isa = PBXBuildFile; fileRef = 4656A75720F9054F0002E21F /* SimpleServiceWorkerRegistrations-3.sqlite3 */; };
46C519DA1D355AB200DAA51A /* LocalStorageNullEntries.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46C519D81D355A7300DAA51A /* LocalStorageNullEntries.mm */; };
@@ -1077,6 +1079,7 @@
26F52EB218288F240023D412 /* geolocationWatchPosition.html in Copy Resources */,
26F52EB318288F240023D412 /* geolocationWatchPositionWithHighAccuracy.html in Copy Resources */,
07E1F6A21FFC44FA0096C7EC /* getDisplayMedia.html in Copy Resources */,
+ 467C565321B5ED130057516D /* GetSessionCookie.html in Copy Resources */,
074994421EA5034B000DA44E /* getUserMedia.html in Copy Resources */,
F46A095B1ED8A6E600D4AA55 /* gif-and-file-input.html in Copy Resources */,
9B4F8FA7159D52DD002D9F94 /* HTMLCollectionNamedItem.html in Copy Resources */,
@@ -1202,6 +1205,7 @@
7A66BDB81EAF18D500CCC924 /* set-long-title.html in Copy Resources */,
CE6E81A420A933D500E2C80F /* set-timeout-function.html in Copy Resources */,
52B8CF9815868D9100281053 /* SetDocumentURI.html in Copy Resources */,
+ 467C565421B5ED130057516D /* SetSessionCookie.html in Copy Resources */,
CEBABD491B71687C0051210A /* should-open-external-schemes.html in Copy Resources */,
F4E3D80820F70BB9007B58C5 /* significant-text-milestone-article.html in Copy Resources */,
F4FC077720F013B600CA043C /* significant-text-milestone.html in Copy Resources */,
@@ -1492,6 +1496,8 @@
4647B1251EBA3B730041D7EF /* ProcessDidTerminate.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ProcessDidTerminate.cpp; sourceTree = "<group>"; };
4656A75720F9054F0002E21F /* SimpleServiceWorkerRegistrations-3.sqlite3 */ = {isa = PBXFileReference; lastKnownFileType = file; path = "SimpleServiceWorkerRegistrations-3.sqlite3"; sourceTree = "<group>"; };
466C3842210637CE006A88DE /* notify-resourceLoadObserver.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "notify-resourceLoadObserver.html"; sourceTree = "<group>"; };
+ 467C565121B5ECDF0057516D /* GetSessionCookie.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = GetSessionCookie.html; sourceTree = "<group>"; };
+ 467C565221B5ECDF0057516D /* SetSessionCookie.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = SetSessionCookie.html; sourceTree = "<group>"; };
46A911582108E66B0078D40D /* CustomUserAgent.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CustomUserAgent.mm; sourceTree = "<group>"; };
46C519D81D355A7300DAA51A /* LocalStorageNullEntries.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = LocalStorageNullEntries.mm; sourceTree = "<group>"; };
46C519E21D35629600DAA51A /* LocalStorageNullEntries.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = LocalStorageNullEntries.html; sourceTree = "<group>"; };
@@ -2791,6 +2797,7 @@
CDE195B21CFE0ADE0053D256 /* FullscreenTopContentInset.html */,
636353A61E9861940009F8AF /* GeolocationGetCurrentPositionResult.html */,
07E1F6A11FFC44F90096C7EC /* getDisplayMedia.html */,
+ 467C565121B5ECDF0057516D /* GetSessionCookie.html */,
F47D30ED1ED28A6C000482E1 /* gif-and-file-input.html */,
510477761D298E57009747EB /* IDBDeleteRecovery.html */,
5104776F1D298D85009747EB /* IDBDeleteRecovery.sqlite3 */,
@@ -2875,6 +2882,7 @@
2E92B8F6216490C3005B64F0 /* rich-text-attributes.html */,
F4E0A295211FC5A300AF7C7F /* selected-text-and-textarea.html */,
F4D65DA71F5E46C0009D8C27 /* selected-text-image-link-and-editable.html */,
+ 467C565221B5ECDF0057516D /* SetSessionCookie.html */,
F4E3D80720F708E4007B58C5 /* significant-text-milestone-article.html */,
F4FC077620F0108100CA043C /* significant-text-milestone.html */,
C9B4AD291ECA6EA500F5FEA0 /* silence-long.m4a */,
Added: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/GetSessionCookie.html (0 => 238828)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/GetSessionCookie.html (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/GetSessionCookie.html 2018-12-04 00:08:42 UTC (rev 238828)
@@ -0,0 +1,3 @@
+<script>
+window.webkit.messageHandlers.testHandler.postMessage(document.cookie);
+</script>
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (238827 => 238828)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-12-04 00:08:37 UTC (rev 238827)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-12-04 00:08:42 UTC (rev 238828)
@@ -2988,6 +2988,63 @@
done = false;
}
+TEST(ProcessSwap, UseSessionCookiesAfterProcessSwapInPrivateBrowsing)
+{
+ auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+ processPoolConfiguration.get().processSwapsOnNavigation = YES;
+ auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+ RetainPtr<WKWebsiteDataStore> ephemeralStore = [WKWebsiteDataStore nonPersistentDataStore];
+
+ auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [webViewConfiguration setProcessPool:processPool.get()];
+ webViewConfiguration.get().websiteDataStore = ephemeralStore.get();
+
+ auto handler = adoptNS([[PSONScheme alloc] init]);
+ [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"pson"];
+
+ auto messageHandler = adoptNS([[PSONMessageHandler alloc] init]);
+ [[webViewConfiguration userContentController] addScriptMessageHandler:messageHandler.get() name:@"testHandler"];
+
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"SetSessionCookie" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid1 = [webView _webProcessIdentifier];
+
+ // Should process-swap.
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid2 = [webView _webProcessIdentifier];
+ EXPECT_NE(pid1, pid2);
+
+ // Should process-swap.
+ request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"GetSessionCookie" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto pid3 = [webView _webProcessIdentifier];
+ EXPECT_NE(pid2, pid3);
+
+ TestWebKitAPI::Util::run(&receivedMessage);
+ receivedMessage = false;
+
+ EXPECT_EQ(1u, [receivedMessages count]);
+ EXPECT_WK_STREQ(@"foo=bar", receivedMessages.get()[0]);
+}
+
#if PLATFORM(MAC)
TEST(ProcessSwap, TerminateProcessAfterProcessSwap)
Added: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/SetSessionCookie.html (0 => 238828)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/SetSessionCookie.html (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/SetSessionCookie.html 2018-12-04 00:08:42 UTC (rev 238828)
@@ -0,0 +1,3 @@
+<script>
+document.cookie = "foo=bar";
+</script>
