[webkit-changes] [WebKit/WebKit] 5d11f0: REGRESSION (248723 at main): Videos on bbc.co.uk fail...

Wenson Hsieh noreply at github.com
Tue Jan 3 11:18:10 PST 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 5d11f0377c180c6fb87a0d60f84082c75753eac1
      https://github.com/WebKit/WebKit/commit/5d11f0377c180c6fb87a0d60f84082c75753eac1
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2023-01-03 (Tue, 03 Jan 2023)

  Changed paths:
    A LayoutTests/http/tests/contentfiltering/load-event-in-allowed-subframe-expected.txt
    A LayoutTests/http/tests/contentfiltering/load-event-in-allowed-subframe.html
    A LayoutTests/http/tests/contentfiltering/resources/lots-of-text.html
    M Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

  Log Message:
  -----------
  REGRESSION (248723 at main): Videos on bbc.co.uk fail to load when content filtering is enabled
https://bugs.webkit.org/show_bug.cgi?id=249811
rdar://103247851

Reviewed by Brent Fulgham and Alex Christensen.

On articles on bbc.co.uk, video elements are embedded inside subframes, and are only interactive
once the containing subframe has finished loading (i.e. dispatched the "load" event). In the case
where:

1.  Content filtering is enabled, and also happens in the network process via the
    `CONTENT_FILTERING_IN_NETWORKING_PROCESS` codepath new in iOS 16/macOS Ventura.

2.  The subframe is loaded from a disk cache entry (as opposed to over the network, or in memory
    cache).

...we end up never dispatching the load event on the subframe, due to the fact that the
corresponding `WebCore::SubresourceLoader` driving the subframe load never gets notified that
loading finished. This is because, in following code within the network process (in the codepath
marked below), we exit early after delivering the cached data to the content filter, before either
sending `WebResourceLoader::DidReceiveResource` or `WebResourceLoader::DidFinishResourceLoad` (the
latter of which we use in the case where we don't already have a shareable resource handle). To fix
this, we simply pull the logic to dispatch `WebResourceLoader::DidFinishResourceLoad` out into a
separate callback, and invoke it from both places.

```
    #if ENABLE(SHAREABLE_RESOURCE)
        if (!entry->shareableResourceHandle().isNull()) {
    #if ENABLE(CONTENT_FILTERING_IN_NETWORKING_PROCESS)
            if (m_contentFilter && !m_contentFilter->continueAfterDataReceived(entry->buffer()->makeContiguous(), entry->buffer()->size())) {
                m_contentFilter->continueAfterNotifyFinished(m_parameters.request.url());
                m_contentFilter->stopFilteringMainResource();               // <-------
                return;
            }
    #endif
            send(Messages::WebResourceLoader::DidReceiveResource(entry->shareableResourceHandle()));
            return;
        }
    #endif
```

Note that we prefer dispatching `DidFinishResourceLoad` over `DidReceiveResource` below (which we
would normally use in this shareable resource handle codepath), since the resource loader in the web
process would already have received the data when content filtering is enabled, so sending the
entire cached resource handle again is redundant.

Test: http/tests/contentfiltering/load-event-in-allowed-subframe.html

* LayoutTests/http/tests/contentfiltering/load-event-in-allowed-subframe-expected.txt: Added.
* LayoutTests/http/tests/contentfiltering/load-event-in-allowed-subframe.html: Added.
* LayoutTests/http/tests/contentfiltering/resources/lots-of-text.html: Added.

Add a new test resource that consists of an HTML page that contains a large amount of text. This
ensures that we'll attempt to store it in disk cache, which is a necessary part of reproducing this
bug.

* Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::sendResultForCacheEntry):

Dispatch `WebResourceLoader::DidFinishResourceLoad` in both content filtering codepaths when
`CONTENT_FILTERING_IN_NETWORKING_PROCESS` is enabled.

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




More information about the webkit-changes mailing list