Importing from Muon stops when an error occurs

[bsclifton]

Description

There are a few known scenarios where the import will abort and no further actions happen. For reference, here is the order the import executes in:

1. Referral program data
2. History
3. Bookmarks
4. Passwords
5. Cookies
6. Stats
7. Windows & Tabs
8. Brave Payments

Known failure scenarios

Scenario M - Upgrade from muon but then deny keychain access in b-c

(reported by @LaurenWags)

1. launch brave-beta. Have profile with History, Bookmarks, Passwords, Cookies, Stats, Payments. Set ‘Brave Starts With’ to be something like homepage. (copy this profile to release channel, that’s where data is imported from)
2. check for updates
3. Verify update found
4. Once update found, accept and restart. Do NOT allow keychain access when requested.
   —> b-c launches, the following are imported: history, bookmarks
   the following are NOT imported: Passwords, Cookies, Stats, Rewards
   additionally, due to ‘Brave Starts With’ setting, open windows/tabs were not imported which was expected.

Bookmark import fails scenario

(reported by several folks on Reddit; need help finding an example session with bad bookmarks)

1. Have a specific setup / bookmarks on Muon
2. Exit Muon and Launch Brave Core with a fresh profile
3. Use chrome://settings/importData to import your Muon profile
4. If you have "corrupt" bookmarks, the import will fail
5. Steps after the bookmark import (PW, Cookies, Stats, Windows/Tabs, Brave Payments) are not executed

[btlechowski]

Update from b-l Beta when b-l Release is running
@bsclifton mentioned in slack that b-l can still be running when importing in b-c starts
My idea was that user will relaunch b-l before import in b-c. So that when actual import will happen b-l Release will be running.
STR:

1. In b-l release (0.25.2)

• set Muon profile to open your homepage Brave Starts with
• add Cookies, Saved Passwords, Tabs and Windows
• add Payments, Bookmarks, Stats, History
• keep browser running

2. Install b-l beta (0.23.204) and update it.
3. Verify b-c release

• [PASS] --upgrade-from-muon is present in chrome://version/
• [FAIL]Cookies and Saved Passwords were imported
• [FAIL]Tabs and Windows were imported
• [FAIL]Payments, Bookmarks, Stats, History were imported

If any of the users reported that b-c launched, but nothing was imported, this could be the reason.

[bsclifton]

@btlechowski 's scenario above MAY be the reason a lot of folks didn't have the import run automatically

There is a potential race-condition because the importer requires Muon Brave to be closed:

1. Muon Brave launches
2. Brave Core is installed + launched (import runs as soon as it loads)
3. Muon Brave quits

If step 2 ran the importer too quickly, it could have hit a lock. If that happens, I'm not sure what the user experience looks like. They may have been shown the "Please close Brave and try again" or maybe it fails silently

[garrettr]

@btlechowski @bsclifton b-l must be closed for the b-c importer to run; some of the state files (e.g. sqlite databases) require exclusive access, and I generally think it would be risky to parse any of the state files in b-c while b-l is running: what if the state files are updated on disk partway through the import?

When a user manually initiates import in b-c from b-l, the b-l user data directory is examined for lock files that indicate whether b-l is currently running; if so, the user is prompted to quit b-l, and import is blocked from continuing until b-c can verify that b-l has indeed been quit. See brave/brave-core#245 for more info.

When an automatic import is initiated in b-c from b-l, i.e. via --upgrade-from-muon, ImportFromSourceProfile uses a "headless" ExternalProcessImporterHost, so it is prevented from displaying any of its own dialogs, including the blocking prompt that asks the user to quit b-l if it is running. Since we cannot prompt the user, we have to abort the import if we detect b-l is running—see BraveExternalProcessImporterHost::CheckForChromeOrBraveLock.

Note that keychain access prompts are still shown on some platforms, which I expect might be confusing; that is because they are triggered by a distant portion of the codebase that does not know how to check the headlessness of the ExternalProcessImporterHost.

It was my understanding that the b-l updater would take care of shutting down b-l before it calls b-c with --upgrade-from-muon. If that condition is not or cannot be consistently satisfied, I am not sure what we can do. It seems like the easiest and cleanest solution is for the b-l updater to make sure b-l is shut down before it calls b-c with --upgrade-from-muon; perhaps it could also check for the lock files in the user data directory, in the same fashion as BraveProfileLock?

[garrettr]

This issue was split into specific sub-issues, which, to the best of my knowledge (due to the limited information we had from some of the reports) now comprehensively cover everything described in this issue in a more granular and detailed manner.

Therefore, I think it would make sense to:

1. Close this issue
2. Transfer the QA and release-notes labels from this issue to the specific sub-issues.

Does that sound appropriate? cc @bsclifton @bbondy @rebron @kjozwiak @sri

[bsclifton]

Closing per comment above by @garrettr - the specific issues should already have test plans / labels