Changeset 290233 in webkit


Ignore:
Timestamp:
Feb 20, 2022 3:09:55 PM (5 months ago)
Author:
sihui_liu@apple.com
Message:

Origin file may not be created after migrating data to general storage directory
​https://bugs.webkit.org/show_bug.cgi?id=236901

Reviewed by Chris Dumez.

Source/WebKit:

In r289081, we started writing origin file asynchronouly (we used to write it at creation of
OriginStorageManager), to avoid delay in replying sync WebStorage message. In the async task, we check if the
OriginStorageManager still exists. If it does not, we will not write origin file, because the
OriginStorageManager may be removed after origin data gets deleted, and there's no need to add new origin file.
It's also possible that the OriginStorageManager is created for website data fetch, and removed immediately
after the fetch task is done (see NetworkStorageManager::fetchDataFromDisk). We used to not store
new data in general storage directory during data fetch, so it's fine to not write origin file in this case.

With r289878, existing data can be migrated to general storage directory at data fetch, so it is important to
ensure we write origin to file; otherwise, we will lose track of the origin. To solve this, let's restore the
old behavior that synchronouly writes origin file at creation of OriginStorageManager. To solve the delay issue,
in the sync message handler (NetworkStorageManager::connectToStorageArea), we specifically ask to not write
origin file, and manually write origin file after message is replied.

Updated existing API test to add test coverage.

  • NetworkProcess/storage/NetworkStorageManager.cpp:

(WebKit::NetworkStorageManager::localOriginStorageManager):
(WebKit::NetworkStorageManager::connectToStorageArea):

  • NetworkProcess/storage/NetworkStorageManager.h:

Tools:

  • TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:

(TEST):

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r290227 r290233  
     12022-02-20  Sihui Liu  <sihui_liu@apple.com>
     2
     3        Origin file may not be created after migrating data to general storage directory
     4        https://bugs.webkit.org/show_bug.cgi?id=236901
     5
     6        Reviewed by Chris Dumez.
     7
     8        In r289081, we started writing origin file asynchronouly (we used to write it at creation of
     9        OriginStorageManager), to avoid delay in replying sync WebStorage message. In the async task, we check if the
     10        OriginStorageManager still exists. If it does not, we will not write origin file, because the
     11        OriginStorageManager may be removed after origin data gets deleted, and there's no need to add new origin file.
     12        It's also possible that the OriginStorageManager is created for website data fetch, and removed immediately
     13        after the fetch task is done (see NetworkStorageManager::fetchDataFromDisk). We used to not store
     14        new data in general storage directory during data fetch, so it's fine to not write origin file in this case.
     15
     16        With r289878, existing data can be migrated to general storage directory at data fetch, so it is important to
     17        ensure we write origin to file; otherwise, we will lose track of the origin. To solve this, let's restore the
     18        old behavior that synchronouly writes origin file at creation of OriginStorageManager. To solve the delay issue,
     19        in the sync message handler (NetworkStorageManager::connectToStorageArea), we specifically ask to not write
     20        origin file, and manually write origin file after message is replied.
     21
     22        Updated existing API test to add test coverage.
     23
     24        * NetworkProcess/storage/NetworkStorageManager.cpp:
     25        (WebKit::NetworkStorageManager::localOriginStorageManager):
     26        (WebKit::NetworkStorageManager::connectToStorageArea):
     27        * NetworkProcess/storage/NetworkStorageManager.h:
     28
    1292022-02-20  Brandon Stewart  <brandonstewart@apple.com>
    230

  • trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp

    r290214 r290233  
    228228}
    229229
    230 OriginStorageManager& NetworkStorageManager::localOriginStorageManager(const WebCore::ClientOrigin& origin)
     230OriginStorageManager& NetworkStorageManager::localOriginStorageManager(const WebCore::ClientOrigin& origin, ShouldWriteOriginFile shouldWriteOriginFile)
    231231{
    232232    ASSERT(!RunLoop::isMain());
    … …  
    234234    return *m_localOriginStorageManagers.ensure(origin, [&] {
    235235        auto originDirectory = originDirectoryPath(m_path, origin, m_salt);
    236         // Write origin file asynchronously to avoid delay in replying sync messages for Web Storage API.
    237         if (!originDirectory.isEmpty()) {
    238             m_queue->dispatch([this, protectedThis = Ref { *this }, origin, originDirectory]() mutable {
    239                 if (!m_localOriginStorageManagers.contains(origin))
    240                     return;
    241 
    242                 writeOriginToFileIfNecessary(originFilePath(originDirectory), origin);
    243             });
    244         }
     236        if (!originDirectory.isEmpty() && shouldWriteOriginFile == ShouldWriteOriginFile::Yes)
     237            writeOriginToFileIfNecessary(originFilePath(originDirectory), origin);
    245238        auto localStoragePath = LocalStorageManager::localStorageFilePath(m_customLocalStoragePath, origin);
    246239        auto idbStoragePath = IDBStorageManager::idbStorageOriginDirectory(m_customIDBStoragePath, origin);
    … …  
    730723
    731724    auto connectionIdentifier = connection.uniqueID();
    732     auto& originStorageManager = localOriginStorageManager(origin);
     725    bool willCreateOriginStorageManager = !m_localOriginStorageManagers.contains(origin);
     726    // Avoid delay in replying sync message by writing origin file after replying message.
     727    auto& originStorageManager = localOriginStorageManager(origin, ShouldWriteOriginFile::No);
     728    auto writeOriginFile = makeScopeExit([&] {
     729        if (!willCreateOriginStorageManager)
     730            return;
     731
     732        if (auto originDirectory = originDirectoryPath(m_path, origin, m_salt); !originDirectory.isEmpty())
     733            writeOriginToFileIfNecessary(originFilePath(originDirectory), origin);
     734    });
     735
    733736    StorageAreaIdentifier resultIdentifier;
    734737    switch (type) {

  • trunk/Source/WebKit/NetworkProcess/storage/NetworkStorageManager.h

    r290033 r290233  
    9898    NetworkStorageManager(PAL::SessionID, IPC::Connection::UniqueID, const String& path, const String& customLocalStoragePath, const String& customIDBStoragePath, const String& customCacheStoragePath, uint64_t defaultOriginQuota, uint64_t defaultThirdPartyOriginQuota, bool shouldUseCustomPaths);
    9999    ~NetworkStorageManager();
    100     OriginStorageManager& localOriginStorageManager(const WebCore::ClientOrigin&);
     100    enum class ShouldWriteOriginFile : bool { No, Yes };
     101    OriginStorageManager& localOriginStorageManager(const WebCore::ClientOrigin&, ShouldWriteOriginFile = ShouldWriteOriginFile::Yes);
    101102    bool removeOriginStorageManagerIfPossible(const WebCore::ClientOrigin&);
    102103    void deleteOriginDirectoryIfPossible(const WebCore::ClientOrigin&);

  • trunk/Tools/ChangeLog

    r290227 r290233  
     12022-02-20  Sihui Liu  <sihui_liu@apple.com>
     2
     3        Origin file may not be created after migrating data to general storage directory
     4        https://bugs.webkit.org/show_bug.cgi?id=236901
     5
     6        Reviewed by Chris Dumez.
     7
     8        * TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
     9        (TEST):
     10
    1112022-02-20  Brandon Stewart  <brandonstewart@apple.com>
    212

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm

    r290147 r290233  
    963963    NSURL *webkitGeneralLocalStorageDirectory = [webkitGeneralStorageDirectory URLByAppendingPathComponent:@"LocalStorage"];
    964964    NSURL *webKitGeneralLocalStorageFile = [webkitGeneralLocalStorageDirectory URLByAppendingPathComponent:@"localstorage.sqlite3"];
    965     NSURL *appleGeneralLocalStorageFile = [generalStorageDirectory URLByAppendingPathComponent:@"xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI/xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI/LocalStorage/localstorage.sqlite3"];
    966     NSURL *appleGeneralIndexedDBDatabaseFile = [generalStorageDirectory URLByAppendingPathComponent:@"xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI/xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI/IndexedDB/620AD548000B0A49C02D2E8D75C32E0F85697B54DF86146ECAE360DE104AB3F9/IndexedDB.sqlite3"];
     965    NSURL *appleGeneralStorageDirectory = [generalStorageDirectory URLByAppendingPathComponent:@"xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI/xKK8XQeHebxhSzMyotOOtNKp2jWZ4l_CEN4WggYYXbI"];
     966    NSURL *appleGeneralOriginFile = [appleGeneralStorageDirectory URLByAppendingPathComponent:@"origin"];
     967    NSURL *appleGeneralLocalStorageFile = [appleGeneralStorageDirectory URLByAppendingPathComponent:@"LocalStorage/localstorage.sqlite3"];
     968    NSURL *appleGeneralIndexedDBDatabaseFile = [appleGeneralStorageDirectory URLByAppendingPathComponent:@"IndexedDB/620AD548000B0A49C02D2E8D75C32E0F85697B54DF86146ECAE360DE104AB3F9/IndexedDB.sqlite3"];
    967969
    968970    // Copy apple.com files to custom path.
    … …  
    990992    // Ensure data is fetched from both custom path and new path.
    991993    auto dataTypes = [NSSet setWithObjects:WKWebsiteDataTypeLocalStorage, WKWebsiteDataTypeIndexedDBDatabases, nil];
     994    auto sortFunction = ^(WKWebsiteDataRecord *record1, WKWebsiteDataRecord *record2) {
     995        return [record1.displayName compare:record2.displayName];
     996    };
    992997    done = false;
    993998    [websiteDataStore fetchDataRecordsOfTypes:dataTypes completionHandler:^(NSArray<WKWebsiteDataRecord *> *records) {
    994999        EXPECT_EQ(records.count, 2u);
    995         auto sortFunction = ^(WKWebsiteDataRecord *record1, WKWebsiteDataRecord *record2) {
    996             return [record1.displayName compare:record2.displayName];
    997         };
    9981000        auto sortedRecords = [records sortedArrayUsingComparator:sortFunction];
    9991001        auto appleRecord = [sortedRecords objectAtIndex:0];
    … …  
    10101012    EXPECT_FALSE([fileManager fileExistsAtPath:appleCustomLocalStorageFile.path]);
    10111013    EXPECT_FALSE([fileManager fileExistsAtPath:appleCustomIndexedDBDatabaseFile.path]);
     1014    EXPECT_TRUE([fileManager fileExistsAtPath:appleGeneralOriginFile.path]);
    10121015    EXPECT_TRUE([fileManager fileExistsAtPath:appleGeneralLocalStorageFile.path]);
    10131016    EXPECT_TRUE([fileManager fileExistsAtPath:appleGeneralIndexedDBDatabaseFile.path]);
     1017
     1018    done = false;
     1019    [websiteDataStore fetchDataRecordsOfTypes:dataTypes completionHandler:^(NSArray<WKWebsiteDataRecord *> *records) {
     1020        EXPECT_EQ(records.count, 2u);
     1021        auto sortedRecords = [records sortedArrayUsingComparator:sortFunction];
     1022        EXPECT_WK_STREQ(@"apple.com", [[sortedRecords objectAtIndex:0] displayName]);
     1023        done = true;
     1024    }];
     1025    TestWebKitAPI::Util::run(&done);
    10141026}
    10151027
Note: See TracChangeset for help on using the changeset viewer.