1814608 - Inline script may easily have empy breakable lines

Status: RESOLVED FIXED

(DevTools :: Debugger, task)

Description

[Alexandre Poirot [:ochameau]]

STR:

• reload many times on http://techno-barje.fr/fission/inline/ with the html page opened in the debugger

Notice the empty breakable lines on the second inline script.
Note that it seems to be working more frequently when you set a breakpoint.
This seems to come from this workaround which isn't working on this test page:
https://searchfox.org/mozilla-central/rev/11cf68787bd00edfa7ac6a0ecd07794698cdebdc/devtools/client/debugger/src/actions/sources/newSources.js#323-329

There is many issues here...

• breakable lines reducer is confusing. It is only about source actor IDs, but this isn't extra clear when reading the reducer module.
• breakable lines actions setBreakableLines is called from random places, whereas it sounds like it should be called after text content is retrieved?
• for some reason SourceActor.getBreakableLines doesn't work if you call it early. It may depend on calling getSourceContent first??
• source actor still clone its internal maps, which is an unecesarry significant perf issue.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1814608 - [devtools] Fix breakable lines of inline script.

Breakable lines could easily be missed when reloading.
The very original issue is that SourceActor.getBreakableLines can't be called too early during the page load.
The SourceActor may not have received enough content of the HTML page and the HTML string may not
include the inline script sources itself.
This lead to wrong _startLineColumnDisplacement being set to zero and breakable lines
of the inline script are starting from the very first line of the HTML page, instead of where the <script> starts.

Then... the frontend somewhat worksaround that very implicitely and not correctly.
The key is to force fetching the breakable lines after the source text content is fetched.

Covering this isn't trivial as this is a race condition.
But I expanded the coverage in feature tests and tweaked browser_dbg-inline-script-offset.js
which could fail intermittently and no longer fail with this patch and the next one applied.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1814608 - [devtools] Simplify source actor breakable line computation.

• Remove the usage of PROMISE action, which isn't used
• Remove the memoizedAction as the reducer is already memoizing the breakableLines

All of this is only useful if we call setBreakableLines concurrently while
this is already in process of being loading.
This may still be theoriticaly possible between newSources and select actions.
But we should rather better coordinate when we pull breakable lines rather than
introduce such complexity. i.e. reduce the number of callsites requiring to load them.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/abc019424cec
[devtools] Simplify source actor breakable line computation. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/2e41f4a14afd
[devtools] Fix breakable lines of inline script. r=bomsy,devtools-reviewers

Comment 4

[Noemi Erli[:noemi_erli]]

Backed out 4 changesets (Bug 1814607, Bug 1814608, Bug 1809168) for causing devtools failures CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=8f7f098d9af03eed0fc6fc4c1e71734e797aa215&searchStr=os%2Cx%2C10.15%2Cwebrender%2Copt%2Cmochitests%2Cwith%2Cnetworking%2Con%2Csocket%2Cprocess%2Ctest-macosx1015-64-qr%2Fopt-mochitest-devtools-chrome-spi-nw%2Cdt1

Log: https://treeherder.mozilla.org/logviewer?job_id=406913328&repo=autoland&lineNumber=2297
https://treeherder.mozilla.org/logviewer?job_id=406914933&repo=autoland&lineNumber=2673

Backout: https://hg.mozilla.org/integration/autoland/rev/219bc96600a47cd4b95ac71b0b378db961481b33

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3403f3c933da
[devtools] Simplify source actor breakable line computation. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/cef8502055c5
[devtools] Fix breakable lines of inline script. r=bomsy,devtools-reviewers

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/3403f3c933da
https://hg.mozilla.org/mozilla-central/rev/cef8502055c5