1806796 - [meta] Missing sources in the debugger when breaking on unload listener

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P3)

Description

[Julian Descottes [:jdescottes]]

Attachment: test_case.html

STRs:

• open test page
• open devtools > debugger > enable Pause on Exceptions
• reload

ER: Debugger should show the source where it breaks
AR: No source displayed

Comment 1

[Julian Descottes [:jdescottes]]

Additional information: the debugger breaks, but we cannot resume

Comment 2

[Nicolas Frandeboeuf]

Hi,

can I also add that setting a breakpoint on the line that will throw won't work either (on unload too)?

Comment 3

[Julian Descottes [:jdescottes]]

Also:

Not specific to inline scripts, just used that for the sake of simplicity.
Same thing happens with a dedicated script file: https://bug1806796-test-case-script.glitch.me/
And as mentioned above, you get similar symptoms with actual breakpoints or debugger statements (but I get the "resume" popup on top of the overlay in that case, so at least we can recover)

Comment 4

[Hubert Boma Manilla (:bomsy)]

Thanks for filing and STRs.
This is likely related to Bug 1569775.

Comment 5

[Alexandre Poirot [:ochameau]]

So we are having an overall design flaw around navigations.

When we pause we receive "pause" THREAD_STATE resource over there:
https://searchfox.org/mozilla-central/rev/cef96316b3643720769dec96542604c3209f1877/devtools/client/debugger/src/client/firefox.js#160-167
this ends up being stuck on the following code:
https://searchfox.org/mozilla-central/rev/cef96316b3643720769dec96542604c3209f1877/devtools/client/debugger/src/client/firefox/create.js#38-41

  // Because of throttling, the source may be available a bit late.
  const sourceActor = await waitForSourceActorToBeRegisteredInStore(
    frame.where.actor
  );

We never get the related source actor and so never dispatch the paused action.

We never get the related source actor because of how we handle navigation in the debugger.
We clear absolutely everything on early navigation. i.e. when we receive DOCUMENT_EVENT "will-navigate" resource:
https://searchfox.org/mozilla-central/rev/cef96316b3643720769dec96542604c3209f1877/devtools/client/debugger/src/client/firefox.js#176-182
The issue is that this event fires before we hit the breakpoint.
It means that we are hitting a breakpoint:

• while the related source has already been unregistered from the debugger frontend,
• we can hit various breakpoints (unload, exceptions) for source of the previous page,
• I'm even wondering if that's possible to be hitting breakpoint from sources of previous and new page at the same time!

The debugger frontend should probably more wisely clear its state on navigation.
For sources, it would probably be safer to release sources only on target destruction.
Target destruction should fire after all late code of the previous page ran.
This would probably make it easier to leak objects in case of unpexpected errors... but would probably solve various issues on late-running code!

Comment 6

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1806796 - [devtools] Clear sources and source actors only on target removal.

That, instead of clearing them entirely on navigation (will-navigate),
which would clear them too early. Typically before the various unload/navigation/pagehide events are fired.
This was making debugging any of these code impossible.

Comment 7

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

The severity field is not set for this bug.
:bomsy, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 9

[Alexandre Poirot [:ochameau]]

Most dependencies of this bugs are about ensuring that all reducers will correctly clear themself correctly on each target/thread removal,
so that in this bug we can drop the full clearance of most reducers on NAVIGATE, which happens to be fired before the unload sequence of the previous page.

A preliminary work to help doing that has been to work on bug 1815500, which expose source and sourceActor object directly on all location objects. This will help the reducers identify the related thread of any location and ease cleaning the reducers per thread.

Another independant, but required refactoring will be bug 1822310. The existing "context" assertions will break all actions being asserted during unload event. This will help finally make sense of these context assertions which have been very inconsistent and not handling thread removal properly.

Comment 10

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1822310 - [devtools] Add tests to cover breakpoints on unload.

Also fix an issue where Exception tooltip was always overriden
by the preview tooltip when we were paused.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a35fad815a44
Bug 1822310 - [devtools] Add tests to cover breakpoints on unload. r=devtools-reviewers,nchevobbe

Comment 12

[Sandor Molnar[:smolnar]]

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