1642039 - UpdateManager: Convert the call stacks to asynchronous functions.

Status: ASSIGNED

(Toolkit :: Application Update, task, P3)

Description

[Alex Vincent [:WeirdAl]]

This involves adding async and await keywords as necessary, and possibly some IDL changes as we go. (BrowserHandler.defaultArgs is one known case.)

Comment 1

[Alex Vincent [:WeirdAl]]

Doing this by hand is time-consuming and risky. I'm going to attempt writing an automated static analysis tool, "stacklizard", to assist.

https://github.com/ajvincent/stacklizard/

Comment 2

[Alex Vincent [:WeirdAl]]

One aspect of this that's going to be harder than I anticipated is that we can't mark constructors async, period:

function f() {
  function P() {
    // await setTimeout(() => null, 1000);
    this.q = 1;
  }

  var F = new P();
  alert(F.q);
}

Uncommenting the await means that function P() must become async function P(), and that means TypeError: P is not a constructor gets thrown. This is precisely why I'm writing a static analysis tool, but points to another blocking problem that I need advice to solve in the XPCOM world. Maybe we have to do this one in tandem with the introduction of UpdateManagerInit in bug 1642037.

Comment 3

[Alex Vincent [:WeirdAl]]

Attachment: Analysis of needed async/await keywords

This is an auto-generated analysis of where we will need async and await keywords, based on tonight's mozilla-central.

The tool I used to create this analysis is one of my own design, named StackLizard, heavily using several tools mozilla-central already uses for eslint. This is its raison d'être.

I'd like some feedback before I start adding in these keywords. It's not perfect. StackLizard missed looking for instances of the browser's AppUpdater() constructor, for one, and that's one problem point.

Comment 4

[Robin Steuber (they/them) [:bytesized]]

There's a lot going on in your attachment. Just looking through a few lines, it seems like this is probably doing what you want, but I'm not keen on checking everything, given the quantity. Was there anything in particular you wanted me to look at? Or was there particular feedback you were hoping for?

Comment 5

[Alex Vincent [:WeirdAl]]

I was hoping for (1) overall design points on whether a different approach than just marking async or await might be preferable, and (2) introduction to appropriate peers of other modules to weigh in on those different approaches for their areas of scope.

Comment 6

[Robin Steuber (they/them) [:bytesized]]

I do still think that getting the necessary async/await keywords in the right places is a good first step.

I'm not sure which modules you think we need peer input for. Are there places where we are going to require structural changes to other modules? We shouldn't really need peer input just to mark some functions as async. It would be nice if we could keep the patches specific to particular modules so we can more easily request review. But unless structural changes are needed, it doesn't seem like we need to bring in any module peers at this time.

Comment 7

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

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 8

[Suhaib Mujahid [:suhaib]]

Sorry, there was a problem with the detection of inactive users. I'm reverting the change.