Skip to content

Commit

Permalink
[ WK2 macOS ] http/wpt/cache-storage/cache-quota-after-restart.any.ht…
Browse files Browse the repository at this point in the history
…ml is a flaky failure

https://bugs.webkit.org/show_bug.cgi?id=259677
rdar://113189461

Reviewed by Youenn Fablet.

Web process sends NetworkConnectionToWebProcess::UpdateQuotaBasedOnSpaceUsageForTesting to network process to reset
quota based on usage. In the test, this happens before cache put operation (i.e. web process sends
NetworkStorageManager::CacheStoragePutRecords to network process), and it expects quota resetting to happen before put
operation.

However, NetworkConnectionToWebProcess messages are dispatched to the main thread of network process from IPC
thread, and NetworkStorageManager messages are dispatched to storage queue, which means put operation can happen
before quota is reset. In the failure case, here is what happens:
1. Network process main thread handles UpdateQuotaBasedOnSpaceUsageForTesting message, and dispatch a task to storage
queue.
2. Network process storage queue handles CacheStoragePutRecords messsage, starts put operation, performs quota check and
decides to ask UI process for a quota increase (with existing quota, e.g. 810KB).
3. Network process storage queue performs dipatched task , resetting usage to null and quota to initialQuoata
(e.g. 400KB), so that next quota check will fetch usage and update quota based on usage.
4. Network process receives new quota from UI process, updates its quota using new quota, and performs quota check again.
Since the test disallows quota increase, new quota will be the same as quota passed to UI process (i.e. 810KB). Then in
quota check, network process updates quota based on usage and defaultQuotaStep, which is 10% of current quota, i.e. 81KB.
However, the tests expects the defaultQuotaStep to be 40KB based on initialQuota.

To fix this issue, the patch moves the message to quota resetting message to NetworkStorageManager to ensure the
ordering of tasks. Now the quota resetting will always happen in the first quota check of put operation, and it uses
initialQuota for defaultQuotaStep.

* LayoutTests/platform/mac-wk2/TestExpectations:
* Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::updateQuotaBasedOnSpaceUsageForTesting): Deleted.
* Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h:
* Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in:
* Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp:
(WebKit::NetworkStorageManager::resetQuotaUpdatedBasedOnUsageForTesting):
* Source/WebKit/NetworkProcess/storage/NetworkStorageManager.messages.in:
* Source/WebKit/WebProcess/Cache/WebCacheStorageConnection.cpp:
(WebKit::WebCacheStorageConnection::updateQuotaBasedOnSpaceUsage):

Canonical link: https://commits.webkit.org/267748@main
  • Loading branch information
szewai committed Sep 7, 2023
1 parent a5cc5d4 commit 24e3d35
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 17 deletions.
2 changes: 1 addition & 1 deletion LayoutTests/platform/mac-wk2/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@ http/wpt/cache-storage/cache-remove-twice.html [ Slow ]
http/wpt/cache-storage/cache-open-delete-in-parallel.https.html [ Slow ]
http/wpt/cache-storage/cache-quota-add.any.html [ Slow ]
http/wpt/cache-storage/quota-third-party.https.html [ Slow ]
webkit.org/b/259677 http/wpt/cache-storage/cache-quota-after-restart.any.html [ Slow Pass Failure ]
http/wpt/cache-storage/cache-quota-after-restart.any.html [ Slow ]

webkit.org/b/181502 swipe/pushstate-with-manual-scrollrestoration.html [ Failure ]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,13 +430,6 @@ bool NetworkConnectionToWebProcess::didReceiveSyncMessage(IPC::Connection& conne
return false;
}

void NetworkConnectionToWebProcess::updateQuotaBasedOnSpaceUsageForTesting(ClientOrigin&& origin)
{
NETWORK_PROCESS_MESSAGE_CHECK(allowTestOnlyIPC());
if (auto* session = m_networkProcess->networkSession(sessionID()))
session->storageManager().resetQuotaUpdatedBasedOnUsageForTesting(WTFMove(origin));
}

void NetworkConnectionToWebProcess::didClose(IPC::Connection& connection)
{
#if ENABLE(SERVICE_WORKER)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ class NetworkConnectionToWebProcess
void setCaptureExtraNetworkLoadMetricsEnabled(bool);

void createSocketChannel(const WebCore::ResourceRequest&, const String& protocol, WebCore::WebSocketIdentifier, WebPageProxyIdentifier, std::optional<WebCore::FrameIdentifier>, std::optional<WebCore::PageIdentifier>, const WebCore::ClientOrigin&, bool hadMainFrameMainResourcePrivateRelayed, bool allowPrivacyProxy, OptionSet<WebCore::AdvancedPrivacyProtections>, WebCore::ShouldRelaxThirdPartyCookieBlocking, WebCore::StoredCredentialsPolicy);
void updateQuotaBasedOnSpaceUsageForTesting(WebCore::ClientOrigin&&);

void establishSharedWorkerServerConnection();
void unregisterSharedWorkerConnection();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ messages -> NetworkConnectionToWebProcess LegacyReceiver {
ConnectToRTCDataChannelRemoteSource(struct WebCore::RTCDataChannelIdentifier source, struct WebCore::RTCDataChannelIdentifier handler) -> (std::optional<bool> result)
#endif

[EnabledIf='allowTestOnlyIPC()'] UpdateQuotaBasedOnSpaceUsageForTesting(struct WebCore::ClientOrigin origin)
CreateNewMessagePortChannel(struct WebCore::MessagePortIdentifier port1, struct WebCore::MessagePortIdentifier port2)
EntangleLocalPortInThisProcessToRemote(struct WebCore::MessagePortIdentifier local, struct WebCore::MessagePortIdentifier remote)
MessagePortDisentangled(struct WebCore::MessagePortIdentifier local)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1230,13 +1230,10 @@ void NetworkStorageManager::resetQuotaForTesting(CompletionHandler<void()>&& com

void NetworkStorageManager::resetQuotaUpdatedBasedOnUsageForTesting(WebCore::ClientOrigin&& origin)
{
ASSERT(RunLoop::isMain());
assertIsCurrent(workQueue());

m_queue->dispatch([this, protectedThis = Ref { *this }, origin = crossThreadCopy(WTFMove(origin))]() mutable {
assertIsCurrent(workQueue());
if (auto manager = m_originStorageManagers.get(origin))
manager->quotaManager().resetQuotaUpdatedBasedOnUsageForTesting();
});
if (auto manager = m_originStorageManagers.get(origin))
manager->quotaManager().resetQuotaUpdatedBasedOnUsageForTesting();
}

#if PLATFORM(IOS_FAMILY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,6 @@
CacheStoragePutRecords(WebCore::DOMCacheIdentifier cacheIdentifier, Vector<WebCore::DOMCacheEngine::CrossThreadRecord> records) -> (WebCore::DOMCacheEngine::RecordIdentifiersOrError result)
CacheStorageClearMemoryRepresentation(struct WebCore::ClientOrigin origin) -> (std::optional<WebCore::DOMCacheEngine::Error> error)
CacheStorageRepresentation() -> (String representation)

ResetQuotaUpdatedBasedOnUsageForTesting(struct WebCore::ClientOrigin origin)
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ void WebCacheStorageConnection::engineRepresentation(CompletionHandler<void(cons

void WebCacheStorageConnection::updateQuotaBasedOnSpaceUsage(const WebCore::ClientOrigin& origin)
{
connection().send(Messages::NetworkConnectionToWebProcess::UpdateQuotaBasedOnSpaceUsageForTesting(origin), 0);
connection().send(Messages::NetworkStorageManager::ResetQuotaUpdatedBasedOnUsageForTesting(origin), 0);
}

void WebCacheStorageConnection::networkProcessConnectionClosed()
Expand Down

0 comments on commit 24e3d35

Please sign in to comment.