Open Bug 1571637 Opened 4 months ago Updated 2 months ago

Closing devtools should resume the event loop we stop at

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(Not tracked)

People

(Reporter: ochameau, Unassigned, NeedInfo)

References

(Blocks 3 open bugs)

Details

Attachments

(1 file)

While verifying bug 1567210 I saw a couple of issues around breakpoints.

While the page is now correctly resumed in term of timeouts/intervals and page events,
the actual event loop we stopped at is not resumed when closing the DevTools.

STR:

  • open: data:text/html,<script>window.onclick=()=>{%0Adocument.body.innerHTML+="x";%0A};</script>
  • open the debugger, break at document.body.innerHTML+="x"; line
  • click on the page in order to trigger the breakpoint
  • the page should be paused and "x" is not printend on the page
  • now close the DevTools

Expected result: the breakpoint is released and "x" is displayed.

Current result on nightly: "x" is not displayed, which seems to indicated that the breakpoint isn't correctly released

Thanks for the report Alex, I can reproduce that on my machine.

I am also seeing an error message in the Browser Console. Attached in case it's relevant.

Honza
Priority: -- → P2
Duplicate of this bug: 1586048

@ochameau Curious for your thoughts here. This is caused by https://searchfox.org/mozilla-central/rev/171109434c6f2fe086af3b2322839b346a112a99/devtools/server/actors/thread.js#846 returning null and thus stopping execution.

Is this code still something that actually does anything useful? I ask because we only appear to set _parentClosed on exit (https://searchfox.org/mozilla-central/rev/171109434c6f2fe086af3b2322839b346a112a99/devtools/server/actors/targets/browsing-context.js#550), not on page navigation, meaning that most of the time it doesn't actually do the job of stopping "Executing JS after the content window is gone" anyway, so if it were a critical piece of functionality it seems like it would already be a problem? We also already use pauseAndRespond in a few other places where the return value is ignored, like in XHR, DOM, and Event breakpoints, because they pause inside deep C++ code, not inside a Debugger.cpp hook and so don't have a useful way to cancel execution anyway.

Would it make sense to remove this "feature" of returning null, and then see if we actually run into cases where the window responds badly at this point in time?

Flags: needinfo?(poirot.alex)
You need to log in before you can comment on or make changes to this bug.