[webkit-changes] [WebKit/WebKit] 6b5f40: Regression(266170 at main) Crash under CachedRawResou...

[Chris Dumez]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 6b5f40310886172cd02969c7cd5b49d7c1e21096
      https://github.com/WebKit/WebKit/commit/6b5f40310886172cd02969c7cd5b49d7c1e21096
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2023-07-21 (Fri, 21 Jul 2023)

  Changed paths:
    A LayoutTests/http/tests/workers/memory-cache-crash2-expected.txt
    A LayoutTests/http/tests/workers/memory-cache-crash2.html
    M Source/WebCore/loader/cache/CachedResource.cpp
    M Source/WebCore/loader/cache/CachedResource.h
    M Source/WebCore/loader/cache/MemoryCache.cpp

  Log Message:
  -----------
  Regression(266170 at main) Crash under CachedRawResource::switchClientsToRevalidatedResource()
https://bugs.webkit.org/show_bug.cgi?id=259401
rdar://112663008

Reviewed by Brent Fulgham.

In 266170 at main, I updated MemoryCache::revalidationSucceeded() to deal with the fact
that there may already be a resource in the MemoryCache with the same URL when the
resource revalidation finishes. I dealt with this by assuming that the resource
already in the cache was good enough and I ignored the revalidated resource, and then
transferred the clients of the revalidation resource to the resource that is already
in the cache.

However, it turns out that this isn't safe to do because the resource already in the
cache may have a different type (even though it has the same URL). In the included
test, for example, the same URL is loaded both as a RawResource and a FontResource.
This would lead to crashes later on in switchClientsToRevalidatedResource().

To address the issue, I am reverting 266170 at main and dealing with the original issue
in a simpler way. Upon successful revalidation, if there is already a resource in the
memory cache with the same URL, we now remove it from the cache before proceeding.

This is much safer and matches what would happen if you tried to load URL1 as a
RawResource and then load URL2 as a FontResource. Our MemoryCache logic would remove
the existing RawResource in the cache and create a new CachedResource of FontResource
type.

* LayoutTests/http/tests/workers/memory-cache-crash2.html: Added.
* LayoutTests/http/tests/workers/memory-cache-crash2-expected.txt: Added.
* Source/WebCore/loader/cache/CachedResource.cpp:
(WebCore::CachedResource::replaceResourceToRevalidate): Deleted.
* Source/WebCore/loader/cache/CachedResource.h:
* Source/WebCore/loader/cache/MemoryCache.cpp:
(WebCore::MemoryCache::revalidationSucceeded):

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