1931334 - Network event content.size is always set to 0

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, defect, P2)

Description

[Julian Descottes [:jdescottes]]

Since Bug 1882803, we are monitoring http-on-before-stop-request in content processes in order to fetch the decodedBodySize. However we setup one listener per MessageHandler instance. Observer notifications are scoped to a whole content process, not to a specific browsing context, which means that if we have several MessageHandler instances living in the same process (eg frames in the same domain), we will emit unnecessary events.

We should filter out events which don't match the context we are monitoring.

This currently leads to too much IPC, as well as frequent log pollution such as

console.error: (new InvalidStateError("JSWindowActorChild.sendAsyncMessage: JSWindowActorChild cannot send at the moment", (void 0), 101))

This is very visible on some websites with many shortlived iframes such as https://www.hs.fi/

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1931334 - [bidi] Filter out network._beforeStopRequest events for other contexts

Reuse the same approach as for our other windowglobal network listener (CachedResourceListener).
The current context is passed as a constructor argument to the BeforeStopRequestListener and observer notifications
for channels which are not related to this context will not generate any event.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1882803

Comment 3

[Julian Descottes [:jdescottes]]

After digging a bit more through the implementation for fetching the decodedBodySize in the content process I found a few additional issues.

First, the NetworkDecodedBodySizeMap has a race condition and can fail intermittently.

But more importantly we are also setting up a BeforeStopRequestListener in the parent process, which gets notifications for ALL requests, but always with a size of 0 (because this is parent process and the information is normally not available there).

The issue is that the http-on-before-stop-request notification is always received in the parent process before we have time to communicate it from the content process. This means that at the moment, we always set ... 0, as the decoded body size.

The currently attached patch unexpectedly fixed this, by making the parent process listener ineffective. But by doing so, all tests about network continueWithAuth with provideCredentials start to timeout. It seems that the additional authentication attempts are only captured in the parent process here. Internally we skip the responseCompleted event for the intermediary attempts, and only emit an event for the last event, for which we can't get the decoded body size at the moment.

TLDR, the decoded body size handling is quite broken

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1882803

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1931334 - [bidi] Fix race condition in NetworkDecodedBodySizeMap

Depends on D229004

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1931334 - [bidi] Stop listening for beforeRequestStop in the root module

Depends on D239495

Comment 7

[Julian Descottes [:jdescottes]]

Attachment: Bug 1931334 - [wdspec] Relax BiDi network tests for cached requests

Depends on D239499

Waiting for the decoded body size in content processes introduces a slight delay
which impacts cached response completed tests.

Comment 8

[Julian Descottes [:jdescottes]]

Attachment: Bug 1931334 - [wdspec] Add wdspec tests for network.responseCompleted content.size

Depends on D241086

Adds a simple test for the content size of gzipped network response.
This test only asserts that the value is non-zero. At the moment Chrome seems to
always return 0. Once other implementations than Firefox return something else than
0 we can update the assertion to assert the actual size.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be6aafb658cc
[bidi] Fix race condition in NetworkDecodedBodySizeMap r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/e8bc2a74f3e8
[bidi] Stop listening for beforeRequestStop in the root module r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/e22041831189
[wdspec] Relax BiDi network tests for cached requests r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/09dd3cea930e
[wdspec] Add wdspec tests for network.responseCompleted content.size r=webdriver-reviewers,whimboo

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/51580 for changes under testing/web-platform/tests

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/be6aafb658cc
https://hg.mozilla.org/mozilla-central/rev/e8bc2a74f3e8
https://hg.mozilla.org/mozilla-central/rev/e22041831189
https://hg.mozilla.org/mozilla-central/rev/09dd3cea930e

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot