Diff
Modified: trunk/Source/WebCore/ChangeLog (232422 => 232423)
--- trunk/Source/WebCore/ChangeLog 2018-06-01 22:55:27 UTC (rev 232422)
+++ trunk/Source/WebCore/ChangeLog 2018-06-01 22:56:29 UTC (rev 232423)
@@ -1,3 +1,23 @@
+2018-06-01 Chris Dumez <[email protected]>
+
+ Regression(r230567): Unable to log into twitter.com in private sessions
+ https://bugs.webkit.org/show_bug.cgi?id=186205
+ <rdar://problem/40670799>
+
+ Reviewed by Youenn Fablet.
+
+ We were using the same SWServer for all private sessions and the SWServer's sessionID would
+ be legacyPrivateSessionID(). As a result, the service worker's sessionID would be legacyPrivateSessionID()
+ as well and would not match the sessionID of its client pages. This sessionID mismatch was
+ causing the breakage.
+
+ Instead of using the same SWServer of all private sessions, we now go back to using a SWServer
+ per private session. However, we now make sure that the SWServer gets destroyed whenever its
+ corresponding session gets destroyed.
+
+ * workers/service/server/SWServer.cpp:
+ (WebCore::SWServer::~SWServer):
+
2018-06-01 Youenn Fablet <[email protected]>
Add an option to restrict communication to localhost sockets
Modified: trunk/Source/WebCore/workers/service/server/SWServer.cpp (232422 => 232423)
--- trunk/Source/WebCore/workers/service/server/SWServer.cpp 2018-06-01 22:55:27 UTC (rev 232422)
+++ trunk/Source/WebCore/workers/service/server/SWServer.cpp 2018-06-01 22:56:29 UTC (rev 232423)
@@ -70,10 +70,6 @@
SWServer::~SWServer()
{
- RELEASE_ASSERT(m_connections.isEmpty());
- RELEASE_ASSERT(m_registrations.isEmpty());
- RELEASE_ASSERT(m_jobQueues.isEmpty());
-
allServers().remove(this);
}
Modified: trunk/Source/WebKit/ChangeLog (232422 => 232423)
--- trunk/Source/WebKit/ChangeLog 2018-06-01 22:55:27 UTC (rev 232422)
+++ trunk/Source/WebKit/ChangeLog 2018-06-01 22:56:29 UTC (rev 232423)
@@ -1,3 +1,42 @@
+2018-06-01 Chris Dumez <[email protected]>
+
+ Regression(r230567): Unable to log into twitter.com in private sessions
+ https://bugs.webkit.org/show_bug.cgi?id=186205
+ <rdar://problem/40670799>
+
+ Reviewed by Youenn Fablet.
+
+ We were using the same SWServer for all private sessions and the SWServer's sessionID would
+ be legacyPrivateSessionID(). As a result, the service worker's sessionID would be legacyPrivateSessionID()
+ as well and would not match the sessionID of its client pages. This sessionID mismatch was
+ causing the breakage.
+
+ Instead of using the same SWServer of all private sessions, we now go back to using a SWServer
+ per private session. However, we now make sure that the SWServer gets destroyed whenever its
+ corresponding session gets destroyed.
+
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::destroySession):
+ * NetworkProcess/cache/CacheStorageEngine.cpp:
+ (WebKit::CacheStorage::Engine::from):
+ * StorageProcess/StorageProcess.cpp:
+ (WebKit::StorageProcess::destroySession):
+ (WebKit::StorageProcess::swServerForSession):
+ * StorageProcess/StorageProcess.h:
+ * StorageProcess/StorageProcess.messages.in:
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::setAnyPageGroupMightHavePrivateBrowsingEnabled):
+ * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+ (WebKit::WebsiteDataStore::~WebsiteDataStore):
+
+ (WebKit::WebsiteDataStore::enableResourceLoadStatisticsAndSetTestingCallback):
+ * UIProcess/WebsiteData/WebsiteDataStore.h:
+ (WebKit::WebsiteDataStore::weakPtrFactory const):
+ Fix memory leak caused by a reference cycle between the WebsiteDataStore and its
+ WebResourceLoadStatisticsStore, by using WeakPtr to break the cycle. This was causing
+ us to leak WebsiteDataStore objects, which would prevent the destruction of sessions.
+
+
2018-06-01 Youenn Fablet <[email protected]>
Add an option to restrict communication to localhost sockets
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp (232422 => 232423)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2018-06-01 22:55:27 UTC (rev 232422)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2018-06-01 22:56:29 UTC (rev 232423)
@@ -363,6 +363,7 @@
{
SessionTracker::destroySession(sessionID);
m_sessionsControlledByAutomation.remove(sessionID);
+ CacheStorage::Engine::destroyEngine(sessionID);
}
void NetworkProcess::grantSandboxExtensionsToStorageProcessForBlobs(const Vector<String>& filenames, Function<void ()>&& completionHandler)
Modified: trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp (232422 => 232423)
--- trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp 2018-06-01 22:55:27 UTC (rev 232422)
+++ trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp 2018-06-01 22:56:29 UTC (rev 232423)
@@ -82,9 +82,6 @@
Engine& Engine::from(PAL::SessionID sessionID)
{
- if (sessionID.isEphemeral())
- sessionID = PAL::SessionID::legacyPrivateSessionID();
-
auto addResult = globalEngineMap().add(sessionID, nullptr);
if (addResult.isNewEntry)
addResult.iterator->value = Engine::create(NetworkProcess::singleton().cacheStorageDirectory(sessionID));
Modified: trunk/Source/WebKit/StorageProcess/StorageProcess.cpp (232422 => 232423)
--- trunk/Source/WebKit/StorageProcess/StorageProcess.cpp 2018-06-01 22:55:27 UTC (rev 232422)
+++ trunk/Source/WebKit/StorageProcess/StorageProcess.cpp 2018-06-01 22:56:29 UTC (rev 232423)
@@ -284,6 +284,16 @@
#endif
}
+void StorageProcess::destroySession(PAL::SessionID sessionID)
+{
+#if ENABLE(SERVICE_WORKER)
+ m_swServers.remove(sessionID);
+ m_swDatabasePaths.remove(sessionID);
+#else
+ UNUSED_PARAM(sessionID);
+#endif
+}
+
void StorageProcess::fetchWebsiteData(PAL::SessionID sessionID, OptionSet<WebsiteDataType> websiteDataTypes, uint64_t callbackID)
{
auto websiteData = std::make_unique<WebsiteData>();
@@ -424,10 +434,6 @@
{
ASSERT(sessionID.isValid());
- // Use the same SWServer for all ephemeral sessions.
- if (sessionID.isEphemeral())
- sessionID = PAL::SessionID::legacyPrivateSessionID();
-
auto result = m_swServers.add(sessionID, nullptr);
if (!result.isNewEntry) {
ASSERT(result.iterator->value);
Modified: trunk/Source/WebKit/StorageProcess/StorageProcess.h (232422 => 232423)
--- trunk/Source/WebKit/StorageProcess/StorageProcess.h 2018-06-01 22:55:27 UTC (rev 232422)
+++ trunk/Source/WebKit/StorageProcess/StorageProcess.h 2018-06-01 22:56:29 UTC (rev 232423)
@@ -128,6 +128,7 @@
void initializeWebsiteDataStore(const StorageProcessCreationParameters&);
void createStorageToWebProcessConnection(bool isServiceWorkerProcess, WebCore::SecurityOriginData&&);
+ void destroySession(PAL::SessionID);
void fetchWebsiteData(PAL::SessionID, OptionSet<WebsiteDataType> websiteDataTypes, uint64_t callbackID);
void deleteWebsiteData(PAL::SessionID, OptionSet<WebsiteDataType> websiteDataTypes, WallTime modifiedSince, uint64_t callbackID);
void deleteWebsiteDataForOrigins(PAL::SessionID, OptionSet<WebsiteDataType> websiteDataTypes, const Vector<WebCore::SecurityOriginData>& origins, uint64_t callbackID);
Modified: trunk/Source/WebKit/StorageProcess/StorageProcess.messages.in (232422 => 232423)
--- trunk/Source/WebKit/StorageProcess/StorageProcess.messages.in 2018-06-01 22:55:27 UTC (rev 232422)
+++ trunk/Source/WebKit/StorageProcess/StorageProcess.messages.in 2018-06-01 22:56:29 UTC (rev 232423)
@@ -35,6 +35,8 @@
DidGetSandboxExtensionsForBlobFiles(uint64_t requestID, WebKit::SandboxExtension::HandleArray extensions)
#endif
+ DestroySession(PAL::SessionID sessionID)
+
#if ENABLE(SERVICE_WORKER)
DidNotHandleFetch(WebCore::SWServerConnectionIdentifier serverConnectionIdentifier, WebCore::FetchIdentifier fetchIdentifier)
DidFailFetch(WebCore::SWServerConnectionIdentifier serverConnectionIdentifier, WebCore::FetchIdentifier fetchIdentifier)
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (232422 => 232423)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-06-01 22:55:27 UTC (rev 232422)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-06-01 22:56:29 UTC (rev 232423)
@@ -721,17 +721,14 @@
void WebProcessPool::setAnyPageGroupMightHavePrivateBrowsingEnabled(bool privateBrowsingEnabled)
{
- if (networkProcess()) {
- if (privateBrowsingEnabled)
- networkProcess()->send(Messages::NetworkProcess::AddWebsiteDataStore(WebsiteDataStoreParameters::legacyPrivateSessionParameters()), 0);
- else
- networkProcess()->send(Messages::NetworkProcess::DestroySession(PAL::SessionID::legacyPrivateSessionID()), 0);
- }
-
- if (privateBrowsingEnabled)
+ if (privateBrowsingEnabled) {
+ sendToNetworkingProcess(Messages::NetworkProcess::AddWebsiteDataStore(WebsiteDataStoreParameters::legacyPrivateSessionParameters()));
sendToAllProcesses(Messages::WebProcess::AddWebsiteDataStore(WebsiteDataStoreParameters::legacyPrivateSessionParameters()));
- else
+ } else {
+ sendToNetworkingProcess(Messages::NetworkProcess::DestroySession(PAL::SessionID::legacyPrivateSessionID()));
+ sendToStorageProcess(Messages::StorageProcess::DestroySession(PAL::SessionID::legacyPrivateSessionID()));
sendToAllProcesses(Messages::WebProcess::DestroySession(PAL::SessionID::legacyPrivateSessionID()));
+ }
}
void (*s_invalidMessageCallback)(WKStringRef messageName);
Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (232422 => 232423)
--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2018-06-01 22:55:27 UTC (rev 232422)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2018-06-01 22:56:29 UTC (rev 232423)
@@ -32,6 +32,7 @@
#include "NetworkProcessMessages.h"
#include "StorageManager.h"
#include "StorageProcessCreationParameters.h"
+#include "StorageProcessMessages.h"
#include "WebCookieManagerProxy.h"
#include "WebProcessMessages.h"
#include "WebProcessPool.h"
@@ -116,8 +117,10 @@
if (m_sessionID.isValid() && m_sessionID != PAL::SessionID::defaultSessionID()) {
ASSERT(nonDefaultDataStores().get(m_sessionID) == this);
nonDefaultDataStores().remove(m_sessionID);
- for (auto& processPool : WebProcessPool::allProcessPools())
+ for (auto& processPool : WebProcessPool::allProcessPools()) {
processPool->sendToNetworkingProcess(Messages::NetworkProcess::DestroySession(m_sessionID));
+ processPool->sendToStorageProcess(Messages::StorageProcess::DestroySession(m_sessionID));
+ }
}
}
@@ -1462,16 +1465,21 @@
}
#if HAVE(CFNETWORK_STORAGE_PARTITIONING)
- m_resourceLoadStatistics = WebResourceLoadStatisticsStore::create(m_configuration.resourceLoadStatisticsDirectory, WTFMove(callback), m_sessionID.isEphemeral(), [this, protectedThis = makeRef(*this)] (const Vector<String>& domainsToPartition, const Vector<String>& domainsToBlock, const Vector<String>& domainsToNeitherPartitionNorBlock, ShouldClearFirst shouldClearFirst) {
- updatePrevalentDomainsToPartitionOrBlockCookies(domainsToPartition, domainsToBlock, domainsToNeitherPartitionNorBlock, shouldClearFirst);
- }, [this, protectedThis = makeRef(*this)] (const String& resourceDomain, const String& firstPartyDomain, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void(bool hasAccess)>&& callback) {
- hasStorageAccessForFrameHandler(resourceDomain, firstPartyDomain, frameID, pageID, WTFMove(callback));
- }, [this, protectedThis = makeRef(*this)] (const String& resourceDomain, const String& firstPartyDomain, std::optional<uint64_t> frameID, uint64_t pageID, WTF::CompletionHandler<void(bool wasGranted)>&& callback) {
- grantStorageAccessHandler(resourceDomain, firstPartyDomain, frameID, pageID, WTFMove(callback));
- }, [this, protectedThis = makeRef(*this)] () {
- removeAllStorageAccessHandler();
- }, [this, protectedThis = makeRef(*this)] (const Vector<String>& domainsToRemove) {
- removePrevalentDomains(domainsToRemove);
+ m_resourceLoadStatistics = WebResourceLoadStatisticsStore::create(m_configuration.resourceLoadStatisticsDirectory, WTFMove(callback), m_sessionID.isEphemeral(), [weakThis = makeWeakPtr(*this)] (const Vector<String>& domainsToPartition, const Vector<String>& domainsToBlock, const Vector<String>& domainsToNeitherPartitionNorBlock, ShouldClearFirst shouldClearFirst) {
+ if (weakThis)
+ weakThis->updatePrevalentDomainsToPartitionOrBlockCookies(domainsToPartition, domainsToBlock, domainsToNeitherPartitionNorBlock, shouldClearFirst);
+ }, [weakThis = makeWeakPtr(*this)] (const String& resourceDomain, const String& firstPartyDomain, uint64_t frameID, uint64_t pageID, WTF::CompletionHandler<void(bool hasAccess)>&& callback) {
+ if (weakThis)
+ weakThis->hasStorageAccessForFrameHandler(resourceDomain, firstPartyDomain, frameID, pageID, WTFMove(callback));
+ }, [weakThis = makeWeakPtr(*this)] (const String& resourceDomain, const String& firstPartyDomain, std::optional<uint64_t> frameID, uint64_t pageID, WTF::CompletionHandler<void(bool wasGranted)>&& callback) {
+ if (weakThis)
+ weakThis->grantStorageAccessHandler(resourceDomain, firstPartyDomain, frameID, pageID, WTFMove(callback));
+ }, [weakThis = makeWeakPtr(*this)] () {
+ if (weakThis)
+ weakThis->removeAllStorageAccessHandler();
+ }, [weakThis = makeWeakPtr(*this)] (const Vector<String>& domainsToRemove) {
+ if (weakThis)
+ weakThis->removePrevalentDomains(domainsToRemove);
});
#else
m_resourceLoadStatistics = WebResourceLoadStatisticsStore::create(m_configuration.resourceLoadStatisticsDirectory, WTFMove(callback), m_sessionID.isEphemeral());
Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h (232422 => 232423)
--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h 2018-06-01 22:55:27 UTC (rev 232422)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h 2018-06-01 22:56:29 UTC (rev 232423)
@@ -37,6 +37,7 @@
#include <wtf/OptionSet.h>
#include <wtf/RefCounted.h>
#include <wtf/RefPtr.h>
+#include <wtf/WeakPtr.h>
#include <wtf/WorkQueue.h>
#include <wtf/text/WTFString.h>
@@ -182,6 +183,8 @@
void addSecKeyProxyStore(Ref<SecKeyProxyStore>&&);
#endif
+ auto& weakPtrFactory() const { return m_weakFactory; }
+
private:
explicit WebsiteDataStore(PAL::SessionID);
explicit WebsiteDataStore(Configuration, PAL::SessionID);
@@ -212,6 +215,7 @@
void maybeRegisterWithSessionIDMap();
+ WeakPtrFactory<WebsiteDataStore> m_weakFactory;
const PAL::SessionID m_sessionID;
const Configuration m_configuration;
