1672473 - When DevTools toolbox is opened and cache disabled, closing RDM re enables the cache (browsing context flag is reset)

Status: RESOLVED FIXED

(DevTools :: Framework, defect, P2)

Description

[Ryan Olton]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:82.0) Gecko/20100101 Firefox/82.0

Steps to reproduce:

1. Load webpage (https://www.weather.gov/)
2. Open Dev Tools -> Network -> toggle off then on (check) "Disable Cache" -> F5 (refresh) -- this works just fine
3. Enter "Responsive Design Mode"
4. Inspect any element (right-click -> "Inspect Element"; this causes the Dev Tools "tab" to change to "Inspector")
5. F5 (Refresh) -- this works just fine
6. Exit "Responsive Design Mode"
7. F5 (Refresh)
8. Select "Network" "tab" within DevTools, notice the "Disable Cache" is still checked, however, CSS (and other assets) are status 304

Actual results:

"Responsive Design Mode" does not respect "Disable Cache" whithin DevTools, leading to incorrect (cached) CSS files (etc) being loaded. Many assets are loaded with a status of 304, instead a status of 200.

Expected results:

Cache should be disabled and all assets should loaded new and not from cache; status should be 200 instead of status 304.

Comment 1

[JakeP]

A real "headache" but. Between this and #1661501 Firefox is getting unusable as a Development browser.

Comment 2

[JakeP]

A real "headache" bug. Between this and #1661501 Firefox is getting unusable as a Development browser.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

The cache is re-enabled because we call _restoreParentProcessConfiguration when closing RDM https://searchfox.org/mozilla-release/rev/63677cc33948d70cb9e676f88cdb775495cbdbdb/devtools/server/actors/target-configuration.js#300,312

_restoreParentProcessConfiguration() {
...
  this._setCacheDisabled(false);

called from target destroy https://searchfox.org/mozilla-release/rev/63677cc33948d70cb9e676f88cdb775495cbdbdb/devtools/server/actors/target-configuration.js#496,507

destroy() {
...
    this._restoreParentProcessConfiguration();

which originates from RDM destruction https://searchfox.org/mozilla-release/rev/63677cc33948d70cb9e676f88cdb775495cbdbdb/devtools/client/responsive/ui.js#262,371

async destroy(options) {
...
  const commandsDestroyed = this.commands.destroy();

Note that we might have other issues due to the target destruction for RDM. For example, we're clearing the watchedByDevTools flag https://searchfox.org/mozilla-release/rev/63677cc33948d70cb9e676f88cdb775495cbdbdb/devtools/server/actors/targets/window-global.js#753,779-788

destroy({ isTargetSwitching = false, isModeSwitching = false } = {}) {
...
  // The watchedByDevTools flag is only set on top level BrowsingContext
  // (as it then cascades to all its children),
  // and when destroying the target, we should tell the platform we no longer
  // observe this BrowsingContext and set this attribute to false.
  if (
    this.browsingContext?.watchedByDevTools &&
    !this.browsingContext.parent
  ) {
    this.browsingContext.watchedByDevTools = false;
  }

I wonder if we don't have similar issues when the regular toolbox and the multiprocess toolbox is opened and one of them is getting closed.
A solution might be to turn watchedByDevTools into an integer that we would increment/decrement each time a target for the browsing context is created/destroyed , and only reset the browsing context flags if the watchedByDevTools counter is 0.

Comment 4

[Alexandre Poirot [:ochameau]]

The issue around watchedByDevTools comes from 1620966.

In bug 1593937, I had in mind to leverage this flag to not only help platform know if DevTools is active on the BrowsingContext, but also optimize the load of DevTools content process JS Window Actors module and trigger the creation of all nested target on the whole BrowingContext tree (https://phabricator.services.mozilla.com/D65983).
But it sounds irrelevant today now that DevTools is rather using JS Process actors...

It would be nice to keep watchedByDevTools as a boolean getter. This flag is used by the platform to know if DevTools is involved or not and doesn't care about the number of active sessions.
I'd suggest introducing two methods to notify about devtools starting and stopping to debug a given BrowsingContext. This would match the API exposed on ChromeUtils, which is similar, but for a whole Content Process:
https://searchfox.org/mozilla-central/rev/0d1f8ff61fe506646fe3898ef727817b4436ab32/dom/chrome-webidl/ChromeUtils.webidl#120-137

I can probably help on doing this BrowsingContext tweak as I originally introduced this property, and the ChromeUtils one.

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Note that we already did fix for light/dark theme simulation in Bug 1700548
We might add an ad-hoc fix for the cache flag as well. The following seems to work

diff --git a/devtools/server/actors/target-configuration.js b/devtools/server/actors/target-configuration.js
--- a/devtools/server/actors/target-configuration.js
+++ b/devtools/server/actors/target-configuration.js
@@ -312,7 +312,9 @@ class TargetConfigurationActor extends A
 
     this._setServiceWorkersTestingEnabled(false);
     this._setPrintSimulationEnabled(false);
-    this._setCacheDisabled(false);
+    if (this._resetCacheDisabledOnDestroy) {
+      this._setCacheDisabled(false);
+    }
     this._setTabOffline(false);
 
     // Restore the color scheme simulation only if it was explicitly updated
@@ -482,6 +484,7 @@ class TargetConfigurationActor extends A
       : Ci.nsIRequest.LOAD_NORMAL;
     if (this._browsingContext.defaultLoadFlags != value) {
       this._browsingContext.defaultLoadFlags = value;
+      this._resetCacheDisabledOnDestroy = true;
     }
   }

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1672473 - [devtools] Reset browsing context defaultLoadFlags from the target that set it. r=#devtools

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a04196a09eb1
[devtools] Reset browsing context defaultLoadFlags from the target that set it. r=devtools-reviewers,jdescottes

Comment 8

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/a04196a09eb1