Closed Bug 1062984 Opened 5 years ago Closed 5 years ago

[System] Browser bookmarks not migrated to homescreen icons on OTA from 2.0 to 2.1

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.1+, firefox33 wontfix, firefox34 fixed, firefox35 fixed, b2g-v2.1 verified, b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S4 (12sep)
blocking-b2g 2.1+
Tracking Status
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
b2g-v2.1 --- verified
b2g-v2.2 --- fixed

People

(Reporter: bzumwalt, Assigned: qdot)

References

Details

(Keywords: dataloss, Whiteboard: [2.1-exploratory][systemsfe][2.1-flame-test-run-2])

Attachments

(2 files, 2 obsolete files)

Attached file Logcat
Description:
When user has existing browser bookmarks on 2.0 then performs an OTA update to 2.1, the bookmarks are not migrated to homescreen icons after update. Any existing homescreen icons migrate correctly, but no new homescreen icons are created for existing browser bookmarks.
   
Repro Steps:
1) Update a Flame device to 2.0 BuildID: 20140822000206
2) Change update channel and url on device to appropriate 2.1 build (20140904000203)
3) Ensure that Browser bookmarks exist, if not, create one or more bookmarks
4) Perform OTA update
  
Actual:
Existing browser bookmarks are not migrated to homescreen icons after 2.0 to 2.1 OTA.
  
Expected: 
Browser bookmarks become homescreen icons after 2.0 to 2.1 OTA.
  
Environmental Variables:
Device: Flame 2.1 (319mb)
BuildID: 20140904000203
Gaia: a47ecb6368c015dd72148acde26413fd90ba3136
Gecko: 757931d0149e
Version: 34.0a2 (2.1)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Repro frequency: 5/5, 100%
See attached: Youtube video clip & logcat
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
From logcat:

---

09-04 09:57:54.826: E/GeckoConsole(2512): Content JS LOG at app://system.gaiamobile.org/js/migrators/browser_migrator.js:49 in onUpgradeNeeded: Browser db upgrade needed, migrating so doesn't matter.

09-04 09:57:54.836: E/GeckoConsole(2512): [JavaScript Error: "NotFoundError: The operation failed because the requested database object could not be found. For example, an object store did not exist but was being opened." {file: "app://system.gaiamobile.org/js/migrators/browser_migrator.js" line: 86}]

---

This means something went wrong with the database copy step in the profile. Will investigate.
Assignee: nobody → kyle
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.1?
[Blocking Requested - why for this release]: Bookmark migration not working.
Blocks: 945827
Already nominated 2.1 by Kyle.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
blocking-b2g: 2.1? → 2.2?
Whiteboard: [2.1-exploratory] → [2.1-exploratory][systemsfe]
blocking-b2g: 2.2? → 2.1+
Ok. Looks like we're resolving the wrong path to the IDB file on the device. It's in /data/local/, but we look in the profile directory. 

This passed my local tests because the scripts I wrote just copied a DB into the profile directory without checking that's where it was supposed to be. Oops. 

Just need to figure out how to resolve that correctly based on the platform.
Tested this on phone by creating 2.0 profile, then hardcoding upgrade code paths to always fire. Bookmark appeared on homescreen.
Attachment #8484688 - Flags: review?(fabrice)
Comment on attachment 8484688 [details] [diff] [review]
Patch 1 (v1) - Fix directory service query for IDB storage directory on phone

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

I would rather do the js equivalent of http://mxr.mozilla.org/mozilla-central/source/dom/quota/QuotaManager.cpp#969
That will be more robust when we move indexedDB in the profile directory on device.
Attachment #8484688 - Flags: review?(fabrice) → review-
Removed platform ifdefs, now check profile directory if indexedDBPDir doesn't exist.
Attachment #8484688 - Attachment is obsolete: true
Attachment #8486089 - Flags: review?(fabrice)
Attachment #8486089 - Attachment description: Patch 1 (v1) - Make directory service query for bookmark migration more resilient → Patch 1 (v2) - Make directory service query for bookmark migration more resilient
Target Milestone: --- → 2.1 S4 (12sep)
Comment on attachment 8486089 [details] [diff] [review]
Patch 1 (v2) - Make directory service query for bookmark migration more resilient

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

Maybe that would be easier to read by refactoring the "check id this file exists either in ProfD or indexedDBPDir" ?

::: b2g/components/B2GAppMigrator.js
@@ +48,5 @@
> +
> +    // On the phone, the browser db will only be in the old IDB
> +    // directory, since it only existed up until v2.0. On desktop, it
> +    // will exist in the profile directory.
> +    let browserDBFile = FileUtils.getFile(kIDBDirType,

getFile will create the subdirectories if they are not there, which is not desirable. Can you just use a nsIFile ?

@@ +62,5 @@
> +                                        ["storage",
> +                                         "persistent",
> +                                         browserAppStorageDirName,
> +                                         "idb",
> +                                         browserDBFileName], true);

and here too.

@@ +94,5 @@
> +        return;
> +      }
> +    }
> +
> +    

nit: whitespace!
Attachment #8486089 - Flags: review?(fabrice)
Keywords: dataloss
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
Make sure bookmark migration doesn't create directories along the way.
Attachment #8486089 - Attachment is obsolete: true
Attachment #8488301 - Flags: review?(fabrice)
Blocking on gaia fix to migration.
Depends on: 1066354
Comment on attachment 8488301 [details] [diff] [review]
Patch 1 (v3) - Make directory service query for bookmark migration more resilient

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

::: b2g/components/B2GAppMigrator.js
@@ +98,5 @@
> +        return;
> +      }
> +    }
> +
> +    

nit: whitespace.
Attachment #8488301 - Flags: review?(fabrice) → review+
https://hg.mozilla.org/mozilla-central/rev/56d1ead5277f
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8488301 [details] [diff] [review]
Patch 1 (v3) - Make directory service query for bookmark migration more resilient

Approval Request Comment
[Feature/regressing bug #]: Bug 938171
[User impact if declined]: Bookmarks won't migrate, causes data loss
[Describe test coverage new/current, TBPL]: Hand tested by QA
[Risks and why]: None
[String/UUID change made/needed]: None
Attachment #8488301 - Flags: approval-mozilla-aurora?
Attachment #8488301 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8488301 - Flags: qa-approval?(bzumwalt)
brogan, can you verify this is fixed on 2.1, and mark it off when done?  thanks!
Keywords: verifyme
Comment on attachment 8488301 [details] [diff] [review]
Patch 1 (v3) - Make directory service query for bookmark migration more resilient

Mentioned offline - this needs post build verification, so verifyme is needed here.
Attachment #8488301 - Flags: qa-approval?(bzumwalt)
Peter is working on getting a QA contact to look into this, so I'm adding needinfo to him to set the QA contact to declare who's testing this.
Flags: needinfo?(pbylenga)
Josh Schmitt will work on verifying this, setting flags.
Flags: needinfo?(pbylenga) → needinfo?(jschmitt)
QA Contact: jschmitt
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?][lead-review+]
Flags: needinfo?(ktucker)
Whiteboard: [2.1-exploratory][systemsfe] → [2.1-exploratory][systemsfe][2.1-flame-test-run-2]
Flags: needinfo?(jschmitt)
Issue still reproduces, this bug has been re-written as bug 1069064
QA Whiteboard: [QAnalyst-Triage?][lead-review+] → [QAnalyst-Triage+][lead-review+]
Flags: needinfo?(ktucker)
This issue still reproduces on Flame 2.2. Existing bug 1077715

Actual Results: Existing browser bookmarks are not migrated to homescreen icons after 2.0 to 2.2 OTA. 


This issue is verified fixed on Flame 2.1

Actual Results: Browser bookmarks become homescreen icons after 2.0 to 2.1 OTA.

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141022001201
Gaia: 3d9cc667f4e929861a9a77c41096bbf5a9c1bde0
Gecko: 928b18f7d8ff
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?][lead-review+][failed-verification]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][lead-review+][failed-verification] → [QAnalyst-Triage+][lead-review+][failed-verification]
Depends on: 1077715
Flags: needinfo?(ktucker)
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.