Closed Bug 1471812 Opened 3 years ago Closed 3 years ago

Remove defer usage in webconsole panel initialization

Categories

(DevTools :: Console, enhancement, P3)

enhancement

Tracking

(firefox63 fixed)

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We should probably be able to switch to an async function for the panel's open function
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Priority: -- → P3
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.
Attachment #8988410 - Flags: review?(poirot.alex) → review+
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
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/319854ead979
Remove defer usage in webconsole panel initialization; r=ochameau.
https://hg.mozilla.org/mozilla-central/rev/319854ead979
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.