Clearing all Website Data should remove service worker registrations on disk
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Dec 2017 23:13:49 +0000 (23:13 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Dec 2017 23:13:49 +0000 (23:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180558

Reviewed by Youenn Fablet.

Source/WebCore:

Clear service worker registrations on disk in addition to the ones in memory.

* workers/service/server/RegistrationDatabase.cpp:
(WebCore::v1RecordsTableSchema):
(WebCore::v1RecordsTableSchemaAlternate):
(WebCore::databaseFilename):
Make sure these always get called from the background thread since they use
a static string.

(WebCore::RegistrationDatabase::RegistrationDatabase):
Call importRecordsIfNecessary() instead of openSQLiteDatabase(). importRecordsIfNecessary()
only calls openSQLiteDatabase() if the database file exists, to avoid creating a database
file unnecessarily.

(WebCore::RegistrationDatabase::databasePath const):
New method which returns the database file path.

(WebCore::RegistrationDatabase::openSQLiteDatabase):

(WebCore::RegistrationDatabase::importRecordsIfNecessary):
New methods which imports records if the database file exist. It the database file does
not exist, it does not create it.

(WebCore::RegistrationDatabase::pushChanges):
Call completion handler when changes are pushed.

(WebCore::RegistrationDatabase::clearAll):
Close the database if it is open, then remove the database files.

(WebCore::RegistrationDatabase::doPushChanges):
If the database is not already open, we now open it when trying to write changes for
the first time.

* workers/service/server/RegistrationDatabase.h:
* workers/service/server/RegistrationStore.cpp:
(WebCore::RegistrationStore::clearAll):
(WebCore::RegistrationStore::flushChanges):
* workers/service/server/RegistrationStore.h:

* workers/service/server/SWServer.cpp:
(WebCore::SWServer::clearAll):
(WebCore::SWServer::clear):
Also clear the database.

* workers/service/server/SWServer.h:

* workers/service/server/SWServerWorker.cpp:
(WebCore::SWServerWorker::terminate):
Only call SWServer::terminateWorker() if the worker is running. Otherwise, we hit
an assertion when clearing a registration would worker was already terminated.

Source/WebKit:

* StorageProcess/StorageProcess.cpp:
(WebKit::StorageProcess::deleteWebsiteData):
(WebKit::StorageProcess::deleteWebsiteDataForOrigins):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225711 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
Source/WebCore/ChangeLog
Source/WebCore/workers/service/ServiceWorkerRegistrationKey.cpp
Source/WebCore/workers/service/ServiceWorkerRegistrationKey.h
Source/WebCore/workers/service/server/RegistrationDatabase.cpp
Source/WebCore/workers/service/server/RegistrationDatabase.h
Source/WebCore/workers/service/server/RegistrationStore.cpp
Source/WebCore/workers/service/server/RegistrationStore.h
Source/WebCore/workers/service/server/SWServer.cpp
Source/WebCore/workers/service/server/SWServer.h
Source/WebCore/workers/service/server/SWServerWorker.cpp
Source/WebKit/ChangeLog
Source/WebKit/StorageProcess/StorageProcess.cpp

index b0f6365..88109c4 100644 (file)
@@ -1,3 +1,61 @@
+2017-12-08  Chris Dumez  <cdumez@apple.com>
+
+        Clearing all Website Data should remove service worker registrations on disk
+        https://bugs.webkit.org/show_bug.cgi?id=180558
+
+        Reviewed by Youenn Fablet.
+
+        Clear service worker registrations on disk in addition to the ones in memory.
+
+        * workers/service/server/RegistrationDatabase.cpp:
+        (WebCore::v1RecordsTableSchema):
+        (WebCore::v1RecordsTableSchemaAlternate):
+        (WebCore::databaseFilename):
+        Make sure these always get called from the background thread since they use
+        a static string.
+
+        (WebCore::RegistrationDatabase::RegistrationDatabase):
+        Call importRecordsIfNecessary() instead of openSQLiteDatabase(). importRecordsIfNecessary()
+        only calls openSQLiteDatabase() if the database file exists, to avoid creating a database
+        file unnecessarily.
+
+        (WebCore::RegistrationDatabase::databasePath const):
+        New method which returns the database file path.
+
+        (WebCore::RegistrationDatabase::openSQLiteDatabase):
+
+        (WebCore::RegistrationDatabase::importRecordsIfNecessary):
+        New methods which imports records if the database file exist. It the database file does
+        not exist, it does not create it.
+
+        (WebCore::RegistrationDatabase::pushChanges):
+        Call completion handler when changes are pushed.
+
+        (WebCore::RegistrationDatabase::clearAll):
+        Close the database if it is open, then remove the database files.
+
+        (WebCore::RegistrationDatabase::doPushChanges):
+        If the database is not already open, we now open it when trying to write changes for
+        the first time.
+
+        * workers/service/server/RegistrationDatabase.h:
+        * workers/service/server/RegistrationStore.cpp:
+        (WebCore::RegistrationStore::clearAll):
+        (WebCore::RegistrationStore::flushChanges):
+        * workers/service/server/RegistrationStore.h:
+
+        * workers/service/server/SWServer.cpp:
+        (WebCore::SWServer::clearAll):
+        (WebCore::SWServer::clear):
+        Also clear the database.
+
+        * workers/service/server/SWServer.h:
+
+        * workers/service/server/SWServerWorker.cpp:
+        (WebCore::SWServerWorker::terminate):
+        Only call SWServer::terminateWorker() if the worker is running. Otherwise, we hit
+        an assertion when clearing a registration would worker was already terminated.
+
 2017-12-08  Joseph Pecoraro  <pecoraro@apple.com>
 
         ServiceWorker Inspector: Various issues inspecting service worker on mobile.twitter.com
index 2c89dd4..b84c66e 100644 (file)
@@ -28,6 +28,7 @@
 
 #if ENABLE(SERVICE_WORKER)
 
+#include "SecurityOrigin.h"
 #include "URLHash.h"
 
 namespace WebCore {
@@ -76,6 +77,15 @@ bool ServiceWorkerRegistrationKey::originIsMatching(const SecurityOriginData& to
     return protocolHostAndPortAreEqual(clientURL, m_scope);
 }
 
+bool ServiceWorkerRegistrationKey::relatesToOrigin(const SecurityOrigin& origin) const
+{
+    if (m_topOrigin == SecurityOriginData::fromSecurityOrigin(origin))
+        return true;
+
+    auto scopeOrigin = SecurityOrigin::create(m_scope);
+    return scopeOrigin->isSameOriginAs(origin);
+}
+
 static const char separatorCharacter = '_';
 
 String ServiceWorkerRegistrationKey::toDatabaseKey() const
index 14ec5fa..239a300 100644 (file)
@@ -49,6 +49,8 @@ public:
     const URL& scope() const { return m_scope; }
     void setScope(URL&& scope) { m_scope = WTFMove(scope); }
 
+    bool relatesToOrigin(const SecurityOrigin&) const;
+
     ServiceWorkerRegistrationKey isolatedCopy() const;
 
     template<class Encoder> void encode(Encoder&) const;
index 27de94e..8710b0e 100644 (file)
@@ -35,6 +35,8 @@
 #include "SQLiteFileSystem.h"
 #include "SQLiteStatement.h"
 #include "SQLiteTransaction.h"
+#include "SecurityOrigin.h"
+#include <wtf/CompletionHandler.h>
 #include <wtf/MainThread.h>
 #include <wtf/NeverDestroyed.h>
 #include <wtf/Scope.h>
@@ -48,18 +50,21 @@ static const String v1RecordsTableSchema(const String& tableName)
 
 static const String v1RecordsTableSchema()
 {
+    ASSERT(!isMainThread());
     static NeverDestroyed<WTF::String> schema(v1RecordsTableSchema("Records"));
     return schema;
 }
 
 static const String v1RecordsTableSchemaAlternate()
 {
+    ASSERT(!isMainThread());
     static NeverDestroyed<WTF::String> schema(v1RecordsTableSchema("\"Records\""));
     return schema;
 }
 
 static const String& databaseFilename()
 {
+    ASSERT(isMainThread());
     static NeverDestroyed<String> filename = "ServiceWorkerRegistrations.sqlite3";
     return filename;
 }
@@ -68,10 +73,13 @@ RegistrationDatabase::RegistrationDatabase(RegistrationStore& store, const Strin
     : CrossThreadTaskHandler("ServiceWorker I/O Thread")
     , m_store(store)
     , m_databaseDirectory(databaseDirectory)
+    , m_databaseFilePath(FileSystem::pathByAppendingComponent(m_databaseDirectory, databaseFilename()))
 {
     ASSERT(isMainThread());
 
-    postTask(createCrossThreadTask(*this, &RegistrationDatabase::openSQLiteDatabase, FileSystem::pathByAppendingComponent(m_databaseDirectory, databaseFilename())));
+    postTask(CrossThreadTask([this] {
+        importRecordsIfNecessary();
+    }));
 }
 
 RegistrationDatabase::~RegistrationDatabase()
@@ -95,8 +103,8 @@ void RegistrationDatabase::openSQLiteDatabase(const String& fullFilename)
         postTaskReply(createCrossThreadTask(*this, &RegistrationDatabase::databaseFailedToOpen));
     });
 
-    SQLiteFileSystem::ensureDatabaseDirectoryExists(m_databaseDirectory.isolatedCopy());
-    
+    SQLiteFileSystem::ensureDatabaseDirectoryExists(m_databaseDirectory);
+
     m_database = std::make_unique<SQLiteDatabase>();
     if (!m_database->open(fullFilename)) {
         errorMessage = "Failed to open registration database";
@@ -112,6 +120,15 @@ void RegistrationDatabase::openSQLiteDatabase(const String& fullFilename)
         return;
     
     scopeExit.release();
+}
+
+void RegistrationDatabase::importRecordsIfNecessary()
+{
+    ASSERT(!isMainThread());
+
+    if (FileSystem::fileExists(m_databaseFilePath))
+        openSQLiteDatabase(m_databaseFilePath);
+
     postTaskReply(createCrossThreadTask(*this, &RegistrationDatabase::databaseOpenedAndRecordsImported));
 }
 
@@ -202,15 +219,41 @@ static std::optional<WorkerType> stringToWorkerType(const String& type)
     return std::nullopt;
 }
 
-void RegistrationDatabase::pushChanges(Vector<ServiceWorkerContextData>&& datas)
+void RegistrationDatabase::pushChanges(Vector<ServiceWorkerContextData>&& datas, WTF::CompletionHandler<void()>&& completionHandler)
 {
-    postTask(createCrossThreadTask(*this, &RegistrationDatabase::doPushChanges, datas));
+    postTask(CrossThreadTask([this, datas = crossThreadCopy(datas), completionHandler = WTFMove(completionHandler)]() mutable {
+        doPushChanges(WTFMove(datas));
+
+        if (!completionHandler)
+            return;
+
+        postTaskReply(CrossThreadTask([completionHandler = WTFMove(completionHandler)] {
+            completionHandler();
+        }));
+    }));
+}
+
+void RegistrationDatabase::clearAll(WTF::CompletionHandler<void()>&& completionHandler)
+{
+    postTask(CrossThreadTask([this, completionHandler = WTFMove(completionHandler)]() mutable {
+        m_database = nullptr;
+
+        SQLiteFileSystem::deleteDatabaseFile(m_databaseFilePath);
+        SQLiteFileSystem::deleteEmptyDatabaseDirectory(m_databaseDirectory);
+
+        postTaskReply(CrossThreadTask([completionHandler = WTFMove(completionHandler)] {
+            completionHandler();
+        }));
+    }));
 }
 
 void RegistrationDatabase::doPushChanges(Vector<ServiceWorkerContextData>&& datas)
 {
-    if (!m_database)
-        return;
+    if (!m_database) {
+        openSQLiteDatabase(m_databaseFilePath);
+        if (!m_database)
+            return;
+    }
 
     SQLiteTransaction transaction(*m_database);
     transaction.begin();
index 6364914..9d83e00 100644 (file)
@@ -27,6 +27,7 @@
 
 #if ENABLE(SERVICE_WORKER)
 
+#include "SecurityOrigin.h"
 #include <wtf/CrossThreadTaskHandler.h>
 #include <wtf/text/WTFString.h>
 
@@ -44,14 +45,17 @@ public:
 
     bool isClosed() const { return !m_database; }
 
-    void pushChanges(Vector<ServiceWorkerContextData>&&);
+    void pushChanges(Vector<ServiceWorkerContextData>&&, WTF::CompletionHandler<void()>&&);
+    void clearAll(WTF::CompletionHandler<void()>&&);
 
 private:
     // Methods to be run on the task thread
     void openSQLiteDatabase(const String& fullFilename);
     String ensureValidRecordsTable();
     String importRecords();
+    void importRecordsIfNecessary();
     void doPushChanges(Vector<ServiceWorkerContextData>&&);
+    void doClearOrigin(const SecurityOrigin&);
 
     // Replies to the main thread
     void addRegistrationToStore(ServiceWorkerContextData&&);
@@ -60,6 +64,7 @@ private:
 
     RegistrationStore& m_store;
     String m_databaseDirectory;
+    String m_databaseFilePath;
     std::unique_ptr<SQLiteDatabase> m_database;
 };
 
index 264e55e..dbbd888 100644 (file)
@@ -51,7 +51,7 @@ void RegistrationStore::scheduleDatabasePushIfNecessary()
     m_databasePushTimer.startOneShot(0_s);
 }
 
-void RegistrationStore::pushChangesToDatabase()
+void RegistrationStore::pushChangesToDatabase(WTF::CompletionHandler<void()>&& completionHandler)
 {
     Vector<ServiceWorkerContextData> changesToPush;
     changesToPush.reserveInitialCapacity(m_updatedRegistrations.size());
@@ -59,7 +59,24 @@ void RegistrationStore::pushChangesToDatabase()
         changesToPush.uncheckedAppend(WTFMove(value));
 
     m_updatedRegistrations.clear();
-    m_database.pushChanges(WTFMove(changesToPush));
+    m_database.pushChanges(WTFMove(changesToPush), WTFMove(completionHandler));
+}
+
+void RegistrationStore::clearAll(WTF::CompletionHandler<void()>&& completionHandler)
+{
+    m_updatedRegistrations.clear();
+    m_databasePushTimer.stop();
+    m_database.clearAll(WTFMove(completionHandler));
+}
+
+void RegistrationStore::flushChanges(WTF::CompletionHandler<void()>&& completionHandler)
+{
+    if (m_databasePushTimer.isActive()) {
+        pushChangesToDatabase(WTFMove(completionHandler));
+        m_databasePushTimer.stop();
+        return;
+    }
+    completionHandler();
 }
 
 void RegistrationStore::updateRegistration(const ServiceWorkerContextData& data)
index 583fa51..13379d8 100644 (file)
@@ -32,6 +32,7 @@
 #include "ServiceWorkerRegistrationData.h"
 #include "ServiceWorkerRegistrationKey.h"
 #include "Timer.h"
+#include <wtf/CompletionHandler.h>
 #include <wtf/HashMap.h>
 #include <wtf/text/WTFString.h>
 
@@ -46,6 +47,9 @@ public:
     explicit RegistrationStore(SWServer&, const String& databaseDirectory);
     ~RegistrationStore();
 
+    void clearAll(WTF::CompletionHandler<void()>&&);
+    void flushChanges(WTF::CompletionHandler<void()>&&);
+
     // Callbacks from the SWServer
     void updateRegistration(const ServiceWorkerContextData&);
     void removeRegistration(SWServerRegistration&);
@@ -57,7 +61,8 @@ public:
 
 private:
     void scheduleDatabasePushIfNecessary();
-    void pushChangesToDatabase();
+    void pushChangesToDatabase(WTF::CompletionHandler<void()>&&);
+    void pushChangesToDatabase() { pushChangesToDatabase({ }); }
 
     SWServer& m_server;
     RegistrationDatabase m_database;
index cf2cad9..8c70ef7 100644 (file)
@@ -147,20 +147,33 @@ Vector<ServiceWorkerRegistrationData> SWServer::getRegistrations(const SecurityO
     return matchingRegistrationDatas;
 }
 
-void SWServer::clearAll()
+void SWServer::clearAll(WTF::CompletionHandler<void()>&& completionHandler)
 {
     m_jobQueues.clear();
     while (!m_registrations.isEmpty())
         m_registrations.begin()->value->clear();
     ASSERT(m_registrationsByID.isEmpty());
     m_originStore->clearAll();
+    m_registrationStore.clearAll(WTFMove(completionHandler));
 }
 
-void SWServer::clear(const SecurityOrigin& origin)
+void SWServer::clear(const SecurityOrigin& origin, WTF::CompletionHandler<void()>&& completionHandler)
 {
-    m_originStore->clear(origin);
+    m_jobQueues.removeIf([&](auto& keyAndValue) {
+        return keyAndValue.key.relatesToOrigin(origin);
+    });
+
+    Vector<SWServerRegistration*> registrationsToRemove;
+    for (auto& keyAndValue : m_registrations) {
+        if (keyAndValue.key.relatesToOrigin(origin))
+            registrationsToRemove.append(keyAndValue.value.get());
+    }
+
+    // Calling SWServerRegistration::clear() takes care of updating m_registrations, m_originStore and m_registrationStore.
+    for (auto* registration : registrationsToRemove)
+        registration->clear();
 
-    // FIXME: We should clear entries in m_registrations, m_jobQueues and m_workersByID.
+    m_registrationStore.flushChanges(WTFMove(completionHandler));
 }
 
 void SWServer::Connection::scheduleJobInServer(const ServiceWorkerJobData& jobData)
index da6d77d..212af47 100644 (file)
@@ -117,8 +117,8 @@ public:
     WEBCORE_EXPORT SWServer(UniqueRef<SWOriginStore>&&, String&& registrationDatabaseDirectory, PAL::SessionID);
     WEBCORE_EXPORT ~SWServer();
 
-    WEBCORE_EXPORT void clearAll();
-    WEBCORE_EXPORT void clear(const SecurityOrigin&);
+    WEBCORE_EXPORT void clearAll(WTF::CompletionHandler<void()>&&);
+    WEBCORE_EXPORT void clear(const SecurityOrigin&, WTF::CompletionHandler<void()>&&);
 
     SWServerRegistration* getRegistration(const ServiceWorkerRegistrationKey&);
     void addRegistration(std::unique_ptr<SWServerRegistration>&&);
index b56d724..929c063 100644 (file)
@@ -70,7 +70,8 @@ ServiceWorkerContextData SWServerWorker::contextData() const
 
 void SWServerWorker::terminate()
 {
-    m_server.terminateWorker(*this);
+    if (isRunning())
+        m_server.terminateWorker(*this);
 }
 
 const ClientOrigin& SWServerWorker::origin() const
index 35e8f88..2d5fafa 100644 (file)
@@ -1,3 +1,14 @@
+2017-12-08  Chris Dumez  <cdumez@apple.com>
+
+        Clearing all Website Data should remove service worker registrations on disk
+        https://bugs.webkit.org/show_bug.cgi?id=180558
+
+        Reviewed by Youenn Fablet.
+
+        * StorageProcess/StorageProcess.cpp:
+        (WebKit::StorageProcess::deleteWebsiteData):
+        (WebKit::StorageProcess::deleteWebsiteDataForOrigins):
+
 2017-12-08  Youenn Fablet  <youenn@apple.com>
 
         WebServiceWorkerProvider should use Cancellation error to notify DTL that it cannot handle a fetch
index 619c6ba..ed7d650 100644 (file)
@@ -43,6 +43,7 @@
 #include <WebCore/ServiceWorkerClientIdentifier.h>
 #include <WebCore/TextEncoding.h>
 #include <pal/SessionID.h>
+#include <wtf/CallbackAggregator.h>
 #include <wtf/CrossThreadTask.h>
 #include <wtf/MainThread.h>
 
@@ -295,50 +296,42 @@ void StorageProcess::fetchWebsiteData(PAL::SessionID sessionID, OptionSet<Websit
 
 void StorageProcess::deleteWebsiteData(PAL::SessionID sessionID, OptionSet<WebsiteDataType> websiteDataTypes, std::chrono::system_clock::time_point modifiedSince, uint64_t callbackID)
 {
-    auto completionHandler = [this, callbackID]() {
+    auto callbackAggregator = CallbackAggregator::create([this, callbackID] {
         parentProcessConnection()->send(Messages::StorageProcessProxy::DidDeleteWebsiteData(callbackID), 0);
-    };
+    });
 
 #if ENABLE(SERVICE_WORKER)
     if (websiteDataTypes.contains(WebsiteDataType::ServiceWorkerRegistrations)) {
         if (auto* server = m_swServers.get(sessionID))
-            server->clearAll();
+            server->clearAll([callbackAggregator = callbackAggregator.copyRef()] { });
     }
 #endif
 
 #if ENABLE(INDEXED_DATABASE)
-    if (websiteDataTypes.contains(WebsiteDataType::IndexedDBDatabases)) {
-        idbServer(sessionID).closeAndDeleteDatabasesModifiedSince(modifiedSince, WTFMove(completionHandler));
-        return;
-    }
+    if (websiteDataTypes.contains(WebsiteDataType::IndexedDBDatabases))
+        idbServer(sessionID).closeAndDeleteDatabasesModifiedSince(modifiedSince, [callbackAggregator = WTFMove(callbackAggregator)] { });
 #endif
-
-    completionHandler();
 }
 
 void StorageProcess::deleteWebsiteDataForOrigins(PAL::SessionID sessionID, OptionSet<WebsiteDataType> websiteDataTypes, const Vector<SecurityOriginData>& securityOriginDatas, uint64_t callbackID)
 {
-    auto completionHandler = [this, callbackID]() {
+    auto callbackAggregator = CallbackAggregator::create([this, callbackID]() {
         parentProcessConnection()->send(Messages::StorageProcessProxy::DidDeleteWebsiteDataForOrigins(callbackID), 0);
-    };
+    });
 
 #if ENABLE(SERVICE_WORKER)
     if (websiteDataTypes.contains(WebsiteDataType::ServiceWorkerRegistrations)) {
         if (auto* server = m_swServers.get(sessionID)) {
             for (auto& originData : securityOriginDatas)
-                server->clear(originData.securityOrigin());
+                server->clear(originData.securityOrigin(), [callbackAggregator = callbackAggregator.copyRef()] { });
         }
     }
 #endif
 
 #if ENABLE(INDEXED_DATABASE)
-    if (!websiteDataTypes.contains(WebsiteDataType::IndexedDBDatabases)) {
-        idbServer(sessionID).closeAndDeleteDatabasesForOrigins(securityOriginDatas, WTFMove(completionHandler));
-        return;
-    }
+    if (!websiteDataTypes.contains(WebsiteDataType::IndexedDBDatabases))
+        idbServer(sessionID).closeAndDeleteDatabasesForOrigins(securityOriginDatas, [callbackAggregator = WTFMove(callbackAggregator)] { });
 #endif
-
-    completionHandler();
 }
 
 #if ENABLE(SANDBOX_EXTENSIONS)