1445318 - Remove storage: "persistent" from AddonStudies.jsm

Status: RESOLVED FIXED

(Firefox :: Normandy Client, enhancement, P1)

Description

[Johann Hofmann [:johannh]]

As part of deprecating the storage option for indexedDB.open(), we would like to remove the usage of storage: "persistent" in AddonStudies.jsm.

Since that code seems to run on chrome principal there should not be any bad consequences like data loss, but before I make a patch I'd like to be able to end-to-end test this to be sure.

Mike, what's the easiest way I can test the functionality of that database?

Thanks!

Comment 1

[Johann Hofmann [:johannh]]

Code is here: https://searchfox.org/mozilla-central/rev/99df6dad057b94c2ac4d8b12f7b0b3a7fdf25d04/toolkit/components/normandy/lib/AddonStudies.jsm#51

Comment 2

[Michael Cooper [:mythmon]]

Hi Johann, thanks for looking into this. Probably the easiest way to test this is to interface with a Normandy server, creating a recipe there that would utilize this storage. Then after the patch, you could make sure that the code using the storage acts correctly before and after the patch.

To get access to a server, you have 3 choices

1) We can get you access to the Normandy staging server, where you can create a recipe that will target your builds of Firefox.
2) You could set up a local instance of Normandy server, so you can test it entirely locally.
3) I can work with you directly to set up a recipe on the staging server to test this.

The code in question uses that database to maintain state about studies between runs of Firefox. The biggest thing you could test is to get enrolled in a study, then use about:studies to unenroll. After that, you want to make sure that about:studies continues to show the study, and that you don't get re-enrolled. Both of these behaviors rely on this storage.

Alternatively, you could probably trigger the database to be used via a browser console. That seems like a weaker test to me though. If you'd rather explore this instead, I can help with that too.

Comment 3

[Johann Hofmann [:johannh]]

Attachment: Bug 1445318 - Remove storage: "persistent" from AddonStudies.jsm.

This option is deprecated and we would like to remove it soon.
Since AddonStudies.jsm runs with system privileges, it is getting
the same storage type ("persistent"), no matter the storage option.

I verified this through manual testing.

Review commit: https://reviewboard.mozilla.org/r/230804/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/230804/

Comment 4

[Johann Hofmann [:johannh]]

Thanks for the help getting a test study up and running! I manually verified that this does not change the database type, but you might want to do another quick check as well.

Comment 5

[Michael Cooper [:mythmon]]

Comment on attachment 8961961 [details]
Bug 1445318 - Remove storage: "persistent" from AddonStudies.jsm.

https://reviewboard.mozilla.org/r/230804/#review237156

I tested this locally, and it seems to work for me too. Thanks for the patch!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ac144929b5b
Remove storage: "persistent" from AddonStudies.jsm. r=mythmon

Comment 7

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/3ac144929b5b