Related to the investigation performed in Bug 1694405.

Features recently moved to rely on the watcher are broken when remote debugging a Tab target which has a server that supports the watcher codepath.

The reason is that we only enable Frame watching for local tab debugging
https://searchfox.org/mozilla-central/rev/5a66c4b4a41ab78a87c30c9db0d93c732c534402/devtools/shared/resources/target-list.js#265-267

We do not want to enable frame watching for remote debugging yet, we would prefer to first enable server side target switching and stop reusing clients between remote debugging toolboxes.

Instead we should apply the same workaround as for watchResources [1] and accept calls to WatcherActor::addDataEntry [2] for Frame targets even if we are not watching for frames.

[1] https://searchfox.org/mozilla-central/rev/5a66c4b4a41ab78a87c30c9db0d93c732c534402/devtools/server/actors/watcher.js#338-348
[2] https://searchfox.org/mozilla-central/rev/5a66c4b4a41ab78a87c30c9db0d93c732c534402/devtools/server/actors/watcher.js#523,556

This fixes all features currently depending on data entries, like breakpoints and target configurations.

Depends on D107674

Comment on attachment 9207821 [details]
Bug 1697184 - [devtools] Add breakpoints test with about:devtools-toolbox

Revision D107675 was moved to bug 1697234. Setting attachment 9207821 [details] to obsolete.

https://hg.mozilla.org/mozilla-central/rev/40835a302a86

Comment on attachment 9207774 [details]
Bug 1697184 - Pass data entries to top level target even when the client doesn't watch for frames.

Beta/Release Uplift Approval Request

• User impact if declined: Web Developers cannot set breakpoints when debugging websites on Fenix.

Automated tests landed in Bug 1697234, which should be uplifted as well.

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: (note: I just tested this with Fenix Nightly + FF Nightly, but since we don't have real automated tests for the remote connection part, I think it still makes sense to have QE test here)

tldr: setup remote debugging, connect to a page running in your Fenix instance, set a breakpoint and check the breakpoint is hit and the debugger pauses.

More detailed instructions below:

Setup remote debugging:

• on an Android mobile, enable Android USB debugging,
• open Fenix Nightly, enable Fenix USB debugging in settings
• open Firefox Desktop Nightly,
• open about:debugging
• connect to Firefox Nightly on the mobile

(https://developer.mozilla.org/en-US/docs/Tools/about:debugging#connecting_to_a_remote_device)

STRs:

• on Fenix open a page with a script where you can set a breakpoint
• on about:debugging, select Firefox Nightly in the left sidebar
• (note: if you already were on the Runtime page for Firefox Nightly and you don't see you new tab listed, just reload about about:debugging)
• click on "inspect" for the Fenix page you want to debug
• in the about:devtools-toolbox tab which opened, open the debugger and set a breakpoint in a JS file
• (note: if you don't see any source you are probably hitting https://bugzilla.mozilla.org/show_bug.cgi?id=1694405, apply the workaround mentioned in Bug 1694405 comment 2)
• trigger the breakpoint

ER: the breakpoint should be hit, the debugger should pause.
Without the patch the breakpoint is never hit.

• List of other uplifts needed: Bug 1697234
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Minor javascript change, we have several data we need to send from the DevTools client to the server and they don't share the same code yet. Here we just reuse for Breakpoints a pattern we already applied to another dataset.
• String changes made/needed:

We're out of betas for 87, and are building the release candidate today. This bug is severity S3, so I'm not sure it's worth the risk.

When did this break, and how come we didn't notice?

(In reply to Julien Cristau [:jcristau] from comment #7)

We're out of betas for 87, and are building the release candidate today. This bug is severity S3, so I'm not sure it's worth the risk.

When did this break, and how come we didn't notice?

This broke with Bug 1573327.

The reason we didn't detect it:

• we don't have automated tests for remote debugging
• this only started breaking when Fenix was updated with the server from Bug 1662129 (ie remote debugging without backward compatibility mode)

The second point is important. Most of the remote debugging regressions are related to backward compatibility, so we try to test it whenever a change might cause a problem. But in this particular case remote debugging + backward compatibility was working fine, and things only started breaking when using remote debugging without backward compatibility (ie, with a recent enough DevTools Server). This usually doesn't happen because we trust our regular test suite to catch everything outside of backward compat issues. But there are small configuration differences between Remote debugging and regular devtools, and the issue here was about one of those.

The severity was set to S3 because it only impacts developers using remote debugging with Fenix. But in practice, those users have no way to set breakpoints on Fenix with release 87. So really small fraction of the user base, but big impact for those users.

Now I initially assumed this broke with Bug 1662129 (which landed in 87). After verifying, it broke since Bug 1573327, which landed in 86. So effectively it's already been broken for 1 release. Considering this already made it to the Release channel, and we didn't hear much about the issue I think it's fair to let it ride the trains. Could be nice to get the fix to users sooner, but I understand if it's too risky to take in so late.

Comment on attachment 9207774 [details]
Bug 1697184 - Pass data entries to top level target even when the client doesn't watch for frames.

Thanks for the extra info and context, Julian!

I have verified this fix with Desktop Firefox Nightly v88.0a1 from 2021-03-16 and Android Firefox Nightly 210315 in Windows 10 with the steps in comment 6.