1720512 - Debugger tab is empty

Status: VERIFIED FIXED

(DevTools :: Debugger, defect)

Description

[Ksenia Berezina [:ksenia]]

What were you doing?

1. Open devtools panel in Firefox Nightly 92.0a1 (2021-07-14) on any webpage and observe the Debugger tab

What happened?

Debugger tab content is blank

What should have happened?

Debugger tab content is visible

Anything else we should know?

This issue is not reproducible with a clean profile.

From mozregression:

21:34.65 INFO: Narrowed integration regression window from [ab561bec, f0a9503e] (4 builds) to [64c54eaa, f0a9503e] (2 builds) (~1 steps left)
21:34.65 INFO: No more integration revisions, bisection finished.
21:34.65 INFO: Last good revision: 64c54eaab640c8b2c66fa4d118abbfe2ed58f0a0
21:34.65 INFO: First bad revision: f0a9503e117d726f014955259136facfa389880e
21:34.65 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=64c54eaab640c8b2c66fa4d118abbfe2ed58f0a0&tochange=f0a9503e117d726f014955259136facfa389880e

Comment 1

[Ksenia Berezina [:ksenia]]

Getting the following errors in the Browser Console:

Error while calling actor 'breakpoint-list's method 'setBreakpoint' Breakpoints expect to have either a sourceUrl or a sourceId. Actor.js:92:13
    writeError resource://devtools/shared/protocol/Actor.js:92
    handler resource://devtools/shared/protocol/Actor.js:215
Error: Breakpoints expect to have either a sourceUrl or a sourceId.
    breakpoints resource://devtools/server/actors/watcher/WatchedDataHelpers.jsm:31
    existingIndex resource://devtools/server/actors/watcher/WatchedDataHelpers.jsm:109
    addWatchedDataEntry resource://devtools/server/actors/watcher/WatchedDataHelpers.jsm:108
    addWatcherDataEntry resource://devtools/server/actors/watcher/WatcherRegistry.jsm:166
    addDataEntry resource://devtools/server/actors/watcher.js:523
    setBreakpoint resource://devtools/server/actors/breakpoint-list.js:36
    handler resource://devtools/shared/protocol/Actor.js:175
    onPacket resource://devtools/server/devtools-server-connection.js:379
    send resource://devtools/shared/transport/local-transport.js:68
    makeInfallible resource://devtools/shared/ThreadSafeDevToolsUtils.js:103
    makeInfallible resource://devtools/shared/ThreadSafeDevToolsUtils.js:103

Comment 2

[Ksenia Berezina [:ksenia]]

Hi Alexandre, could you please take a look?

Comment 3

[Dennis Schubert [:denschub]]

I don't know why, but for me, it works in a clean profile, and resetting devtools.debugger.prefs-schema-version and re-opening the debugger made it work for an old profile. Maybe something related to old breakpoints being stored somewhere in an incompatible format?

Comment 4

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

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

Comment 5

[Julian Descottes [:jdescottes]]

ksenia: This might be related to one of your existing breakpoints. Could you:

• open the Browser Console
• run the following snippet:

(async function () {
	var loader = Cu.import("resource://devtools/shared/Loader.jsm", {});
  var asyncStorage = loader.require("devtools/shared/async-storage");
  const breakpoints = await asyncStorage.getItem("debugger.pending-breakpoints");
  console.log(JSON.stringify(breakpoints))  
})()

• copy/paste the output of the command here

This should allow us to import your breakpoints and hopefully reproduce the issue.
If this is not enough, we might have to look at individual debugger preferences, but that would be a good start.

Comment 6

[Ksenia Berezina [:ksenia]]

Attachment: scratch.json

Thanks for looking into this! I've attached a file with the output

Comment 7

[Alexandre Poirot [:ochameau]]

Thanks a lot for your breakpoint list, that's super actionnable thanks to it!

So bug 1717739 made it so that the debugger would fail to open if the existing breakpoint list failed to be applied.
And your list contains invalid breakpoints with line and column, but without any source URL.

I'll try to craft some patch. I imagine you aren't the only one with invalid entries...

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1720512 - [devtools] Avoid storing and restoring broken breakpoints.

This is a bit surprising that broken breakpoints are stored
as the frontend seems to try to do a couple of assertions:
https://searchfox.org/mozilla-central/rev/c0fc8c4852e927b0ae75d893d35772b8c60ee06b/devtools/client/debugger/src/utils/breakpoint/index.js#84-106

But at least, now, we start using same assertions in both sides.

Comment 10

[Julian Descottes [:jdescottes]]

Given the symptoms, we should probably uplift this as soon as possible.

Comment 11

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1720512 - [devtools] Remove unused code from devtools/client/debugger/src/utils/breakpoint/index.js.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c579e2ce174
[devtools] Avoid storing and restoring broken breakpoints. r=jdescottes,bomsy

Comment 13

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/9c579e2ce174

Comment 14

[Alexandre Poirot [:ochameau]]

Comment on attachment 9231404 [details]
Bug 1720512 - [devtools] Avoid storing and restoring broken breakpoints.

Beta/Release Uplift Approval Request

• User impact if declined: Completely broken debugger if broken breakpoints were saved from a previous session.
  And unfortunately, it was quite easy to have broken breakpoints.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: * open any page with some javascript, for ex data:text/html,<script>console.log('foo')</script>
• Set a breakpoint on any javascript code, here, the console.log('foo') line. (We need a valid breakpoint to be set in order to reproduce)
• open devtools and evaluate this

(function() {
  console.log(1);
  debugger;
}) ();

The debugger should pause on debugger;

• Now set a breakpoint on console.log(1) line. (This will try to save a BP that can't be save)
• Close the DevTools
• Re-open DevTools on the debugger

Without this patch, the debugger was reopening broken, blank, without any source.
No matter against which page you reopen it. Even if you close and restart Firefox, it would still be broken.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The risk is limited to DevTools and especially on the Debugger.
  The patch is rather trivial.

We don't have explicit coverage about this particular STR.
We don't have testing covering closing and reopening the debugger.
But the modified code is in hot codepath and being already covered in many other STRs.

We are looking forward providing specific test coverage in https://bugzilla.mozilla.org/show_bug.cgi?id=1721197.

• String changes made/needed: no

Comment 15

[Pascal Chevrel:pascalc]

Comment on attachment 9231404 [details]
Bug 1720512 - [devtools] Avoid storing and restoring broken breakpoints.

Approved for 91 beta 6, thanks.

Comment 16

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/52687472b618

Comment 17

[Phabricator Automation]

Comment on attachment 9231909 [details]
Bug 1720512 - [devtools] Remove unused code from devtools/client/debugger/src/utils/breakpoint/index.js.

Revision D120227 was moved to bug 1721862. Setting attachment 9231909 [details] to obsolete.

Comment 18

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Reproduced the issue on Firefox 92.0a1 (2021-07-14) under macOS 10.15 by following the info provided in Comment 14.

The issue is fixed on Firefox 91.0b6 and Firefox 92.0a1 (2021-07-22). Tests were performed on macOS 10.15, Ubuntu 20.04 and Windows 10.