[webkit-changes] [WebKit/WebKit] 7fa506: REGRESSION (STP 161): cache-storage 100+ test fail...

[youennf]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 7fa5066e9d64de34b5fa1c46bf3b8fecf3957fc2
      https://github.com/WebKit/WebKit/commit/7fa5066e9d64de34b5fa1c46bf3b8fecf3957fc2
  Author: Youenn Fablet <youennf at gmail.com>
  Date:   2023-01-26 (Thu, 26 Jan 2023)

  Changed paths:
    A LayoutTests/http/wpt/cache-storage/cache-matchAll.https.any-expected.txt
    A LayoutTests/http/wpt/cache-storage/cache-matchAll.https.any.html
    A LayoutTests/http/wpt/cache-storage/cache-matchAll.https.any.js
    M Source/WebCore/platform/network/ResourceResponseBase.cpp
    M Source/WebCore/platform/network/ResourceResponseBase.h
    M Source/WebKit/NetworkProcess/storage/CacheStorageMemoryStore.cpp
    M Source/WebKit/NetworkProcess/storage/CacheStorageRecord.h

  Log Message:
  -----------
  REGRESSION (STP 161): cache-storage 100+ test failures on WPT
https://bugs.webkit.org/show_bug.cgi?id=250590
rdar://problem/104237218

Reviewed by Chris Dumez.

This is a regression of switching to the new CacheStorage backend in ephemeral sessions.
WPT is using ephemeral sessions which is the only code path to store ResponseResourceBase::CrossThreadData in CacheStorageMemoryStore.
It is manipulated in a WorkQueue where several threads might be involved, which is not working well with AtomString that are bound by thread.
In particular, when we create a ResponseResourceBase from a CacheStorageMemoryStore record CrossThreadData, the CrossThreadData String will be change to an AtomString.
To prevent this, we make sure to isolate copy all Strings inside ResponseResourceBase::CrossThreadData::isolatedCopy() and create a new CrossThreadData from CacheStorageMemoryStore records.

We isolate copy all strings as it is safer in general, the result of ResponseResourceBase::CrossThreadData::copy() could be sent to another thread for instance.
There should be no perf impact except on CacheStorage in ephemeral sessions since ResponseResourceBase::CrossThreadData::isolatedCopy() is only used there.

We also fix ResourceResponseBase::CrossThreadData copy as it was missing statusText copy.
And we also add a bunch of WTFMove as a small improvement in ResourceResponseBase::fromCrossThreadData.

We remove CacheStorageRecord::copy and put it as a static function in CacheStorageMemoryStore as it is only used there and is very specific in that it isolate copy the ResponseResourceBase::CrossThreadData but not other fields.

To cover this patch, we copy WPT cache-matchAll.https.any.html in http/wpt to run it with an ephemeral session.

* LayoutTests/http/wpt/cache-storage/cache-matchAll.https.any-expected.txt: Added.
* LayoutTests/http/wpt/cache-storage/cache-matchAll.https.any.html: Added.
* LayoutTests/http/wpt/cache-storage/cache-matchAll.https.any.js: Added.
* Source/WebCore/platform/network/ResourceResponseBase.cpp:
(WebCore::ResourceResponseBase::CrossThreadData::copy const):
(WebCore::ResourceResponseBase::fromCrossThreadData):
* Source/WebKit/NetworkProcess/storage/CacheStorageMemoryStore.cpp:
(WebKit::copyCacheStorageRecord):
(WebKit::CacheStorageMemoryStore::readAllRecords):
(WebKit::CacheStorageMemoryStore::readRecords):
* Source/WebKit/NetworkProcess/storage/CacheStorageRecord.h:
(WebKit::CacheStorageRecord::copy const): Deleted.

Canonical link: https://commits.webkit.org/259418@main