Bug 306402 – CheckedPtr crash in NetworkResourceLoader::contentFilterDidBlock() when ContentFilter is deleted during delayed async callback

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[David Kilzer (:ddkilzer)]

CheckedPtr detection crash occurs when ContentFilter object is destroyed while an async unblock callback is pending in NetworkResourceLoader::contentFilterDidBlock(). 

The lambda in requestUnblockAsync() captures a CheckedPtr<ContentFilter>, but when NetworkResourceLoader is destroyed (which owns m_contentFilter via unique_ptr), the CheckedPtr becomes invalid. When the delayed callback eventually fires and the lambda is destroyed, CheckedPtr detects the freed object and crashes.

Stack trace:
```
Thread 0 Crashed::  Dispatch queue: com.apple.main-thread:
0   WebKit:  WTFCrashWithInfo()
1   WebKit:  decrementCheckedPtrCount()
2   WebKit:  derefIfNotNull()
3   WebKit:  ~CheckedPtr()
4   WebKit:  ~CheckedPtr()
5   WebKit:  contentFilterDidBlock()::::~()
6   WebKit:  contentFilterDidBlock()::::~()
7   WebKit:  __alloc_func<..., void (bool)>::destroy()
8   WebKit:  __func<..., void (bool)>::destroy_deallocate()
9   WebCore:  __value_func<void (bool)>::~__value_func()
10  WebCore:  __value_func<void (bool)>::~__value_func()
11  WebCore:  ~function<void (bool)>()
12  WebCore:  ContentFilterUnblockHandler::requestUnblockAsync()::::operator()::lambda()::~()
[...]
```

Occurs on production builds with content filtering enabled. This is detected by CheckedPtr's safety mechanism.

<rdar://165364915>

[David Kilzer (:ddkilzer)]

The fix changes the lambda capture from `CheckedPtr contentFilter` to `WeakPtr weakContentFilter` in NetworkResourceLoader::contentFilterDidBlock() and startContentFiltering().

**NetworkResourceLoader::contentFilterDidBlock():**

```cpp
// Before:
m_unblockHandler.requestUnblockAsync([this, protectedThis = Ref { *this }, contentFilter](bool unblocked) mutable {
    contentFilter->setBlockedError(error);
    // ...
});

// After:
WeakPtr weakContentFilter = m_contentFilter.get();
m_unblockHandler.requestUnblockAsync([this, protectedThis = Ref { *this }, weakContentFilter](bool unblocked) mutable {
    CheckedPtr contentFilter = weakContentFilter.get();
    if (\!contentFilter)
        return;
    contentFilter->setBlockedError(error);
    // ...
});
```

**NetworkResourceLoader::startContentFiltering():**

Similar pattern - replace `CheckedPtr contentFilter` capture with `WeakPtr weakContentFilter`, get the `CheckedPtr` inside the lambda, and check for null before using.

When ContentFilter is destroyed before the callback executes, `weakContentFilter.get()` returns null, allowing safe early exit. This prevents the CheckedPtr detector crash while maintaining memory safety.

No functional change - callbacks that would crash now safely exit when filter is destroyed.

[David Kilzer (:ddkilzer)]

## Regression Analysis

The regression was likely caused by commit f5665ae82cc3 ("The ContentFilter method continueAfterWillSendRequest should use a completion handler"), cherry-picked as 7ca9e7b0ce09 and 82b6be1a4b2e:

https://commits.webkit.org/302627@main

### Before (WebKit-7622.1.22.10.8 / iOS 26.0)

`startContentFiltering()` was synchronous:

```
bool NetworkResourceLoader::startContentFiltering(ResourceRequest& request)
{
    // ...
    CheckedPtr contentFilter = m_contentFilter.get();
    // ...
    if (\!contentFilter->continueAfterWillSendRequest(request, ResourceResponse())) {
        contentFilter->stopFilteringMainResource();
        return false;
    }
    return true;
}
```

The call blocked until the decision was ready. No lambda, no lifetime issue.

### After (WebKit-7623.1.14.10.9 / iOS 26.2)

`startContentFiltering()` became asynchronous with a completion handler:

```
void NetworkResourceLoader::startContentFiltering(ResourceRequest&& request,
    CompletionHandler<void(ResourceRequest)>&& completionHandler)
{
    // ...
    CheckedPtr contentFilter = m_contentFilter.get();
    // ...
    CompletionHandler<void(ResourceRequest)> completion =
        [contentFilter, completionHandler = WTFMove(completionHandler)](ResourceRequest&& request) mutable {
            ASSERT(isMainRunLoop());
            if (CheckedPtr filter = std::exchange(contentFilter, nullptr); request.isNull())
                filter->stopFilteringMainResource();
            completionHandler(WTFMove(request));
        };
    contentFilter->continueAfterWillSendRequest(WTFMove(request), ResourceResponse(), WTFMove(completion));
}
```

The lambda captures `contentFilter` (a `CheckedPtr<ContentFilter>`) by value. This lambda is stored in `ContentFilterCallbackAggregator` and called asynchronously.

### The Bug

1. Lambda is created with `CheckedPtr<ContentFilter>` capture
2. Lambda is passed to `continueAfterWillSendRequest()` and stored in the aggregator
3. `NetworkResourceLoader` is destroyed (taking `m_contentFilter` with it)
4. Lambda destructor runs, tries to decrement `CheckedPtr` reference count
5. `CheckedPtr` detects the object was freed and crashes via `RELEASE_ASSERT`

### Why the Change Was Made

The original commit message explains:

> The ContentFilter method continueAfterWillSendRequest should use a completion handler to avoid blocking the main thread waiting for a decision that can take a significant amount of time. [...] the responsiveness timer in the UI process would terminate the Networking process if the content filter was blocking the main thread for more than 6s.

The intent was correct, but the implementation introduced a lifetime bug.

[David Kilzer (:ddkilzer)]

Actually, the fix is even simpler.  I'll just add a `protectedThis = Ref { *this }` argument to the lambda in NetworkResourceLoader::startContentFiltering().

The CheckedPtr use seems like overkill, but it's not wrong per se.

[David Kilzer (:ddkilzer)]

Pull request: https://github.com/WebKit/WebKit/pull/57412

[David Kilzer (:ddkilzer)]

Crash is in NetworkResourceLoader::contentFilterDidBlock(), but we're going to update NetworkResourceLoader::startContentFiltering() to use the same patterns.

[EWS]

Committed 306652@main (4c8d209dd76f): <https://commits.webkit.org/306652@main>

Reviewed commits have been landed. Closing PR #57412 and removing active labels.