1079439 - Fix exception from bug 1052519 in Fx 17 and lower

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Bug 1052519 comment 22 has a pretty good summary of the issue:

> (In reply to Paul Silaghi, QA [:pauly] from comment #18)
> > The hotfix doesn't seem to work on versions 10-17.
> > In browser console I see:
> > "Error: [Exception... "Component returned failure code: 0x80004005
> > (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005
> > (NS_ERROR_FAILURE)"  location: "JS frame ::
> > resource://firefox-hotfix/update.jsm :: <TOP_LEVEL> :: line 1135"  data: no]
> > Source File: resource://firefox-hotfix/update.jsm
> > Line: 1135"
> > 
> > Works fine on FF 18-28.
> > Thoughts?
> 
> That could be a legitimate regression from bug 1052519, as line 1135 is
> calling Services.dirsvc.get("UpdRootD") and that appears to be throwing. Did
> UpdRootD not exist until Firefox 18? Let's investigate this in bug 1052519.

Comment 1

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: hotfix XPI v1

Comment 2

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch v1

The fix should be as simple as this.

:pauly - could you try the attached XPI and verify that you no longer see the issue in Firefox 17 and below? Just checking one version should be fine since we'll have to do another QA signoff of this XPI over in bug 1061975

Comment 3

[Paul Silaghi, QA [:pauly]]

FF 10, Win 7 x64
The XPI works, but I still see this error in the browser console after restarting with the new version:
[Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIFile.moveTo]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: resource://gre/components/nsUpdateServiceStub.js :: migrateOldUpdateDir/< :: line 126"  data: no] nsUpdateServiceStub.js:129
formatURL: Couldn't find value for key: OLD_VERSION nsURLFormatter.js:120

Comment 4

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to Paul Silaghi, QA [:pauly] from comment #3)
> FF 10, Win 7 x64
> The XPI works, but I still see this error in the browser console after
> restarting with the new version:
> [Exception... "Component returned failure code: 0x80520012
> (NS_ERROR_FILE_NOT_FOUND) [nsIFile.moveTo]"  nsresult: "0x80520012
> (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame ::
> resource://gre/components/nsUpdateServiceStub.js :: migrateOldUpdateDir/< ::
> line 126"  data: no] nsUpdateServiceStub.js:129
> formatURL: Couldn't find value for key: OLD_VERSION nsURLFormatter.js:120

It looks like we're trying to migrate a file or files that didn't exist in the old version. It seems to me like this is probably OK to ignore, but let's ask :bbondy.

:bbondy - you wrote the function that's throwing here. Does this look like an issue we need to resolve or do you think the hotfix is currently OK as is?

Comment 5

[Brian R. Bondy [:bbondy]]

Sorry I'm confused by this and need some help before I can give a good answer.

The code in the patch doesn't seem to be from anything in mozilla-central.
In particular I don't know of a path similar to: v20140527.01/resource/update.jsm
I also tried to dxr some related code and couldn't find it.

What you are talking about does sound somewhat familiar though.  We used to use the leaf installed folder name (Nightly) for the update dir name.  But when we wanted to support both x86 and x64 this no longer made sense.  So we migrated to a dir which is a hash of the install path instead.  In order to preserve update history we copied old update data to the new dir.

If this is related to that I'd probably say to simply ignore the error if you can't copy. It isn't critical, the user would just lose update history if it can't be migrated. 

Would appreciate some extra info for my own knowledge though in any case for how I'd apply this code and which repo to apply it to. Thanks Tim!

Comment 6

[Brian R. Bondy [:bbondy]]

Also I'm confused why the fix is for Fx17 and lower (the title). I thought we didn't support old versions.

Comment 7

[Benjamin Smedberg]

This is specifically about the update hotfix, which is delivered to users of Firefox 10+: https://hg.mozilla.org/releases/firefox-hotfixes/

Comment 8

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to Brian R. Bondy [:bbondy] from comment #5)
> Sorry I'm confused by this and need some help before I can give a good
> answer.
>
> The code in the patch doesn't seem to be from anything in mozilla-central.
> In particular I don't know of a path similar to:
> v20140527.01/resource/update.jsm
> I also tried to dxr some related code and couldn't find it.
>
> What you are talking about does sound somewhat familiar though.  We used to
> use the leaf installed folder name (Nightly) for the update dir name.  But
> when we wanted to support both x86 and x64 this no longer made sense.  So we
> migrated to a dir which is a hash of the install path instead.  In order to
> preserve update history we copied old update data to the new dir.
> 
> If this is related to that I'd probably say to simply ignore the error if
> you can't copy. It isn't critical, the user would just lose update history
> if it can't be migrated. 
> 
> Would appreciate some extra info for my own knowledge though in any case for
> how I'd apply this code and which repo to apply it to. Thanks Tim!

I should have given more context! This bug and the patch relate to an issue with the update hotfix. The attached XPI is the hotfix after incorporating the patch. :pauly tested the XPI/hotfix on Fx10, so the flow was something like this:
  1. Launch Fx10
  2. Apply update hotfix
  3. (hotfix updates Firefox to Fx32.0.3
  4. Shutdown Fx10
  5. Launch Fx (this time 32.0.3 launches)
  6. See the error pasted above in the browser console

We're interested in whether the error is something we should try to address. From your comments so far it sounds like it is something we can ignore. Let me know if this isn't the case!

Comment 9

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Comment on attachment 8505828 [details] [diff] [review]
Patch v1

Requesting review per video discussion earlier today

Comment 10

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

http://hg.mozilla.org/releases/firefox-hotfixes/rev/5985718e5f4d