1471812 - Remove defer usage in webconsole panel initialization

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P3)

Description

[Nicolas Chevobbe [:nchevobbe]]

We should probably be able to switch to an async function for the panel's open function

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1471812 - Remove defer usage in webconsole panel initialization; .

This patch removes the use of defer and switches the open function to
an async one, which makes things easier to follow.

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

Comment 2

[Alexandre Poirot [:ochameau]]

Comment on attachment 8988410 [details]
Bug 1471812 - Remove defer usage in webconsole panel initialization; .

https://reviewboard.mozilla.org/r/253706/#review260280

Thanks for this cleanup!
Looks good to me if try is happy with this change (who know what subtle races promise changes can introduce...)

::: devtools/client/webconsole/panel.js:63
(Diff revision 1)
> +      // Wait for the iframe to load if it's not already.
> -    if (!this.target.isRemote) {
> +      if (!this.target.isRemote) {
> -      promiseTarget = this.target.makeRemote();
> +        target = await this.target.makeRemote();
> -    } else {
> +      } else {
> -      promiseTarget = promise.resolve(this.target);
> +        target = this.target;
> -    }
> +      }

makeRemote seems to resolve a null value:
  https://searchfox.org/mozilla-central/source/devtools/client/framework/target.js#473
So I think this `target variable is useless and we only need:
  if (!this.target.isRemote) {
    await this.target.makeRemote();
  }

::: devtools/client/webconsole/panel.js:65
(Diff revision 1)
> -    // 2. Wait for the remote target.
> -    // 3. Open the Web Console.
> -    return deferredIframe.promise
> -      .then(() => promiseTarget)
> -      .then((target) => {
> -        this._frameWindow._remoteTarget = target;
> +      this._frameWindow._remoteTarget = target;

This attribute is no longer used.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8988410 [details]
Bug 1471812 - Remove defer usage in webconsole panel initialization; .

https://reviewboard.mozilla.org/r/253706/#review260280

Thanks for the review and for the extra pair of eyes. The code looks even better now.
Will puh again to TRY to see if everything is okay

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8988410 [details]
Bug 1471812 - Remove defer usage in webconsole panel initialization; .

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/319854ead979
Remove defer usage in webconsole panel initialization; r=ochameau.

Comment 6

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/319854ead979