simple-storage not being saved on upgrade FF31+

RESOLVED FIXED

Status

Add-on SDK
General
P1
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Brad McDermott, Assigned: irakli)

Tracking

({dataloss, regression})

unspecified
dataloss, regression

Firefox Tracking Flags

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

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
Created attachment 8479262 [details]
simple-storage-test.zip

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.
(Reporter)

Updated

3 years ago
Severity: normal → critical
If you have the time you might want to help further by finding the regressor using http://mozilla.github.io/mozregression/
Keywords: regression, regressionwindow-wanted

Comment 2

3 years ago
[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
status-firefox32: --- → affected
status-firefox33: --- → affected
status-firefox34: --- → affected
status-firefox35: --- → affected
status-firefox-esr31: --- → affected
tracking-firefox32: --- → ?
tracking-firefox34: --- → ?
tracking-firefox35: --- → ?
tracking-firefox-esr31: --- → ?
Ever confirmed: true
Keywords: regressionwindow-wanted → dataloss

Comment 3

3 years ago
[Tracking Requested - why for this release]:
tracking-firefox33: --- → ?

Updated

3 years ago
Flags: needinfo?(evold)
This is probably due to bug 893276.
Flags: needinfo?(evold) → needinfo?(dtownsend+bugmail)
status-firefox32: affected → wontfix
tracking-firefox32: ? → -
tracking-firefox33: ? → +
tracking-firefox34: ? → +
tracking-firefox35: ? → +
tracking-firefox-esr31: ? → +
(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)

Updated

3 years ago
Assignee: nobody → rFobic
Priority: -- → P1
I've backed out bug 893276 from all branches.
status-firefox33: affected → fixed
status-firefox34: affected → fixed
status-firefox35: affected → fixed
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(rFobic)
Resolution: --- → FIXED
This does not meet ESR landing criteria.
status-firefox-esr31: affected → wontfix
tracking-firefox-esr31: + → ---
You need to log in before you can comment on or make changes to this bug.