1947001 - DevTools "script override" should defeat the not-overridden cache in SharedScriptCache

Status: ASSIGNED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

Possibly similar to bug 1946348.

DevTools Debugger provides "script override", and that feature stops working if navigation cache is enabled (also with applied to preload),
possibly because the request doesn't reach the "override" part.
Force-reload reads the overridden file, but normal reload after that reads not-overridden cached file.

This would need some combination of the following:

• (a) The "script override" should clear the cached content for the specific file, so that ScriptLoader doesn't use the cached content, and the subsequent request reaches the "override" part
• (b) The "script override" should return "no-cache" to the ScriptLoader, if not yet
• (c) ScriptLoader should evict the cache if it receives "no-cache" response, so that the subsequent request won't use either of not-overridden or overridden cache, but always reaches the "override" part

(c) may apply also to stylesheet, given it's part of SharedSubResourceCache.

Comment 1

[Tooru Fujisawa [:arai]]

Actually this issue doesn't happen with stylesheet, because the stylesheet case also uses the SharedSubResourceCache for incomplete load.

Comment 2

[Tooru Fujisawa [:arai]]

The related code in DevTools's side would be the following:

https://searchfox.org/mozilla-central/rev/206eaea9a2fd4307da16e1614cd934920368165a/devtools/shared/network-observer/NetworkObserver.sys.mjs#241-244

Services.obs.addObserver(
  this.#httpBeforeConnect,
  "http-on-before-connect"
);

https://searchfox.org/mozilla-central/rev/206eaea9a2fd4307da16e1614cd934920368165a/devtools/shared/network-observer/NetworkObserver.sys.mjs#336-344,357-359

#httpBeforeConnect = DevToolsInfaillibleUtils.makeInfallible(
  (subject, topic) => {
    if (
      this.#isDestroyed ||
      topic != "http-on-before-connect" ||
      !(subject instanceof Ci.nsIHttpChannel)
    ) {
      return;
    }
...
    // Handle overrides in http-on-before-connect because we need to redirect
    // the request to the override before reaching the server.
    this.#checkForContentOverride(httpActivity);

https://searchfox.org/mozilla-central/rev/206eaea9a2fd4307da16e1614cd934920368165a/devtools/shared/network-observer/NetworkObserver.sys.mjs#415-420,423-424

#checkForContentOverride(httpActivity) {
  const channel = httpActivity.channel;
  const overridePath = this.#overrides.get(channel.URI.spec);
  if (!overridePath) {
    return false;
  }
...
  try {
    lazy.NetworkOverride.overrideChannelWithFilePath(channel, overridePath);

Then, apparently the feature is also used for other resources, accessible from the network monitor.

https://searchfox.org/mozilla-central/rev/206eaea9a2fd4307da16e1614cd934920368165a/devtools/client/netmonitor/src/widgets/RequestListContextMenu.js#435

setNetworkOverride(toolbox.commands, url, content, window)

And I confirmed the override isn't reflected to the stylesheets until force-reload, if the file is cached.

Comment 3

[Tooru Fujisawa [:arai]]

(In reply to Tooru Fujisawa [:arai] from comment #0)

• (b) The "script override" should return "no-cache" to the ScriptLoader, if not yet

This seems to be already done.
To the ScriptLoader, the overridden response is not cacheable.

Comment 4

[Tooru Fujisawa [:arai]]

Prototyped the cache API in bug 1947158 and integrated into the network override, and just realized that the http-on-before-connect notification is used only when the necko cache is also disabled, or not cached.
This means, just evicting the corresponding cache from the in-memory cache doesn't actually solve bug 1933455.

But at least this keeps the previous behavior.
If necessary, we can look into integrating the network override to the necko cache in separate bug.

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1947001 - Part 1: Clear shared sub resource cache when setting network override. r?#devtools-reviewers!

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Bug 1947001 - Part 2: Add setNetworkOverride parameter to support the case where the response is not available. r?#devtools-reviewers!

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Bug 1947001 - Part 3: Check the initiator to correctly select the request. r?#devtools-reviewers!

Once the SharedScriptCache is enabled, the order of the requests inside the
Network Monitor isn't stable inside the testcase.

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1947001 - Part 4: Add network override test with cache. r?#devtools-reviewers!