Breakpoints added in the browser toolbox do not apply until after a restart
Categories
(DevTools :: Debugger, defect)
Tracking
(firefox-esr78 unaffected, firefox84 unaffected, firefox85 unaffected, firefox86 fixed)
| Tracking | Status | |
|---|---|---|
| firefox-esr78 | --- | unaffected |
| firefox84 | --- | unaffected |
| firefox85 | --- | unaffected |
| firefox86 | --- | fixed |
People
(Reporter: standard8, Assigned: ochameau)
References
(Regression)
Details
(Keywords: regression)
Attachments
(3 files)
What were you doing?
Pre-setup: Enable the browser toolbox in a profile.
- Start Firefox
- Open the Browser Toolbox
- In the debugger pane, search for
SearchService.jsm, then within that file look forset defaultEngine. - In that function insert a breakpoint.
- Go back to the main Firefox window, then Preferences -> Search.
- Change the default search engine.
What happened?
Breakpoint is not hit.
What should have happened?
Breakpoint should have been hit.
Anything else we should know?
Restarting and opening the toolbox again (without re-adding the breakpoint) causes it to be hit.
Likewise, removed breakpoints remain until restart.
Mozregression says:
18:29.02 INFO: Last good revision: 6305955b7b18010281925ea4670499221f31c09a
18:29.02 INFO: First bad revision: 28b974a963360339a991c880581e05bc5b2f90fe
18:29.02 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6305955b7b18010281925ea4670499221f31c09a&tochange=28b974a963360339a991c880581e05bc5b2f90fe
Needinfo to :ochameau as this hurts core development flow.
|
||
Updated•4 years ago
|
|
Assignee | |
Comment 1•4 years ago
|
||
Oops sorry for breaking that, I'm having a look.
A bit scary that it did not break any test, I'll also have a look...
|
|
Assignee | |
Comment 2•4 years ago
|
||
The one test covering the browser toolbox breakpoints was disabled and quite broken:
https://searchfox.org/mozilla-central/source/devtools/client/framework/browser-toolbox/test/browser.ini#26-27
[browser_browser_toolbox_debugger.js]
skip-if = os == 'win' || debug || (bits == 64 && !debug && (os == 'mac' || os == 'linux')) # Bug 1282269, Bug 1448084, Bug 1270731
I'm attaching patches to:
- ease debugging browser toolbox test
- fix this test
- fix the regression I introduced in bug 1573327
About the regression, the watcher actor wasn't passing breakpoints/watched data to the ParentProcessTarget actor.
That's because:
- nor the frame target helper will iterate over it, because it only consider remote frames, so only target actors running in the content process
- nor the process target helper will iterate over it, because it only consider content processes.
At the end, we need something similar to what we do inwatchResources. Manually reach the target actors running in the parent process.
We could possibly merge watchResource and addWatcherDataEntry, to the cost of passing unecessary messages between processes and threads for resources. Today, watchResurces checks, target per target, if we have to send a message. Using the generic addWatcherDataEntry would force to send new watched resources to all targets, which would have to filter the incoming list.
|
|
Assignee | |
Comment 3•4 years ago
|
||
We need to use the same technique as in watchResources in order to ensure
reaching out the top level targets running in the parent process...
|
|
Assignee | |
Comment 4•4 years ago
|
||
Without this, we were silently ignoring many errors when evaluating a piece of JS.
Imported function may still work, but were missing global variables. Very misleading!
|
|
Assignee | |
Comment 5•4 years ago
|
||
|
|
Assignee | |
Comment 6•4 years ago
|
||
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 7•4 years ago
|
||
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ecdc1fe95497 [devtools] Set breakpoints against the top level process target in the context of the browser toolbox. r=jdescottes https://hg.mozilla.org/integration/autoland/rev/b79aff425ff1 [devtools] Print exceptions when importing functions thrown in browser toolbox tests. r=jdescottes https://hg.mozilla.org/integration/autoland/rev/e08f8ab3453c [devtools] Fix and re-enable browser toolbox test for breakpoints. r=jdescottes
|
||
Comment 9•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/ecdc1fe95497
https://hg.mozilla.org/mozilla-central/rev/b79aff425ff1
https://hg.mozilla.org/mozilla-central/rev/e08f8ab3453c
|
||
Comment 10•4 years ago
|
||
Should we uplift to 85, or live with this regression for one release? We're building the last beta today.
|
|
Assignee | |
Comment 11•4 years ago
|
||
This regression comes from bug 1573327, which landed in 86.
So there should be no need to uplift to 85, right?
|
|
||
Comment 13•4 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #11)
This regression comes from bug 1573327, which landed in 86.
So there should be no need to uplift to 85, right?
The reason I set 85 to affected was that the regression window points to https://hg.mozilla.org/integration/autoland/rev/28b974a963360339a991c880581e05bc5b2f90fe , and that page lists:
milestone 85.0a1
I was told in the past that this information is authoritative. Apparently not, because it's clearly lying here, as https://hg.mozilla.org/integration/autoland/rev/28b974a963360339a991c880581e05bc5b2f90fe exists but (as of writing) https://hg.mozilla.org/releases/mozilla-beta/rev/28b974a963360339a991c880581e05bc5b2f90fe does not. :jcristau, can you forward this to whoever would be able to fix it? I'm not sure which team / bugzilla component is in charge of that metadata creation and/or hgweb at this point.
|
|
||
Comment 14•4 years ago
|
||
As far as I know, that data comes from the contents of config/milestone.txt for that revision. So on autoland there's a window between the last changeset that is merged to central before branching, and the central version bump getting merged back, where that doesn't match which release it actually ends up in.
|
||
Updated•4 years ago
|
Description
•