1416928 - Web extension content_scripts often don't show or hit breakpoints in the debugger upon page reload

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Matthew N. [:MattN]]

STR:
1) Go to about:debugging
2) Install a web extension that uses content_scripts with js e.g. https://github.com/mdn/webextensions-examples/tree/master/borderify
3) Load a page that should have the content script injected
4) Open the Debugger for that tab and see the moz-extension… script in the Sources sidebar
5) Add a breakpoint on a line of extension JS that runs during page load
6) Reload the page

Expected result:
The content scripts should be shown in the debugger sources sidebar and the breakpoint should be hit

Actual result:
The source will not be shown for any moz-extension files and the breakpoint will not be hit.

Partial Workaround:
If you need to debug a specific line upon injection time you can add a `debugger` statement to pause for debugging and cause that one source to show in the sidebar. Other content_scripts sources in that same extension will still be missing in the sidebar.

This bug makes it painful to debug content script code running upon page load.

I tested in a local 59.0a1 build and Firefox Release 56.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I had hit a variant of this via the "web-ext" npm command. Sometimes, the breakpoint will not be loaded (usually after the first try), and I have to close the browser and re-run via "web-ext" again.

I wrote about this caveat several months ago:

https://garykwong.wordpress.com/2017/09/18/porting-a-legacy-add-on-to-webextensions/

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

I took a deep look into this, follows some notes from my findings.

There are two issues that could lead to a missing content script from the tab developer tools' Debugger panel:

- when the tab is reloaded while the developers tools are already opened, 
  if a "cached precompiled content script" is executed, then it is not being listed in the Debugger panel
- the Content Script source may be already garbage collected when the developer tools are opened on the tab

### "cached precompiled content script" is executed but it is not being listed in the Debugger panel

The WebExtension Content Script defined as extension urls are precompiled and cached internally by ExtensionContent.jsm:

- https://searchfox.org/mozilla-central/rev/a662f122c37704456457a526af90db4e3c0fd10e/toolkit/components/extensions/ExtensionContent.jsm#138-151

And so if the developers tools have already "seen" the "cached precompiled script" (which is the case when, as described in the STR from comment 0, we have selected the content script in the Debugger panel and we have set a breakpoint on it),

Then, when we reload the tab, the "cached precompiled script" is executed again as expected, but the ThreadActor will ignore it in the ThreadActor._addSource method because `this._debuggerSourcesSeen.has(source)` returns true for it:

- https://searchfox.org/mozilla-central/rev/a662f122c37704456457a526af90db4e3c0fd10e/devtools/server/actors/script.js#1952-1954

The source actor will not be created for it, and so it will not be notified to the Debugger panel running on the client side, 
it will be missing from the Debugger panel source tree, and the breakpoint set will not be able to be fired as expected and the debugger paused.

## the Content Script source may be already garbage collected when the developer tools are opened on the tab

The Debugger panel asks for the existent sources to the debugger server and then, on the debugger server side, the ThreadActor._discoverSources is going to discover them by running dbg.findSources() which is "not entirely deterministic due to the garbage collector's behavior", as explicitly mentioned in the related docs https://searchfox.org/mozilla-central/source/js/src/doc/Debugger/Debugger.md:

    Whether such scripts appear can be affected by the garbage collector's behavior, 
    so this function's behavior is not entirely deterministic.

This issue is particularly easy to trigger when the content script doesn't do anything that could convince the garbage collector that it cannot be collected, e.g. a content script containing the following code can be garbage collected at any time after its execution (which is the one injected by the borderify mdn example, https://github.com/mdn/webextensions-examples/blob/master/borderify/borderify.js): 

```
document.body.style.border = "5px solid red";
```

on the contrary the following content script will not be garbage collected until the document has been also destroyed:

```
document.body.style.border = "5px solid red";
window.onload = () => {};
```

As a side-notes: reloading the tab again while the developer tools are already opened, will force the script to be re-executed and then to be listed in the Debugger panel (which also requires the issue with the "cached precompiled content scripts" described above to be fixed to work reliably)

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1416928 - Ensure that cached ContentScript sources are not skipped by the ThreadActor.onNewScript.

Review commit: https://reviewboard.mozilla.org/r/200286/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/200286/

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1416928 - Test content script debugging on the new debugger UI.

Review commit: https://reviewboard.mozilla.org/r/200288/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/200288/

Comment 5

[Jason Laster [:jlast]]

Comment on attachment 8928943 [details]
Bug 1416928 - Test content script debugging on the new debugger UI.

https://reviewboard.mozilla.org/r/200288/#review205422

Thanks!

small nits, r+ when it is green

::: devtools/client/debugger/new/test/mochitest/browser_dbg-content-script-sources.js:53
(Diff revision 1)
> +    files: {
> +      "content_script.js": contentScript,
> +    },
> +  });
> +
> +  await extension.startup();

style nit: it might be nice to move this into a function: const extension = startExtension()

the benefit would be it is a bit clearer what local data we need below for testing

::: devtools/client/debugger/new/test/mochitest/browser_dbg-content-script-sources.js:72
(Diff revision 1)
> +
> +  for (let i = 1; i < 3; i++) {
> +    info(`Reloading tab (${i} time)`);
> +    gBrowser.reloadTab(gBrowser.selectedTab);
> +    await waitForPaused(dbg);
> +    await waitForSources(dbg, "content_script.js");

we should wait for the source to be selected, which will also check to see if we have the source loaded and it is shown. waitForSelectedSource

::: devtools/client/debugger/new/test/mochitest/browser_dbg-content-script-sources.js:79
(Diff revision 1)
> +      findElementWithSelector(dbg, ".sources-list .focused"),
> +      "Source is focused"
> +    );
> +    assertPausedLocation(dbg);
> +    assertDebugLine(dbg, 2);
> +    resume(dbg);

this might be async. `await resume`

Comment 6

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8928943 [details]
Bug 1416928 - Test content script debugging on the new debugger UI.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200288/diff/1-2/

Comment 7

[Yulia Startsev [:yulia]]

Comment on attachment 8928942 [details]
Bug 1416928 - Ensure that cached ContentScript sources are not skipped by the ThreadActor.onNewScript.

https://reviewboard.mozilla.org/r/200286/#review208194

this looks good to me

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/303b35020290
Ensure that cached ContentScript sources are not skipped by the ThreadActor.onNewScript. r=yulia
https://hg.mozilla.org/integration/autoland/rev/3de9dc374632
Test content script debugging on the new debugger UI. r=jlast

Comment 9

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/303b35020290
https://hg.mozilla.org/mozilla-central/rev/3de9dc374632