Closed Bug 1058840 Opened 5 years ago Closed 5 years ago

simple-storage not being saved on upgrade FF31+

Categories

(Add-on SDK Graveyard :: General, defect, P1, critical)

defect

Tracking

(firefox32- wontfix, firefox33+ fixed, firefox34+ fixed, firefox35+ fixed, firefox-esr31 wontfix)

RESOLVED FIXED
Tracking Status
firefox32 - wontfix
firefox33 + fixed
firefox34 + fixed
firefox35 + fixed
firefox-esr31 --- wontfix

People

(Reporter: bradmcdermott, Assigned: irakli)

References

Details

(Keywords: dataloss, regression)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2132.0 Safari/537.36

Steps to reproduce:

1. Run firefox with -console or -jsconsole and a new profile
2. Install fftest1.xpi
3. Install fftest2.xpi


Actual results:

The extension just saves the value 'true' to storage.test. It console logs the value before and after setting the value. It also notes when the extension is unloaded, logging the reason.
Here is the console output:

(First install)
SIMPLE STORAGE BEFORE SETTING TEST VARIABLE test = undefined
SIMPLE STORAGE AFTER SETTING TEST VARIABLE test = true
fftest onUnload. Reason: upgrade
(Second install)
SIMPLE STORAGE BEFORE SETTING TEST VARIABLE test = undefined
SIMPLE STORAGE AFTER SETTING TEST VARIABLE test = true

As you can see, storage.test is undefined when version 2 of the extension runs for the first time. The 'true' value should have persisted.



Expected results:

Here is the console log of what happens on FF30 and below:

(First install)
SIMPLE STORAGE BEFORE SETTING TEST VARIABLE test = undefined
SIMPLE STORAGE AFTER SETTING TEST VARIABLE test = true
fftest onUnload. Reason: upgrade
(Second install)
SIMPLE STORAGE BEFORE SETTING TEST VARIABLE test = true
SIMPLE STORAGE AFTER SETTING TEST VARIABLE test = true

This is the expected behavior. storage.test == true when running version 2 for the first time, since the value was saved in version 1. 

This bug seems to only happen in FF31 and higher, and only if you 1. haven't restarted firefox, 2. haven't disabled the extension, and 3. don't run the upgrade within 5 minutes of changing a simple storage value (since simple-storage saves every 5 minutes or in those two cases).

As you can see from the onUnload console log, it is being unloaded with the reason "upgrade", but this isn't causing simple storage to save like "disable" does for some reason.
Severity: normal → critical
If you have the time you might want to help further by finding the regressor using http://mozilla.github.io/mozregression/
[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

Regression window(fx)
Good:
https://hg.mozilla.org/integration/fx-team/rev/8364bff2be23
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 ID:20140425110908
Bad:
https://hg.mozilla.org/integration/fx-team/rev/2fc524c6144b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 ID:20140425114608
Pushlog:
http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=8364bff2be23&tochange=2fc524c6144b

Regressed by:
2fc524c6144b	Erik Vold — Bug 1001074 - Uplift Add-on SDK to Firefox r=me
Blocks: 1001074
Status: UNCONFIRMED → NEW
Ever confirmed: true
[Tracking Requested - why for this release]:
Flags: needinfo?(evold)
This is probably due to bug 893276.
Flags: needinfo?(evold) → needinfo?(dtownsend+bugmail)
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #4)
> This is probably due to bug 893276.

We probably need to make simple-storage do a synchronous write to disk on unload to fix this. I'm swamped with other stuff right now though so it might be best to just back out but 893276 in the meantime.
Flags: needinfo?(dtownsend+bugmail)
Duplicate of this bug: 1059525
(In reply to Dave Townsend [:mossop] from comment #5)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #4)
> > This is probably due to bug 893276.
> 
> We probably need to make simple-storage do a synchronous write to disk on
> unload to fix this. I'm swamped with other stuff right now though so it
> might be best to just back out but 893276 in the meantime.

Irakli, can you revert the patch in bug 893276?
Flags: needinfo?(rFobic)
Assignee: nobody → rFobic
Priority: -- → P1
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(rFobic)
Resolution: --- → FIXED
This does not meet ESR landing criteria.
You need to log in before you can comment on or make changes to this bug.