Delay initialization of quota users until the first quota request
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Apr 2019 16:30:58 +0000 (16:30 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Apr 2019 16:30:58 +0000 (16:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196467

Reviewed by Chris Dumez.

Source/WebCore:

Instead of triggering initialization of each user when being added,
delay initialization until the first call to requestSpace with a non zero task size.
This will make sure we do not load Cache API information in memory or check for
IDB space until actually necessary.

To implement that, move from a HashSet of being initialized users to a HashMap where the key is user and
the value is the user initialization state.

When removing a user, delay the call to processPendingRequest so that a synchronous call to addUser
can be taken into consideration.

This unflakes some Cache API tests as these tests do clear the Cache API and check for the clearing result.
Clearing the caches triggers a removeUser/addUser dance which then triggers initialization of the Caches structure.

Covered by existing tests.

* storage/StorageQuotaManager.cpp:
(WebCore::StorageQuotaManager::initializeUsersIfNeeded):
(WebCore::StorageQuotaManager::askUserToInitialize):
(WebCore::StorageQuotaManager::addUser):
(WebCore::StorageQuotaManager::requestSpace):
* storage/StorageQuotaManager.h:

LayoutTests:

Unflake cache storage tests.

* TestExpectations:
* platform/mac-wk2/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/storage/StorageQuotaManager.cpp
Source/WebCore/storage/StorageQuotaManager.h

index 884a5ba..c17aa9d 100644 (file)
@@ -1,3 +1,15 @@
+2019-04-10  Youenn Fablet  <youenn@apple.com>
+
+        Delay initialization of quota users until the first quota request
+        https://bugs.webkit.org/show_bug.cgi?id=196467
+
+        Reviewed by Chris Dumez.
+
+        Unflake cache storage tests.
+
+        * TestExpectations:
+        * platform/mac-wk2/TestExpectations:
+
 2019-04-10  Philippe Normand  <pnormand@igalia.com>
 
         there is no vp8 support in youtube.com/html5 page with libwebkit2gtk 2.24 (MSE enabled)
index b327044..4def0a0 100644 (file)
@@ -2229,7 +2229,6 @@ webkit.org/b/182311 imported/w3c/web-platform-tests/service-workers/service-work
 webkit.org/b/90980 fast/forms/textarea/textarea-state-restore.html [ Pass Timeout ]
 
 webkit.org/b/182928 http/tests/cache-storage/cache-representation.https.html [ Pass Failure ]
-webkit.org/b/193976 http/tests/cache-storage/cache-clearing-origin.https.html [ Pass Failure ]
 
 webkit.org/b/116621 fast/replaced/preferred-widths.html [ Pass Failure ]
 
index 7523298..2c0e580 100644 (file)
@@ -854,8 +854,6 @@ webkit.org/b/183860 [ Sierra Release ] http/wpt/service-workers/third-party-regi
 
 webkit.org/b/184245 http/tests/workers/service/service-worker-cache-api.https.html [ Pass Failure ]
 
-webkit.org/b/177380 http/tests/cache-storage/cache-records-persistency.https.html [ Pass Failure ]
-
 webkit.org/b/184937 transitions/opacity-transition-zindex.html [ Skip ]
 
 webkit.org/b/186362 [ Release ] http/tests/resourceLoadStatistics/prevalent-resource-with-user-interaction.html [ Skip ]
@@ -925,8 +923,6 @@ webkit.org/b/194253 scrollingcoordinator/scrolling-tree/fixed-inside-frame.html
 
 webkit.org/b/194916 fast/mediastream/MediaStream-video-element.html [ Pass Failure ]
 
-webkit.org/b/196228 http/tests/cache-storage/cache-clearing-origin.https.html [ Pass Failure ]
-
 webkit.org/b/195719 fast/events/wheel-event-destroys-overflow.html [ Pass Timeout ]
 webkit.org/b/195719 fast/events/wheelevent-mousewheel-interaction.html  [ Pass Timeout ]
 webkit.org/b/195719 fast/events/wheel-event-destroys-frame.html  [ Pass Timeout ]
index 66147c5..e060dd7 100644 (file)
@@ -1,3 +1,33 @@
+2019-04-10  Youenn Fablet  <youenn@apple.com>
+
+        Delay initialization of quota users until the first quota request
+        https://bugs.webkit.org/show_bug.cgi?id=196467
+
+        Reviewed by Chris Dumez.
+
+        Instead of triggering initialization of each user when being added,
+        delay initialization until the first call to requestSpace with a non zero task size.
+        This will make sure we do not load Cache API information in memory or check for
+        IDB space until actually necessary.
+
+        To implement that, move from a HashSet of being initialized users to a HashMap where the key is user and
+        the value is the user initialization state.
+
+        When removing a user, delay the call to processPendingRequest so that a synchronous call to addUser
+        can be taken into consideration.
+
+        This unflakes some Cache API tests as these tests do clear the Cache API and check for the clearing result.
+        Clearing the caches triggers a removeUser/addUser dance which then triggers initialization of the Caches structure.
+
+        Covered by existing tests.
+
+        * storage/StorageQuotaManager.cpp:
+        (WebCore::StorageQuotaManager::initializeUsersIfNeeded):
+        (WebCore::StorageQuotaManager::askUserToInitialize):
+        (WebCore::StorageQuotaManager::addUser):
+        (WebCore::StorageQuotaManager::requestSpace):
+        * storage/StorageQuotaManager.h:
+
 2019-04-10  Philippe Normand  <pnormand@igalia.com>
 
         there is no vp8 support in youtube.com/html5 page with libwebkit2gtk 2.24 (MSE enabled)
index d2884af..9933c0e 100644 (file)
@@ -53,11 +53,26 @@ void StorageQuotaManager::updateQuotaBasedOnSpaceUsage()
     m_quota = std::max(m_quota, defaultQuotaStep * ((spaceUsage() / defaultQuotaStep) + 1));
 }
 
-void StorageQuotaManager::addUser(StorageQuotaUser& user)
+void StorageQuotaManager::initializeUsersIfNeeded()
+{
+    if (m_pendingInitializationUsers.isEmpty())
+        return;
+
+    Vector<StorageQuotaUser*> usersToInitialize;
+    for (auto& keyValue : m_pendingInitializationUsers) {
+        if (keyValue.value == WhenInitializedCalled::No) {
+            keyValue.value = WhenInitializedCalled::Yes;
+            usersToInitialize.append(keyValue.key);
+        }
+    }
+    for (auto* user : usersToInitialize) {
+        if (m_pendingInitializationUsers.contains(user))
+            askUserToInitialize(*user);
+    }
+}
+
+void StorageQuotaManager::askUserToInitialize(StorageQuotaUser& user)
 {
-    ASSERT(!m_pendingInitializationUsers.contains(&user));
-    ASSERT(!m_users.contains(&user));
-    m_pendingInitializationUsers.add(&user);
     user.whenInitialized([this, &user, weakThis = makeWeakPtr(this)]() {
         if (!weakThis)
             return;
@@ -73,6 +88,16 @@ void StorageQuotaManager::addUser(StorageQuotaUser& user)
     });
 }
 
+void StorageQuotaManager::addUser(StorageQuotaUser& user)
+{
+    ASSERT(!m_pendingInitializationUsers.contains(&user));
+    ASSERT(!m_users.contains(&user));
+    m_pendingInitializationUsers.add(&user, WhenInitializedCalled::No);
+
+    if (!m_pendingRequests.isEmpty())
+        askUserToInitialize(user);
+}
+
 bool StorageQuotaManager::shouldAskForMoreSpace(uint64_t spaceIncrease) const
 {
     if (!spaceIncrease)
@@ -85,13 +110,34 @@ void StorageQuotaManager::removeUser(StorageQuotaUser& user)
 {
     ASSERT(m_users.contains(&user) || m_pendingInitializationUsers.contains(&user));
     m_users.remove(&user);
-    if (m_pendingInitializationUsers.remove(&user) && m_pendingInitializationUsers.isEmpty())
-        processPendingRequests({ }, ShouldDequeueFirstPendingRequest::No);
+    if (m_pendingInitializationUsers.remove(&user) && m_pendingInitializationUsers.isEmpty()) {
+        // When being cleared, quota users may remove themselves and add themselves to trigger reinitialization.
+        // Let's wait for addUser to be called before processing pending requests.
+        callOnMainThread([this, weakThis = makeWeakPtr(this)] {
+            if (!weakThis)
+                return;
+
+            if (m_pendingInitializationUsers.isEmpty())
+                this->processPendingRequests({ }, ShouldDequeueFirstPendingRequest::No);
+        });
+    }
 }
 
 void StorageQuotaManager::requestSpace(uint64_t spaceIncrease, RequestCallback&& callback)
 {
-    if (!m_pendingRequests.isEmpty() || !m_pendingInitializationUsers.isEmpty()) {
+    if (!m_pendingRequests.isEmpty()) {
+        m_pendingRequests.append({ spaceIncrease, WTFMove(callback) });
+        return;
+    }
+
+    if (!spaceIncrease) {
+        callback(Decision::Grant);
+        return;
+    }
+
+    initializeUsersIfNeeded();
+
+    if (!m_pendingInitializationUsers.isEmpty()) {
         m_pendingRequests.append({ spaceIncrease, WTFMove(callback) });
         return;
     }
@@ -101,6 +147,7 @@ void StorageQuotaManager::requestSpace(uint64_t spaceIncrease, RequestCallback&&
         askForMoreSpace(spaceIncrease);
         return;
     }
+
     callback(Decision::Grant);
 }
 
index 4935ad9..9f64633 100644 (file)
@@ -28,6 +28,7 @@
 #include "ClientOrigin.h"
 #include <wtf/CompletionHandler.h>
 #include <wtf/Deque.h>
+#include <wtf/HashMap.h>
 #include <wtf/HashSet.h>
 #include <wtf/WeakPtr.h>
 
@@ -66,6 +67,9 @@ private:
     bool shouldAskForMoreSpace(uint64_t spaceIncrease) const;
     void askForMoreSpace(uint64_t spaceIncrease);
 
+    void initializeUsersIfNeeded();
+    void askUserToInitialize(StorageQuotaUser&);
+
     enum class ShouldDequeueFirstPendingRequest { No, Yes };
     void processPendingRequests(Optional<uint64_t>, ShouldDequeueFirstPendingRequest);
 
@@ -73,7 +77,9 @@ private:
 
     bool m_isWaitingForSpaceIncreaseResponse { false };
     SpaceIncreaseRequester m_spaceIncreaseRequester;
-    HashSet<const StorageQuotaUser*> m_pendingInitializationUsers;
+
+    enum class WhenInitializedCalled { No, Yes };
+    HashMap<StorageQuotaUser*, WhenInitializedCalled> m_pendingInitializationUsers;
     HashSet<const StorageQuotaUser*> m_users;
 
     struct PendingRequest {