Remove IDBBackingStoreTemporaryFileHandler
authorsihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Oct 2019 00:53:20 +0000 (00:53 +0000)
committersihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Oct 2019 00:53:20 +0000 (00:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=203128

Reviewed by Alex Christensen.

Source/WebCore:

IDBBackingStoreTemporaryFileHandler has only one member function, and implementation of that is to delete
files. To keep the same behavior, we can remove this class and delete the temporary files at the places where
objects of this class were used.

By doing this, we no longer need to pass the NetworkProcess/InProcessIDBServer all the way down to
SQLiteIDBBackingStore that is used only on background thread.

* Modules/indexeddb/server/IDBBackingStore.h:
* Modules/indexeddb/server/IDBServer.cpp:
(WebCore::IDBServer::IDBServer::create):
(WebCore::IDBServer::IDBServer::IDBServer):
(WebCore::IDBServer::IDBServer::createBackingStore):
* Modules/indexeddb/server/IDBServer.h:
* Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
(WebCore::IDBServer::SQLiteIDBBackingStore::SQLiteIDBBackingStore):
* Modules/indexeddb/server/SQLiteIDBBackingStore.h:
(WebCore::IDBServer::SQLiteIDBBackingStore::temporaryFileHandler const): Deleted.
* Modules/indexeddb/server/SQLiteIDBTransaction.cpp:
(WebCore::IDBServer::SQLiteIDBTransaction::moveBlobFilesIfNecessary):
(WebCore::IDBServer::SQLiteIDBTransaction::deleteBlobFilesIfNecessary):
(WebCore::IDBServer::SQLiteIDBTransaction::abort):
* Modules/indexeddb/shared/InProcessIDBServer.cpp:
(WebCore::InProcessIDBServer::InProcessIDBServer):
(WebCore::InProcessIDBServer::accessToTemporaryFileComplete): Deleted.
* Modules/indexeddb/shared/InProcessIDBServer.h:

Source/WebKit:

* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::createIDBServer):
(WebKit::NetworkProcess::accessToTemporaryFileComplete): Deleted.
* NetworkProcess/NetworkProcess.h:

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

12 files changed:
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/server/IDBBackingStore.h
Source/WebCore/Modules/indexeddb/server/IDBServer.cpp
Source/WebCore/Modules/indexeddb/server/IDBServer.h
Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp
Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.h
Source/WebCore/Modules/indexeddb/server/SQLiteIDBTransaction.cpp
Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.cpp
Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.h
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkProcess.cpp
Source/WebKit/NetworkProcess/NetworkProcess.h

index 2f34d4c..fb6caa9 100644 (file)
@@ -1,3 +1,36 @@
+2019-10-21  Sihui Liu  <sihui_liu@apple.com>
+
+        Remove IDBBackingStoreTemporaryFileHandler
+        https://bugs.webkit.org/show_bug.cgi?id=203128
+
+        Reviewed by Alex Christensen.
+
+        IDBBackingStoreTemporaryFileHandler has only one member function, and implementation of that is to delete
+        files. To keep the same behavior, we can remove this class and delete the temporary files at the places where 
+        objects of this class were used.
+
+        By doing this, we no longer need to pass the NetworkProcess/InProcessIDBServer all the way down to 
+        SQLiteIDBBackingStore that is used only on background thread.
+
+        * Modules/indexeddb/server/IDBBackingStore.h:
+        * Modules/indexeddb/server/IDBServer.cpp:
+        (WebCore::IDBServer::IDBServer::create):
+        (WebCore::IDBServer::IDBServer::IDBServer):
+        (WebCore::IDBServer::IDBServer::createBackingStore):
+        * Modules/indexeddb/server/IDBServer.h:
+        * Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:
+        (WebCore::IDBServer::SQLiteIDBBackingStore::SQLiteIDBBackingStore):
+        * Modules/indexeddb/server/SQLiteIDBBackingStore.h:
+        (WebCore::IDBServer::SQLiteIDBBackingStore::temporaryFileHandler const): Deleted.
+        * Modules/indexeddb/server/SQLiteIDBTransaction.cpp:
+        (WebCore::IDBServer::SQLiteIDBTransaction::moveBlobFilesIfNecessary):
+        (WebCore::IDBServer::SQLiteIDBTransaction::deleteBlobFilesIfNecessary):
+        (WebCore::IDBServer::SQLiteIDBTransaction::abort):
+        * Modules/indexeddb/shared/InProcessIDBServer.cpp:
+        (WebCore::InProcessIDBServer::InProcessIDBServer):
+        (WebCore::InProcessIDBServer::accessToTemporaryFileComplete): Deleted.
+        * Modules/indexeddb/shared/InProcessIDBServer.h:
+
 2019-10-21  Jer Noble  <jer.noble@apple.com>
 
         Add MediaCapabilities support for DolbyVision codecs.
index 73167dd..1332ffd 100644 (file)
@@ -56,12 +56,6 @@ enum class IndexRecordType;
 
 namespace IDBServer {
 
-class IDBBackingStoreTemporaryFileHandler {
-public:
-    virtual ~IDBBackingStoreTemporaryFileHandler() = default;
-    virtual void accessToTemporaryFileComplete(const String& path) = 0;
-};
-
 class IDBBackingStore {
 public:
     virtual ~IDBBackingStore() { RELEASE_ASSERT(!isMainThread()); }
index 428aa80..a8e8567 100644 (file)
 namespace WebCore {
 namespace IDBServer {
 
-Ref<IDBServer> IDBServer::create(PAL::SessionID sessionID, IDBBackingStoreTemporaryFileHandler& fileHandler, QuotaManagerGetter&& quotaManagerGetter)
+Ref<IDBServer> IDBServer::create(PAL::SessionID sessionID, QuotaManagerGetter&& quotaManagerGetter)
 {
-    return adoptRef(*new IDBServer(sessionID, fileHandler, WTFMove(quotaManagerGetter)));
+    return adoptRef(*new IDBServer(sessionID, WTFMove(quotaManagerGetter)));
 }
 
-Ref<IDBServer> IDBServer::create(PAL::SessionID sessionID, const String& databaseDirectoryPath, IDBBackingStoreTemporaryFileHandler& fileHandler, QuotaManagerGetter&& quotaManagerGetter)
+Ref<IDBServer> IDBServer::create(PAL::SessionID sessionID, const String& databaseDirectoryPath, QuotaManagerGetter&& quotaManagerGetter)
 {
-    return adoptRef(*new IDBServer(sessionID, databaseDirectoryPath, fileHandler, WTFMove(quotaManagerGetter)));
+    return adoptRef(*new IDBServer(sessionID, databaseDirectoryPath, WTFMove(quotaManagerGetter)));
 }
 
-IDBServer::IDBServer(PAL::SessionID sessionID, IDBBackingStoreTemporaryFileHandler& fileHandler, QuotaManagerGetter&& quotaManagerGetter)
+IDBServer::IDBServer(PAL::SessionID sessionID, QuotaManagerGetter&& quotaManagerGetter)
     : CrossThreadTaskHandler("IndexedDatabase Server", AutodrainedPoolForRunLoop::Use)
     , m_sessionID(sessionID)
-    , m_backingStoreTemporaryFileHandler(fileHandler)
     , m_quotaManagerGetter(WTFMove(quotaManagerGetter))
 {
 }
 
-IDBServer::IDBServer(PAL::SessionID sessionID, const String& databaseDirectoryPath, IDBBackingStoreTemporaryFileHandler& fileHandler, QuotaManagerGetter&& quotaManagerGetter)
+IDBServer::IDBServer(PAL::SessionID sessionID, const String& databaseDirectoryPath, QuotaManagerGetter&& quotaManagerGetter)
     : CrossThreadTaskHandler("IndexedDatabase Server", AutodrainedPoolForRunLoop::Use)
     , m_sessionID(sessionID)
     , m_databaseDirectoryPath(databaseDirectoryPath)
-    , m_backingStoreTemporaryFileHandler(fileHandler)
     , m_quotaManagerGetter(WTFMove(quotaManagerGetter))
 {
     LOG(IndexedDB, "IDBServer created at path %s", databaseDirectoryPath.utf8().data());
@@ -136,7 +134,7 @@ std::unique_ptr<IDBBackingStore> IDBServer::createBackingStore(const IDBDatabase
     if (databaseDirectoryPath.isEmpty())
         return MemoryIDBBackingStore::create(m_sessionID, identifier);
 
-    return makeUnique<SQLiteIDBBackingStore>(m_sessionID, identifier, databaseDirectoryPath, m_backingStoreTemporaryFileHandler);
+    return makeUnique<SQLiteIDBBackingStore>(m_sessionID, identifier, databaseDirectoryPath);
 }
 
 void IDBServer::openDatabase(const IDBRequestData& requestData)
index 5aa3db5..0755ed9 100644 (file)
@@ -54,15 +54,13 @@ struct IDBGetRecordData;
 
 namespace IDBServer {
 
-class IDBBackingStoreTemporaryFileHandler;
-
 enum class ShouldForceStop : bool { No, Yes };
 
 class IDBServer : public RefCounted<IDBServer>, public CrossThreadTaskHandler, public CanMakeWeakPtr<IDBServer> {
 public:
     using QuotaManagerGetter = WTF::Function<StorageQuotaManager*(PAL::SessionID, const ClientOrigin&)>;
-    static Ref<IDBServer> create(PAL::SessionID, IDBBackingStoreTemporaryFileHandler&, QuotaManagerGetter&&);
-    WEBCORE_EXPORT static Ref<IDBServer> create(PAL::SessionID, const String& databaseDirectoryPath, IDBBackingStoreTemporaryFileHandler&, QuotaManagerGetter&&);
+    static Ref<IDBServer> create(PAL::SessionID, QuotaManagerGetter&&);
+    WEBCORE_EXPORT static Ref<IDBServer> create(PAL::SessionID, const String& databaseDirectoryPath, QuotaManagerGetter&&);
 
     WEBCORE_EXPORT void registerConnection(IDBConnectionToClient&);
     WEBCORE_EXPORT void unregisterConnection(IDBConnectionToClient&);
@@ -126,8 +124,8 @@ public:
     WEBCORE_EXPORT void resume();
 
 private:
-    IDBServer(PAL::SessionID, IDBBackingStoreTemporaryFileHandler&, QuotaManagerGetter&&);
-    IDBServer(PAL::SessionID, const String& databaseDirectoryPath, IDBBackingStoreTemporaryFileHandler&, QuotaManagerGetter&&);
+    IDBServer(PAL::SessionID, QuotaManagerGetter&&);
+    IDBServer(PAL::SessionID, const String& databaseDirectoryPath, QuotaManagerGetter&&);
 
     UniqueIDBDatabase& getOrCreateUniqueIDBDatabase(const IDBDatabaseIdentifier&);
     
@@ -199,7 +197,6 @@ private:
     HashMap<uint64_t, Function<void ()>> m_deleteDatabaseCompletionHandlers;
 
     String m_databaseDirectoryPath;
-    IDBBackingStoreTemporaryFileHandler& m_backingStoreTemporaryFileHandler;
 
     HashMap<ClientOrigin, std::unique_ptr<QuotaUser>> m_quotaUsers;
     QuotaManagerGetter m_quotaManagerGetter;
index 279b315..2ac0ce5 100644 (file)
@@ -229,11 +229,10 @@ static const String& blobFilesTableSchemaAlternate()
     return blobFilesTableSchemaString;
 }
 
-SQLiteIDBBackingStore::SQLiteIDBBackingStore(PAL::SessionID sessionID, const IDBDatabaseIdentifier& identifier, const String& databaseRootDirectory, IDBBackingStoreTemporaryFileHandler& fileHandler)
+SQLiteIDBBackingStore::SQLiteIDBBackingStore(PAL::SessionID sessionID, const IDBDatabaseIdentifier& identifier, const String& databaseRootDirectory)
     : m_sessionID(sessionID)
     , m_identifier(identifier)
     , m_databaseRootDirectory(databaseRootDirectory)
-    , m_temporaryFileHandler(fileHandler)
     , m_serializationContext(IDBSerializationContext::getOrCreateIDBSerializationContext(sessionID))
 {
     m_databaseDirectory = fullDatabaseDirectoryWithUpgrade();
index ca6c336..dc1f9c1 100644 (file)
@@ -50,7 +50,7 @@ class SQLiteIDBCursor;
 class SQLiteIDBBackingStore : public IDBBackingStore {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    SQLiteIDBBackingStore(PAL::SessionID, const IDBDatabaseIdentifier&, const String& databaseRootDirectory, IDBBackingStoreTemporaryFileHandler&);
+    SQLiteIDBBackingStore(PAL::SessionID, const IDBDatabaseIdentifier&, const String& databaseRootDirectory);
     
     ~SQLiteIDBBackingStore() final;
 
@@ -88,8 +88,6 @@ public:
 
     void unregisterCursor(SQLiteIDBCursor&);
 
-    IDBBackingStoreTemporaryFileHandler& temporaryFileHandler() const { return m_temporaryFileHandler; }
-
     IDBError getBlobRecordsForObjectStoreRecord(int64_t objectStoreRecord, Vector<String>& blobURLs, Vector<String>& blobFilePaths);
 
     static String databaseNameFromEncodedFilename(const String&);
@@ -212,8 +210,6 @@ private:
     String m_databaseRootDirectory;
     String m_databaseDirectory;
 
-    IDBBackingStoreTemporaryFileHandler& m_temporaryFileHandler;
-
     Ref<IDBSerializationContext> m_serializationContext;
 };
 
index d212de2..6b87ac2 100644 (file)
@@ -92,7 +92,7 @@ void SQLiteIDBTransaction::moveBlobFilesIfNecessary()
         if (!FileSystem::hardLinkOrCopyFile(entry.first, FileSystem::pathByAppendingComponent(databaseDirectory, entry.second)))
             LOG_ERROR("Failed to link/copy temporary blob file '%s' to location '%s'", entry.first.utf8().data(), FileSystem::pathByAppendingComponent(databaseDirectory, entry.second).utf8().data());
 
-        m_backingStore.temporaryFileHandler().accessToTemporaryFileComplete(entry.first);
+        FileSystem::deleteFile(entry.first);
     }
 
     m_blobTemporaryAndStoredFilenames.clear();
@@ -106,7 +106,8 @@ void SQLiteIDBTransaction::deleteBlobFilesIfNecessary()
     String databaseDirectory = m_backingStore.databaseDirectory();
     for (auto& entry : m_blobRemovedFilenames) {
         String fullPath = FileSystem::pathByAppendingComponent(databaseDirectory, entry);
-        m_backingStore.temporaryFileHandler().accessToTemporaryFileComplete(fullPath);
+
+        FileSystem::deleteFile(fullPath);
     }
 
     m_blobRemovedFilenames.clear();
@@ -115,7 +116,7 @@ void SQLiteIDBTransaction::deleteBlobFilesIfNecessary()
 IDBError SQLiteIDBTransaction::abort()
 {
     for (auto& entry : m_blobTemporaryAndStoredFilenames)
-        m_backingStore.temporaryFileHandler().accessToTemporaryFileComplete(entry.first);
+        FileSystem::deleteFile(entry.first);
 
     m_blobTemporaryAndStoredFilenames.clear();
 
index 807a423..aa4823c 100644 (file)
@@ -78,7 +78,7 @@ static inline IDBServer::IDBServer::QuotaManagerGetter storageQuotaManagerGetter
 }
 
 InProcessIDBServer::InProcessIDBServer(PAL::SessionID sessionID)
-    : m_server(IDBServer::IDBServer::create(sessionID, *this, storageQuotaManagerGetter(*this)))
+    : m_server(IDBServer::IDBServer::create(sessionID, storageQuotaManagerGetter(*this)))
 {
     relaxAdoptionRequirement();
     m_connectionToServer = IDBClient::IDBConnectionToServer::create(*this);
@@ -86,7 +86,7 @@ InProcessIDBServer::InProcessIDBServer(PAL::SessionID sessionID)
 }
 
 InProcessIDBServer::InProcessIDBServer(PAL::SessionID sessionID, const String& databaseDirectoryPath)
-    : m_server(IDBServer::IDBServer::create(sessionID, databaseDirectoryPath, *this, storageQuotaManagerGetter(*this)))
+    : m_server(IDBServer::IDBServer::create(sessionID, databaseDirectoryPath, storageQuotaManagerGetter(*this)))
 {
     relaxAdoptionRequirement();
     m_connectionToServer = IDBClient::IDBConnectionToServer::create(*this);
@@ -460,11 +460,6 @@ void InProcessIDBServer::didGetAllDatabaseNames(uint64_t callbackID, const Vecto
     });
 }
 
-void InProcessIDBServer::accessToTemporaryFileComplete(const String& path)
-{
-    FileSystem::deleteFile(path);
-}
-
 } // namespace WebCore
 
 #endif // ENABLE(INDEXED_DATABASE)
index dbfeac9..cadc812 100644 (file)
@@ -51,7 +51,7 @@ namespace IDBServer {
 class IDBServer;
 }
 
-class InProcessIDBServer final : public IDBClient::IDBConnectionToServerDelegate, public IDBServer::IDBConnectionToClientDelegate, public RefCounted<InProcessIDBServer>, public IDBServer::IDBBackingStoreTemporaryFileHandler {
+class InProcessIDBServer final : public IDBClient::IDBConnectionToServerDelegate, public IDBServer::IDBConnectionToClientDelegate, public RefCounted<InProcessIDBServer> {
 public:
     using IDBClient::IDBConnectionToServerDelegate::weakPtrFactory;
     using WeakValueType = IDBClient::IDBConnectionToServerDelegate::WeakValueType;
@@ -125,8 +125,6 @@ public:
     void ref() override { RefCounted<InProcessIDBServer>::ref(); }
     void deref() override { RefCounted<InProcessIDBServer>::deref(); }
 
-    void accessToTemporaryFileComplete(const String& path) override;
-
     StorageQuotaManager* quotaManager(const ClientOrigin&);
 
 private:
index d478525..5997fbe 100644 (file)
@@ -1,3 +1,15 @@
+2019-10-21  Sihui Liu  <sihui_liu@apple.com>
+
+        Remove IDBBackingStoreTemporaryFileHandler
+        https://bugs.webkit.org/show_bug.cgi?id=203128
+
+        Reviewed by Alex Christensen.
+
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::NetworkProcess::createIDBServer):
+        (WebKit::NetworkProcess::accessToTemporaryFileComplete): Deleted.
+        * NetworkProcess/NetworkProcess.h:
+
 2019-10-21  John Wilander  <wilander@apple.com>
 
         Resource Load Statistics: Update cookie blocking in NetworkStorageSession after first user interaction
index f1cb5d3..000977d 100644 (file)
@@ -2257,7 +2257,7 @@ Ref<IDBServer::IDBServer> NetworkProcess::createIDBServer(PAL::SessionID session
         path = m_idbDatabasePaths.get(sessionID);
     }
 
-    return IDBServer::IDBServer::create(sessionID, path, *this, [this, weakThis = makeWeakPtr(this)](PAL::SessionID sessionID, const auto& origin) -> StorageQuotaManager* {
+    return IDBServer::IDBServer::create(sessionID, path, [this, weakThis = makeWeakPtr(this)](PAL::SessionID sessionID, const auto& origin) -> StorageQuotaManager* {
         if (!weakThis)
             return nullptr;
         return &this->storageQuotaManager(sessionID, origin);
@@ -2306,14 +2306,6 @@ void NetworkProcess::performNextStorageTask()
     task.performTask();
 }
 
-void NetworkProcess::accessToTemporaryFileComplete(const String& path)
-{
-    // We've either hard linked the temporary blob file to the database directory, copied it there,
-    // or the transaction is being aborted.
-    // In any of those cases, we can delete the temporary blob file now.
-    FileSystem::deleteFile(path);
-}
-
 void NetworkProcess::collectIndexedDatabaseOriginsForVersion(const String& path, HashSet<WebCore::SecurityOriginData>& securityOrigins)
 {
     if (path.isEmpty())
index 2faf8c4..387eee2 100644 (file)
@@ -121,11 +121,7 @@ namespace CacheStorage {
 class Engine;
 }
 
-class NetworkProcess : public AuxiliaryProcess, private DownloadManager::Client, public ThreadSafeRefCounted<NetworkProcess>
-#if ENABLE(INDEXED_DATABASE)
-    , public WebCore::IDBServer::IDBBackingStoreTemporaryFileHandler
-#endif
-    , public CanMakeWeakPtr<NetworkProcess>
+class NetworkProcess : public AuxiliaryProcess, private DownloadManager::Client, public ThreadSafeRefCounted<NetworkProcess>, public CanMakeWeakPtr<NetworkProcess>
 {
     WTF_MAKE_NONCOPYABLE(NetworkProcess);
 public:
@@ -284,8 +280,6 @@ public:
 
 #if ENABLE(INDEXED_DATABASE)
     WebCore::IDBServer::IDBServer& idbServer(PAL::SessionID);
-    // WebCore::IDBServer::IDBBackingStoreFileHandler.
-    void accessToTemporaryFileComplete(const String& path) final;
 #endif
 
     void syncLocalStorage(CompletionHandler<void()>&&);