1678550 - Load DAMP's root module using a DevTools browser-loader

Status: RESOLVED FIXED

(DevTools :: Framework, task, P3)

Description

[Julian Descottes [:jdescottes]]

Module paths in DAMP are dyamically generated.

Normally to get the path to a module, you need to rely on the rootURI of the extension. Eg rootURI.resolve("content/tests/some/test/path"). We use that in a few spots in DAMP and we also use relative paths our require calls to avoid using it too much.

However we migrated all DevTools code to use absolute require calls and we could leverage the DevTools loader to do the same here.

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1678550 - [devtools] Load damp.js using the main DevTools Loader

Depends on D97626

Using the BrowserLoader still allows DAMP.js to use the window globals, but makes it so that require is immediately available.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1678550 - [devtools] Add a damp-test require path to DAMP's browser-loader

Depends on D97727

The root DAMP path is dynamic, so we need to expose a new configuration option on the browser loader to add a damp-test root path

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1678550 - [devtools] Use absolute paths in DAMP require calls

Depends on D97728

Similar to what was done for the devtools/ folder, we should use absolute paths in all our require calls.

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1678550 - [devtools] Expose rootURI to all DAMP modules

Depends on D97729

rootURI is still necessary to register the JSWindowActors used to monitor load events.
Exposing it directly on the window object avoid having to pass it as an argument and will allow to further refactor the damp.js module.

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1678550 - [devtools] Use require instead of ChromeUtils.import in damp.js

Depends on D97730

Remove some leftover ChromeUtils.import usage

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1678550 - [devtools] Move nested require calls to the top of damp.js

Depends on D97731

We used to delay those calls because they needed rootURI to be set. This is no longer necessary.

Comment 7

[Julian Descottes [:jdescottes]]

Attachment: Bug 1678550 - [devtools] Add proper linting to damp.js and damp-test.js

Depends on D97732

Comment 8

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Can I have a talos push to try to see if it's not breaking aything?
Any regressions expected?

Comment 9

[Julian Descottes [:jdescottes]]

Try at https://treeherder.mozilla.org/jobs?repo=try&revision=4e6c988a5ec270fbf13738c281d6c897afc4b357&selectedTaskRun=LOcS8YjrRMeXWMf_ynhkkQ.0

Fission build failing on netmonitor since this doesn't include the workaround for the netmonitor test (Bug 1574417).
I'm also waiting for more results to see if the changes introduce any difference in the baselines of the tests.

I will request reviews from DevTools peers when this is ready for review.

Comment 10

[Julian Descottes [:jdescottes]]

I didn't notice the reviewer group was automatically flagged even when there are no reviewers set by the patch author. I will set the patches as changes planned for now.

Comment 11

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

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 12

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

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jdescottes, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 13

[Julian Descottes [:jdescottes]]

Will address final review comments and land this asap.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd09dd26dda7
[devtools] Load damp.js using the main DevTools Loader r=perftest-reviewers,nchevobbe,ochameau
https://hg.mozilla.org/integration/autoland/rev/08bd3a0bbfe1
[devtools] Use absolute paths in DAMP require calls r=perftest-reviewers,AlexandruIonescu,ochameau,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/aa690fe1bad7
[devtools] Use require instead of ChromeUtils.import in damp.js r=perftest-reviewers,nchevobbe,ochameau,AlexandruIonescu
https://hg.mozilla.org/integration/autoland/rev/d88937631d65
[devtools] Move nested require calls to the top of damp.js r=perftest-reviewers,nchevobbe,ochameau
https://hg.mozilla.org/integration/autoland/rev/9f036835186c
[devtools] Add proper linting to damp.js and damp-test.js r=perftest-reviewers,nchevobbe,ochameau

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/fd09dd26dda7
https://hg.mozilla.org/mozilla-central/rev/08bd3a0bbfe1
https://hg.mozilla.org/mozilla-central/rev/aa690fe1bad7
https://hg.mozilla.org/mozilla-central/rev/d88937631d65
https://hg.mozilla.org/mozilla-central/rev/9f036835186c