Last Comment Bug 748085 - Previously installed apps no longer work after updating Nightly
: Previously installed apps no longer work after updating Nightly
Status: VERIFIED FIXED
[qa!]
: regression
Product: Firefox Graveyard
Classification: Graveyard
Component: Webapp Runtime (show other bugs)
: unspecified
: All Mac OS X
: -- normal
: ---
Assigned To: Myk Melez [:myk] [@mykmelez]
: Jason Smith [:jsmith]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-23 13:15 PDT by Justin Scott [:fligtar]
Modified: 2016-03-21 12:39 PDT (History)
6 users (show)
jsmith: in‑moztrap+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch v1: naive fix (836 bytes, patch)
2012-04-25 17:11 PDT, Myk Melez [:myk] [@mykmelez]
felipc: review+
Details | Diff | Splinter Review

Description Justin Scott [:fligtar] 2012-04-23 13:15:08 PDT
I have 20 apps installed on my Mac from last week's Nightly. Today I updated Nightly and none of the apps will run. Their icon briefly appears in the dock and then disappears right after.
Comment 1 Dan Walkowski 2012-04-23 15:23:18 PDT
I have discovered that this was due to old apps having a different installrecord format than new ones.  Force a reinstall of the old apps and a new install record should be written and the app should work again.
Comment 2 Ed Lee :Mardak 2012-04-23 16:06:55 PDT
From bug 746771?
Comment 3 Justin Scott [:fligtar] 2012-04-23 16:44:29 PDT
Yeah, new installs work fine. As we're planning to allow real users to begin installing apps pretty soon, are there any other changes like this planned? We should probably lock this down.
Comment 4 Myk Melez [:myk] [@mykmelez] 2012-04-23 16:48:18 PDT
(In reply to Justin Scott [:fligtar] from comment #3)
> Yeah, new installs work fine. As we're planning to allow real users to begin
> installing apps pretty soon, are there any other changes like this planned?
> We should probably lock this down.

We aren't planning other changes, nor did we plan this one, which was due to a bug in one of the patches for the initial implementation.  However, during the current phase of development, when the feature is only in nightly builds, we can't absolutely rule it out.
Comment 5 :Felipe Gomes (needinfo me!) 2012-04-23 16:57:43 PDT
I'm not sure though how the change from bug 746771 could have caused that
Comment 6 Myk Melez [:myk] [@mykmelez] 2012-04-23 17:09:56 PDT
Hmm, indeed, Felipe is right.  Bug 746771 is not the cause of this problem, because the registryDir property (née app.profile) is not in the startup path of the runtime, so the absence of the property would not prevent startup.
Comment 7 Dan Walkowski 2012-04-24 10:25:34 PDT
It's not the runtime that is failing, as far as I can tell. Firefox is failing to load the webapp.  The webapprt launcher completes, and sends the correct command line to Firefox and exits with no errors.
Comment 8 Myk Melez [:myk] [@mykmelez] 2012-04-24 10:28:58 PDT
fligtar: can you attach an archive of the app package (i.e. /Applications/Name.app) for one of the failing apps to this bug and then email Dan and I a copy of its data dir (i.e. ~/Library/Application Support/[origin]/)?
Comment 9 Myk Melez [:myk] [@mykmelez] 2012-04-24 10:54:18 PDT
fligtar: erm, never mind, you already provided Dan with this information, and he has provided it to me!
Comment 10 Jason Smith [:jsmith] 2012-04-24 15:04:33 PDT
For tracking purposes, Justin, could you still attach the archive of the app package to this bug? I'll want to know what I need to test when verifying this bug after it is fixed.
Comment 11 Myk Melez [:myk] [@mykmelez] 2012-04-24 16:59:52 PDT
With the files fligtar provided, I dug into this and figured out that it's a regression from the combination of bug 746771, which fixed a bug in the way an app identifies the webapp registry, and bug 746213, which puts identification of the webapp registry into the startup path of the runtime, specifically into the CommandLineHandler component, via an attempt to import the Webapps.jsm module.

When imported, that module tries to retrieve its datastore directory.  If that attempt throws an exception, as it might for apps installed before the fix for bug 746771 landed, the exception propagates through the component and up to the component loader, which writes an error like this one to the console:

JS Component Loader: ERROR resource://gre/modules/FileUtils.jsm:60
                     NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]

The GRE then exits because it needs the command line handler to start the app.

A naive fix (catch the exception) is trivial, and I'll implement one if needed, although I'd like to think a bit about how best to fix this first, since it'll happen under other circumstances beside the one that triggered this bug (f.e. if the user deletes and recreates their profile).

jsmith: no need to get the archive from fligtar; you can test this with any app.  Just open its webapp.json file and remove the registryDir property.
Comment 12 Jason Smith [:jsmith] 2012-04-25 11:34:14 PDT
For tracking purposes, this bug will relate the Mac issue fligtar hit.
Comment 13 Myk Melez [:myk] [@mykmelez] 2012-04-25 17:11:02 PDT
Created attachment 618485 [details] [diff] [review]
patch v1: naive fix

Here's the naive fix, which is worth taking even while we figure out how to make this better.
Comment 14 :Felipe Gomes (needinfo me!) 2012-04-25 19:17:50 PDT
Myk, I'm trying to understand this bug, let me know if I got it right:

it happened after the fix for bug 746213, which makes it try to read the .json file, and happens only to apps installed before the fix for bug 746771 (i.e. ones that do not contain registryDir).

If so, technically we don't need to do anything to fix this bug, and you're adding this try catch to prevent a similar problem from happening in the future. Is that right?
Comment 15 Myk Melez [:myk] [@mykmelez] 2012-04-26 11:07:46 PDT
That's just about right, except that this patch does fix the problem for those old installs in addition to preventing a similar problem from happening in the future.  I want a better fix in the future as part of a general improvement to registry initialization that has these characteristics:

1. continues to prevent failure in registry initialization from affecting startup;
2. makes registry initialization more robust against changes to the location of the user's default profile;
3. makes registry initialization asynchronous.

But I think this patch is worth taking in the meantime.
Comment 16 :Felipe Gomes (needinfo me!) 2012-04-26 11:14:52 PDT
Comment on attachment 618485 [details] [diff] [review]
patch v1: naive fix

yeah, we're gonna have to think of a better mechanism in the future, maybe versioning the format. The problem is that when this happens, the profile data will be unavailable to the app, right? So a lot of things on the app won't work even if it loads up. And the config file won't fix itself.. so we might have to trigger a file update
Comment 17 Myk Melez [:myk] [@mykmelez] 2012-04-26 11:34:08 PDT
The app profile (and datadir generally) remains available, it's just the Firefox profile that becomes unavailable.  And the app's install record and manifest are copied to its datadir on installation.  So most apps will work just fine.

The exception is apps like Marketplace that use the app management API to install, uninstall, etc. apps.  Those need access to the registry in order to do that work.

Also, in the future, we might start sharing other Firefox profile data with apps besides the registry, like proxy settings, in which case other app functionality might break when the Firefox profile becomes unavailable.

We'll need to do some hard thinking about what information we want to share and how to do it (which may involve moving it out of the profile directory, so it isn't profile-specific, and/or sharing it via a service).
Comment 18 Myk Melez [:myk] [@mykmelez] 2012-04-26 11:41:08 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/cdb6c708164d
Comment 19 Ed Morley [:emorley] 2012-04-27 07:05:52 PDT
https://hg.mozilla.org/mozilla-central/rev/cdb6c708164d
Comment 20 Jason Smith [:jsmith] 2012-05-12 08:36:25 PDT
Clarification - To verify this bug, I need to grab a nightly build from around 4/17, install apps, update to the latest nightly, and verify I can run them, correct? Or is there something else I should be doing?
Comment 21 Myk Melez [:myk] [@mykmelez] 2012-05-14 10:19:48 PDT
(In reply to Jason Smith [:jsmith] from comment #20)
> Clarification - To verify this bug, I need to grab a nightly build from
> around 4/17, install apps, update to the latest nightly, and verify I can
> run them, correct? Or is there something else I should be doing?

That sounds right!
Comment 22 Jason Smith [:jsmith] 2012-05-18 17:30:23 PDT
Verified on OS X 10.7.

Note You need to log in before you can comment on or make changes to this bug.