Closed Bug 706005 Opened 13 years ago Closed 9 years ago

Chrome migrator should handle when databases are locked

Categories

(Firefox :: Migration, defect)

defect
Not set
major
Points:
3

Tracking

()

VERIFIED FIXED
Firefox 42
Iteration:
42.3 - Aug 10
Tracking Status
firefox42 --- verified

People

(Reporter: MattN, Assigned: rittme)

References

Details

(Keywords: uiwanted, Whiteboard: [strings])

Attachments

(1 file)

Currently, if Chrome is open and I import history from the running profile then it will fail without telling me or importing any history. The failure is because the sqlite history database is locked. Below is the error reported in the error console: Error: [Exception... "Component returned failure code: 0x80630001 (NS_ERROR_STORAGE_BUSY) [nsINavHistoryService.runInBatchMode]" nsresult: "0x80630001 (NS_ERROR_STORAGE_BUSY)" location: "JS frame :: file:///Users/foo/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/components/ChromeProfileMigrator.js :: Chrome_migrateHistory :: line 306" data: no] Source File: file:///Users/foo/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/components/ChromeProfileMigrator.js Line: 308 We should probably tell users to close the source browser if we detect an item we want to import from is locked.
Blocks: 505192
No longer depends on: 505192
Migration runs automatically during startup, so our options here are limited. What would happen: 1. Use uses chrome as its default browser and decides to download Fx. 2. Installer runs. Firefox is opened. 3. We ask him/her to close Chrome before he can use Firefox. 4. If that went well, we'll then ask him to also make Fx the default browser. Somewhat violent, I would say. Isn't there any way we can deal with the locked database?
However, it's also bad that the current UI tells you History and Cookies were migrated even though it's notified for errors.
Two more notes: 1. When chrome is open, we also fail to read the "Local State" fail, in which chrome's profile are listed. However, this exception is swallowed, and we fallback to importing the "Default" profile, from which we fail to import history and cookies (sqlite dbs), but succeed in importing bookmarks (json file). If this exceptions wasn't swallowed the way it is, we would't try to import from Chrome at all. 2. So, given comment 1, one solution would be to avoid auto-migration from Chrome in this case, but offer migration from crhome (plus instruction to close it) if migration is manually invoked later. I filed bug 731144 to make this option a little bit more realistic.
Keywords: uiwanted
Severity: normal → major
Alex: this one is pretty important. To be clear, if Chrome runs along with the migration wizard (and that's very likely for someone who's going to migrate from Chrome), migration is guaranteed to fail.
Flags: firefox-backlog+
Points: --- → 13
Flags: qe-verify?
For now we're just going to proactively tell the user to close the source browser since it's better than nothing. The current dialog looks like this (on Nightly not during the first run): http://grab.by/Jovm A "Don't import anything" option appears as the last radio button during first run. I think we can add a sentence (in bold) below the radio buttons telling the user to close the source browser before continuing. To be smart, we could have the text show up depending on the selected radio button so it won't show up for IE (Safari too?) or "Don't import anything" options where it isn't relevant. Matej, what do you think of this: "For best results, please ensure the selected browser is closed before continuing." Thanks.
Assignee: nobody → bernardo
Status: NEW → ASSIGNED
Iteration: --- → 42.3 - Aug 10
Points: 13 → 3
Flags: qe-verify?
Flags: qe-verify+
Flags: needinfo?(matej)
Whiteboard: [strings]
Looks OK to me, just wondering if we need "For best results" at the beginning. It reads cleaner without it.
Flags: needinfo?(matej)
Yeah, I'd nix that part. It's not that you get better results, it's that it's _broken_ if you don't.
Bug 706005 - Chrome migrator notifies user to close browser. r=MattN
Attachment #8645253 - Flags: review?(MattN+bmo)
Comment on attachment 8645253 [details] MozReview Request: Bug 706005 - Chrome migrator notifies user to close browser. r=MattN Bug 706005 - Chrome migrator notifies user to close browser. r=MattN
Comment on attachment 8645253 [details] MozReview Request: Bug 706005 - Chrome migrator notifies user to close browser. r=MattN https://reviewboard.mozilla.org/r/15481/#review13843 ::: browser/components/migration/content/migration.js:88 (Diff revision 2) > > + group.addEventListener("click", function() { I don't think "click" covers keyboard navigation. I think you may want "command" or if it's like HTML, maybe "change"? Please test this manually ::: browser/components/migration/content/migration.js:89 (Diff revision 2) > + group.addEventListener("click", function() { I think you also need to call this function when the page initially displays since Chrome can be pre-selected depending on the OS and other available browsers (you can mock this for testing). You'll want to name the function and call it from the event listener and on load or whatever wizards use.
Attachment #8645253 - Flags: review?(MattN+bmo) → review+
Attachment #8645253 - Flags: review+ → review?(MattN+bmo)
Comment on attachment 8645253 [details] MozReview Request: Bug 706005 - Chrome migrator notifies user to close browser. r=MattN Bug 706005 - Chrome migrator notifies user to close browser. r=MattN
Comment on attachment 8645253 [details] MozReview Request: Bug 706005 - Chrome migrator notifies user to close browser. r=MattN https://reviewboard.mozilla.org/r/15481/#review13865 I made some tweaks myself since we want this landed before string freeze: ::: browser/components/migration/content/migration.js:69 (Diff revision 3) > + if (group.selectedItem.id == "chrome" || > + group.selectedItem.id == "canary" || > + group.selectedItem.id == "chromium") { The front-end shouldn't have to know this much about the migrators so I exposed this as a property on each migrator instead. Take a look at what I landed. ::: browser/components/migration/content/migration.xul:54 (Diff revision 3) > <label id="noSources" hidden="true">&noMigrationSources.label;</label> > + <description class="header" id="closeSourceBrowser" hidden="true">&closeSourceBrowser.label;</description> I added a flexiable spacer above this since it seemed too close to the radio group and attracted less attention since it was further from the next button. ::: browser/components/migration/content/migration.js:72 (Diff revision 3) > + document.getElementById("closeSourceBrowser").hidden = false; > + } else { > + document.getElementById("closeSourceBrowser").hidden = true; Hidden is supposed to be the same as visibility:hidden according to MDN but that didn't seem to be the case so I switched to visibility:hidden; so the layout wouldn't change in some case upon toggling options.
Attachment #8645253 - Flags: review?(MattN+bmo) → review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
QA Contact: florin.mezei
I was able to reproduce this issue on Firefox 41 RC (20150917150946) using Windows 7 64-bit. Verified fixed on Firefox 42 Beta 1 (20150921151815) under Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac Os X 10.10.4. “Please ensure the selected browser is closed before continuing.” message is successfully displayed when Chrome radio button is selected.
Status: RESOLVED → VERIFIED
Blocks: 1633865
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: