746860 - Bookmarks are cleared if you move fennec to the SDcard

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

1. install native fennec
2. visit a couple of sites and bookmark them
3. quit fennec
4. move fennec to the sdcard
5. launch fennec
6. go to the awesome page

Expected: the sites visited in step 2 should be seen
Actual: the history, top sites, bookmarks are all reset

Nexus S, Galaxy S II : Nightly 4/18/2012

Comment 1

[Wesley Johnston (:wesj)]

This is from todays nightly, which doesn't have the patch from bug 721084 right?

Comment 2

[Aaron Train [:aaronmt]]

bug 721084 has not hit m-c yet (04/19) so Naoki's Nightly doesn't have the patch. This can be retested and resolved with an inbound build from today (04/19).

Comment 3

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

ah. ya.  I forgot about that.  Marking this as a dup.

Comment 4

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Reopening.  bug 721084 did not fix the issue.  I had fennec on the SDcard, upgraded, move to the internal, then move back to the SDCard, clicked on the history and ran into a crash/bug 746946.  In the logcat I found that it was because there was an error moving the profile:

04-20 14:29:16.586: D/GeckoProfile(1756): [GeckoProfile.java:213:moveProfilesFrom()] Moving old profile directories from /mnt/sdcard/Android/data/org.mozilla.fennec/files/mozilla
04-20 14:29:16.586: E/GeckoProfile(1756): [GeckoProfile.java:239:moveDirContents()] Unable to create folder /data/data/org.mozilla.fennec/files/mozilla/0uihfyls.default while moving profiles
04-20 14:29:16.586: W/GeckoApp(1756): [GeckoApp.java:213:getPluginDirectories()] zerdatime 1404054 - start of getPluginDirectories

Comment 5

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Noming due to data loss

Comment 6

[Gian-Carlo Pascutto [:gcp]]

Is see some (qa?) comments spread through various unrelated bugs which seem to suggest pretty much every migration scenario where the user had his data on the SDcard is broken with total dataloss due to bug 721084.

So this is hard blocker. (Or we revert bug 721084 :-P)

Comment 7

[Gian-Carlo Pascutto [:gcp]]

That "Unable to create folder" is a trival bug to fix. But while testing it I found another migration scenario that fails spectacularly with similar results:

1) Install XUL Fennec
2) Sync
3) Move XUL Fennec to SDcard
4) Upgrade to Native Fennec

This produces the following log on startup:

D/GeckoProfile(12309): Found profile dir: /data/data/org.mozilla.fennec/files/mozilla/fpf70rmf.default
D/GeckoProfile(12695): Created new profile dir at /data/data/org.mozilla.fennec_morbo/files/mozilla/b3x3rtgy.default
W/GeckoProfile(12695): zerdatime 364416770 - start check sessionstore.js exists
I/GeckoApp(12695): Checking profile migration in: /data/data/org.mozilla.fennec_morbo/files/mozilla/b3x3rtgy.default
I/ProfileMigrator(12695): Opening path: /data/data/org.mozilla.fennec_morbo/files/mozilla/b3x3rtgy.default/places.sqlite
I/ProfileMigrator(12695): No database
D/GeckoProfile(12695): Moving old profile directories from /mnt/sdcard/Android/data/org.mozilla.fennec_morbo/files/mozilla
I/GeckoApp(12695): Startup mode: NEW_PROFILE

We're racing moving the profile with startup *and* profile migration. If moving the profile takes more than an instant, we'll conclude there is nothing to migrate.

After discussing with Brad, we're going to pull all profile-on-SDcard related code into Profile Migration, and that and only that will be responsible for moving the data back to internal storage.

Comment 8

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. Allow moving into existing directories.

Comment 9

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 2. Move the moving code into Profile Migration. Check migration status on every profile.getDir request.

Comment 10

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 622082 [details] [diff] [review]
Patch 1. Allow moving into existing directories.

[Approval Request Comment]
User impact if declined: massive dataloss
Risk to taking this patch (and alternatives if risky): massive dataloss

Comment 11

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 622083 [details] [diff] [review]
Patch 2. Move the moving code into Profile Migration. Check migration status on every profile.getDir request.

[Approval Request Comment]
User impact if declined: massive dataloss
Risk to taking this patch (and alternatives if risky): massive dataloss

Comment 12

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 622083 [details] [diff] [review]
Patch 2. Move the moving code into Profile Migration. Check migration status on every profile.getDir request.

Review of attachment 622083 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/ProfileMigrator.java
@@ +384,5 @@
> +    private class MoveProfileTask implements Runnable {
> +
> +        protected void moveProfilesToAppInstallLocation() {
> +            // check normal install directory
> +            moveProfilesFrom(new File("/data/data/" + mContext.getPackageName()));

why would we ever have to move the profile from internal storage to internal storage? Either comment as to how this is possible or drop this call.

Comment 13

[Gian-Carlo Pascutto [:gcp]]

Looks like a leftover from when we went in to other direction too. Dropped.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4cd27991b1c9
https://hg.mozilla.org/integration/mozilla-inbound/rev/896547412c7f

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4cd27991b1c9
https://hg.mozilla.org/mozilla-central/rev/896547412c7f

Comment 15

[Alex Keybl [:akeybl]]

We are leaving all non-beta+ bugs nominated for Aurora approval in the queue until FN14 Beta 1 is signed off on by QA.

Comment 16

[Mark Finkle (:mfinkle) (use needinfo?)]

Please verify this bug is fixed.

Comment 17

[Brad Lassey [:blassey] (use needinfo?)]

going to wait until QA verifies this bug before approving for aurora.

qawanted to get the verification.

Comment 18

[Adrian Tamas (:AdrianT)]

Verified using both sync set up and no sync account set. Moving the app to SD card no longer removes the history and bookmarks.

Nightly 15.0a1 2012-05-13
Device: HTC Desire Z/HTC Desire
OS: Android 2.3.3/Android 2.2.2

Leaving unverified because the patch was not added to Aurora.

Comment 19

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/14759622c2e5
https://hg.mozilla.org/releases/mozilla-aurora/rev/fe0ca3242893