1058840 - simple-storage not being saved on upgrade FF31+

Status: RESOLVED FIXED

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

Description

[Brad McDermott]

Attachment: 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.

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

If you have the time you might want to help further by finding the regressor using http://mozilla.github.io/mozregression/

Comment 2

[Alice0775 White]

[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

Comment 3

[Alice0775 White]

[Tracking Requested - why for this release]:

Comment 4

[Erik Vold [:erikvold] (please needinfo? me)]

This is probably due to bug 893276.

Comment 5

[Dave Townsend [:mossop]]

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

Comment 7

[Erik Vold [:erikvold] (please needinfo? me)]

(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?

Comment 8

[Dave Townsend [:mossop]]

I've backed out bug 893276 from all branches.

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

This does not meet ESR landing criteria.