1391768 - don't ignore result of getSources in source-map-url-service

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Tom Tromey :tromey]

Currently source-map-url-services calls getSources but ignores the result.
This will fail to notice a source map if the source actor already existed
when the service is created.

Comment 1

[Tom Tromey :tromey]

Attachment: Bug 1391768 - register existing source actors in source map service init;

Review commit: https://reviewboard.mozilla.org/r/174356/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/174356/

Comment 2

[Brian Grinstead [:bgrins]]

Comment on attachment 8902682 [details]
Bug 1391768 - register existing source actors in source map service init;

https://reviewboard.mozilla.org/r/174356/#review179524

::: devtools/client/framework/source-map-url-service.js:39
(Diff revision 1)
>    target.on("will-navigate", this.reset);
>  
>    Services.prefs.addObserver(SOURCE_MAP_PREF, this._onPrefChanged);
>  
>    // Start fetching the sources now.
> -  this._loadingPromise = new Promise(resolve => {
> +  this._loadingPromise = threadClient.getSources(({sources}) => {

What are the cases where getSources will reject? I ask because we comment that we want to ignore errors but we don't ever `.catch()`, so it would throw while awaiting in `originalPositionFor`.

::: devtools/client/framework/test/browser_source_map-init.js:34
(Diff revision 1)
> +const ORIGINAL_LINE = 11;
> +
> +add_task(function* () {
> +  // Opening the debugger causes the source actors to be created.
> +  const toolbox = yield openNewTabAndToolbox(PAGE_URL, "jsdebugger");
> +  // In the bug, when the sourceMapURLService was created, it was

Include bug # in this comment

Comment 3

[Tom Tromey :tromey]

Comment on attachment 8902682 [details]
Bug 1391768 - register existing source actors in source map service init;

https://reviewboard.mozilla.org/r/174356/#review179524

> What are the cases where getSources will reject? I ask because we comment that we want to ignore errors but we don't ever `.catch()`, so it would throw while awaiting in `originalPositionFor`.

I don't know when `getSources` can have an error; but in any case it doesn't reject the promise, instead it fills out the response in a different way.  See https://dxr.mozilla.org/mozilla-central/rev/db7f19e26e571ae1dd309f5d2f387b06ba670c30/devtools/shared/client/main.js#250

I see now I made a small error here, which I'll correct.

Comment 4

[Tom Tromey :tromey]

Comment on attachment 8902682 [details]
Bug 1391768 - register existing source actors in source map service init;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/174356/diff/1-2/

Comment 5

[Tom Tromey :tromey]

Clearly I didn't consider the possibility of a protocol rejection, rather than a server rejection.

Comment 6

[Tom Tromey :tromey]

Comment on attachment 8902682 [details]
Bug 1391768 - register existing source actors in source map service init;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/174356/diff/2-3/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fcd4aabcec11
register existing source actors in source map service init; r=bgrins

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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