Closed Bug 962790 Opened 6 years ago Closed 6 years ago

"reference to undefined property installed.apps" on uninstall

Categories

(Firefox for Android :: Web Apps, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 29

People

(Reporter: myk, Assigned: myk)

Details

Attachments

(1 file)

I recently saw this message when uninstalling an app with synthetic APKs enabled (and the latest changes on inbound):

[JavaScript Warning: "ReferenceError: reference to undefined property installed.apps" {file: "resource://gre/modules/WebappManager.jsm" line: 178}]

I wonder if this is another regression from bug 959420, since WebappManager.jsm can be lazily loaded; and if it's loaded lazily, imports Webapps.jsm, and then immediately calls DOMApplicationRegistry.doGetAll in autoUninstall, then bug 959420 means the registry probably isn't ready to provide a list of installed apps.

Although it should still provide an empty list in that case, so this is puzzling.
Even if bug 959420 didn't cause this particular problem, I think it means that we can't ever count on the registry being ready unless its new registryReady promise is resolved, so any DOMApplicationRegistry calls that might happen soon after importing Webapps.jsm need to wait until registryReady.

Here's a patch that fixes those calls in WebappManager.  Requesting feedback from Marco to confirm my suspicion that we'll need to do this in general when accessing DOMApplicationRegistry from now on.

WesJ: this is another case where the whitespace-excluding diff shows that these changes simply wrap a simple promise chain around a block of indentation-only changes in three different places:

+    DOMApplicationRegistry.registryReady.then(() => {
… (indentation-only changes)
+    });
Assignee: nobody → myk
Status: NEW → ASSIGNED
Attachment #8363999 - Flags: review?(wjohnston)
Attachment #8363999 - Flags: feedback?(mar.castelluccio)
Comment on attachment 8363999 [details] [diff] [review]
0001-Bug-962790-ensure-registryReady-before-calling-DOMAp.patch

We need to wait whenever we need to use the registry soon after startup (even before bug 959420 landed).
Attachment #8363999 - Flags: feedback?(mar.castelluccio) → feedback+
(In reply to Marco Castelluccio [:marco] from comment #2)
> We need to wait whenever we need to use the registry soon after startup
> (even before bug 959420 landed).

Hmm, I think we'll actually have to check registryReady now whenever we access the registry, even if it's long after startup, unless we know for sure that the registry must already have been initialized, since it doesn't necessarily get initialized on startup.
(In reply to Myk Melez [:myk] [@mykmelez] from comment #3)
> (In reply to Marco Castelluccio [:marco] from comment #2)
> > We need to wait whenever we need to use the registry soon after startup
> > (even before bug 959420 landed).
> 
> Hmm, I think we'll actually have to check registryReady now whenever we
> access the registry, even if it's long after startup, unless we know for
> sure that the registry must already have been initialized, since it doesn't
> necessarily get initialized on startup.

That's correct, we need to wait unless we're sure the registry has been loaded (in the desktop case we load it at startup, in Firefox for Android too if I recall correctly).
(In reply to Marco Castelluccio [:marco] from comment #4)
> That's correct, we need to wait unless we're sure the registry has been
> loaded (in the desktop case we load it at startup, in Firefox for Android
> too if I recall correctly).

Synthetic APKs does it more lazily now in Fennec.  But even if it didn't, it sometimes launches Fennec and then immediately accesses the registry, so loading the registry aggressively on startup still wouldn't solve this problem.
Comment on attachment 8363999 [details] [diff] [review]
0001-Bug-962790-ensure-registryReady-before-calling-DOMAp.patch

Review of attachment 8363999 [details] [diff] [review]:
-----------------------------------------------------------------

This seems likely to be forgotten often. All of my solutions also feel like they might too though...
Attachment #8363999 - Flags: review?(wjohnston) → review+
(In reply to Wesley Johnston (:wesj) from comment #6)
> Comment on attachment 8363999 [details] [diff] [review]
> 0001-Bug-962790-ensure-registryReady-before-calling-DOMAp.patch
> 
> Review of attachment 8363999 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This seems likely to be forgotten often. All of my solutions also feel like
> they might too though...

Yup.  Marco and I are both thinking about this, although we haven't come up with a solution yet.
https://hg.mozilla.org/mozilla-central/rev/a85a91d17061
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
(In reply to Myk Melez [:myk] [@mykmelez] from comment #7)
> Yup.  Marco and I are both thinking about this, although we haven't come up
> with a solution yet.

A possible but not beautiful solution would be to make all DOMApplicationRegistry public methods wait on the registryLoaded promise before doing anything.
You need to log in before you can comment on or make changes to this bug.