Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 7fa5066e9d64de34b5fa1c46bf3b8fecf3957fc2
      
https://github.com/WebKit/WebKit/commit/7fa5066e9d64de34b5fa1c46bf3b8fecf3957fc2
  Author: Youenn Fablet <youe...@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


_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to