1806802 - Support "network.responseCompleted" for cached responses

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P1)

Description

[Julian Descottes [:jdescottes]]

Similarly to Bug 1806794, we will handled cached responses after the basic implementation is done. We should wait for Bug 1806794 to be fixed so that we can easily add tests for this.

There is only one trigger for response completed, so this should hopefully be easier.

Comment 1

[Julian Descottes [:jdescottes]]

I am planning to disable the 304 test case for response completed over at https://bugzilla.mozilla.org/show_bug.cgi?id=1810302 because it seems to fail very frequently on android.

Comment 2

[Julian Descottes [:jdescottes]]

Hi Valentin,

Quick question about checking if a response comes from the local cache.
At the moment, DevTools is mostly relying on the fact that the channel was detected by a http-on-examine-cached-response notification. For BiDi, I initially wanted to rely more on channel APIs, in this case nsICacheInfoChannel.isFromCache.

But when adding test cases, I don't always get the expected value from this API. For cached 200 OK responses it seems to work fine. However I was also testing with a 301 Moved Permanently response and here I always get false from nsICacheInfoChannel.isFromCache.

From BiDi's point of view we mark requests as cached if the response's cache state is local. Do you think I should be able to consistently use nsICacheInfoChannel.isFromCache to get this information, or should I rather follow the DevTools approach and just rely on the http-on-examine-cached-response notification?

Maybe I am calling the API at the wrong time, but I would expect it to return undefined in this case, whereas here it returns false.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1806802 - Support "network.response" events for cached responses

Comment 4

[Phabricator Automation]

Comment on attachment 9320893 [details]
Bug 1806802 - Support "network.response" events for cached responses

Revision D171508 was moved to bug 1806794. Setting attachment 9320893 [details] to obsolete.

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1806802 - [wdspec] Add wdspec test for cached responseCompleted events

Depends on D171547

Comment 6

[Valentin Gosu [:valentin] (he/him)]

(In reply to Julian Descottes [:jdescottes] from comment #2)

Hi Valentin,

Quick question about checking if a response comes from the local cache.
At the moment, DevTools is mostly relying on the fact that the channel was detected by a http-on-examine-cached-response notification. For BiDi, I initially wanted to rely more on channel APIs, in this case nsICacheInfoChannel.isFromCache.

But when adding test cases, I don't always get the expected value from this API. For cached 200 OK responses it seems to work fine. However I was also testing with a 301 Moved Permanently response and here I always get false from nsICacheInfoChannel.isFromCache.

Are you checking nsICacheInfoChannel.isFromCache during onStartRequest, or some other notification?
If it's onStartRequest it likely wouldn't work, because we don't send onStartRequest for the 301 response, but for the final response after the redirect.
I think the attribute should return the correct value when called from OnExamineResponse - but I haven't checked.
If you have a failing test case I can add it to bug 1817750.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/42ef14d95f27
[wdspec] Add wdspec test for cached responseCompleted events r=webdriver-reviewers,whimboo,jgraham

Comment 8

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

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

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/42ef14d95f27

Comment 10

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

Upstream PR merged by moz-wptsync-bot