Brave-browser: Allow auto-upgrade from browser-laptop via argument

[garrettr]

Test plan (run on Win32/Win64/macOS/Linux)

Test: first run

1. Have an install of browser-laptop setup and configured (Release channel) with
   
   
   
   • bookmarks
   
   
   • history
   
   
   • cookies
   
   
   • saved passwords
   
   
2. Download/install the brave-core release candidate
3. Delete the brave-core profile (if one was created; sometimes installer will launch Brave after install)
4. Launch brave-core via CLI and make sure to pass --upgrade-from-muon
5. Verify items called out in Step 1 were imported into brave-core
   

   Test: only runs once

After completing steps 1-5 of the first run test:

1. Quit brave-core if it is open
2. Launch brave-core via CLI and make sure to pass --upgrade-from-muon
3. Verify that the auto-import was not repeated. For example, there should not be a second set of identical imported bookmarks.

Test notes

Brave Payments is not covered with this issue. That work is taking place in https://github.com/brave/brave-core/pull/736 (which fixes issue https://github.com/brave/brave-browser/issues/1215)

Description

brave-browser should recognize an optional argument, e.g. --upgrade-from-muon or --upgrade-from-browser-laptop, such that if the argument is provided, it should:

1. Check the system for an existing installation of browser-laptop.
2. If an existing installation is found, automatically import all supported data types from it silently and without user interaction.
3. Allow startup to continue.

We may also want to:

1. Automatically shut down or restart the browser once the import is complete. This could be the default behavior, or optionally triggered by an additional argument (e.g. --quit-after-import).
2. We should consider additionally checking to see if browser-laptop is the user's current default browser, and only auto-import if it is.
   
   
   
   • For example, if I used browser-laptop for a few weeks last year, then switched to a different browser up until the present moment when I've decided to give brave-browser a try, I might be confused/unenthused if a bunch of out-of-date browsing data is automatically included with my first-run experience.
   
   

[bsclifton]

@garrettr I know we've talked about it before: does import handle the referral promo? (importing the downloadId, etc)

[garrettr]

@bsclifton Not yet, but isn't that being tracked in #1294?

[bsclifton]

@garrettr yes- that is correct 😄 Totally forgot about that (nothing to see here!) 😅

[garrettr]

While exploring options for implementing this feature, I discovered that Chromium already has a first-run auto-import feature. Here's how it works:

1. The feature is configured on a per-profile basis via prefs, which we can set in either the default preferences or in master_preferences.
2. The list of specific data types to be imported can be configured via prefs.
3. The selected data types are automatically imported from the user's current default browser, which is assumed to be the first browser in the list returned by DetectSourceProfiles.

To reuse this functionality to achieve auto-import from browser-laptop upon upgrade, we would need to:

• [ ] Modify the default preferences to enable the first-run auto-import feature (easy)
• [ ] Teach first_run::AutoImport about the new importer data types supported by BraveImporter (easy)
• [ ] Detect whether Brave is default browser and order importer list accordingly (moderate, already tracked in #104).

With these items implemented, Brave Browser would auto-import all supported data types from the user's current default browser, regardless of whether it is browser-laptop/Muon or something else. We should then consider whether we want to:

1. Only auto-import if the user's current default browser is browser-laptop/Muon?
   
   
   
   • I am in favor of this and think it aligns well with current desired functionality
   
   
2. Always auto-import from browser-laptop/Muon, even if it's not the user's current default browser?
   
   
   
   • I am personally against this, as I mentioned in https://github.com/brave/brave-browser/issues/1545#issue-369341848
   
   

It'd be nice to leverage this existing functionality, as long as it satisfies our design requirements. @bsclifton What do you think? See any potential conflicts with the functionality you require?

[bsclifton]

@garrettr wow- this is pretty cool and it would be great to re-use this 😄

I guess it depends on how hard implementing #104 would be. If you think we can land this (along with the other import logic) within the next week, I'd say lets go for it. If that might be at risk, I'd say let's go for a flag-only approach for now (and then follow up with this later, if it makes sense)

With regards to the auto-import from browser-laptop/Muon... could we implement both? For example, we can (by default) implement your option 1 (only import if it's the default browser). We could then override this by providing the flag specified here (--import-from-muon for example) if we want to do option 2

So far, all stakeholders have agreed on always importing on first run. We can definitely revisit that though- cc: @rebron @davidtemkin @bbondy @kjozwiak

[davidtemkin]

When we auto-import from Muon following an upgrade, that is a version upgrade from a user POV. It's like going from Chrome 69 to Chrome 70, except that there are, ahem, substantial changes. But the process we are modeling is that of upgrading a browser to the latest version, not that of installing a new browser and importing from a pre-existing browser. So the expectation is that a user's data will be carried over.

A manual download, I think, is different. I don't think we'd want to auto-import from any browser (Muon or other) when a user has manually downloaded/installed brave core. In that case, they need to decide what to do, as the process we are modeling in that case is one of installing a new browser. In that situation, it isn't a given that they'd want to import from Muon, or from their default browser, or from any browser.

[garrettr]

Good point, @davidtemkin—it's important to distinguish the "auto upgrade from muon to brave-core" and "manually installed brave-core" cases, and a command line set on the first run of the new browser by the upgrader is definitely the simplest way to accomplish that.

We could tie reusing the existing first_run::AutoImport functionality together with a command line arg by adding a pref that can be set on the command line, e.g. via CommandLinePrefStore. The only potential downside is it would limit Brave auto-import to running only when both conditions—brave-core is on first run, and pref is set (by command line arg, or another mechanism)—are true. This seems to exactly fit our use case, but I can imagine it might make manual testing somewhat cumbersome because you would have to clear brave-core's state every time so it would think it was on its first run (but devs should probably do that anyway?)

[bsclifton]

@garrettr I think that's pretty reasonable - and you're right, that's the only use-case where we want this logic to fire 😄 So this approach would be perfect

To address the manual import process, #104 captures the work we'd need to do (which we could do at a later time)

[kjozwiak]

his seems to exactly fit our use case, but I can imagine it might make manual testing somewhat cumbersome because you would have to clear brave-core's state every time so it would think it was on its first run (but devs should probably do that anyway?)

@garrettr as long as it's manually testable, no worries about needing to nuke profiles/states. QA nukes states/profiles on a daily basis so no worries there 👍I personally do it at least 10-15 times sometimes.

[garrettr]

Worth noting that some Chromium devs appear to have been considering the removal of first_run::AutoImport semi-recently; unfortunately, the most current relevant issue, 555550, is not publicly visible at the time of this writing.

[LaurenWags]

Using test plan from description and

Brave | 0.56.8 Chromium: 70.0.3538.77 (Official Build) (64-bit)
-- | --
Revision | 0f6ce0b0cd63a12cb4eccea3637b1bc9a29148d9-refs/branch-heads/3538@{#1039}
OS | Mac OS X

I found that:

• bookmarks, history, passwords are brought over. I am prompted once during this auto import to allow access to my keychain.
• cookies are not imported automatically.
• however, running manual import of cookies, (Brave > Import Bookmarks and Settings > choose Brave, choose only Cookies) cookies are then imported. If I do this, I am prompted to allow access to my keychain.

Verification passed on

Brave | 0.56.8 Chromium: 70.0.3538.77 (Official Build) (64-bit)
-- | --
Revision | 0f6ce0b0cd63a12cb4eccea3637b1bc9a29148d9-refs/branch-heads/3538@{#1039}
OS | Windows

• Verified bookmarks/passwords/history imported with arguments
• Verified relaunch with command line params not duplicate entries which are already imported
• Verified manually importing cookie worked but not via the command line launch

Verification passed on

Brave | 0.56.8 Chromium: 70.0.3538.77 (Official Build) (64-bit)
-- | --
Revision | 0f6ce0b0cd63a12cb4eccea3637b1bc9a29148d9-refs/branch-heads/3538@{#1039}
OS | Windows 7 x64

• imported bookmarks, history and passwords
• failed to import cookies

Verification Passed on

Brave | 0.56.8 Chromium: 70.0.3538.77 (Official Build) (64-bit)
-- | --
Revision | 0f6ce0b0cd63a12cb4eccea3637b1bc9a29148d9-refs/branch-heads/3538@{#1039}
OS | Windows 10 x64

• Verified BM's, history and saved passwords imported correctly in b-c
• Failed to import cookies
• Verified relaunch with command line params not duplicate entries which are already imported
• Verified manually importing cookie worked but not via the command line launch

[LaurenWags]

Logged follow up issue #2013 for cookies not being imported

[srirambv]

@garrettr @bsclifton should launching b-c with the command line paramenter when b-l is open should it import ? It does ask the browser to be closed when you try to manually import from Brave

[bsclifton]

@srirambv you'll get a chance to try that with the browser-laptop release, but as soon as brave-core is launched, browser-laptop quits

[garrettr]

Good question, @srirambv, I will test to verify the behavior. The b-l importer cannot run correctly if b-l is open. When a b-c user initiates b-l import, the status of b-l is detected and the user is prompted to close b-l before continuing the import. It is likely that this check is getting ignored or bypassed when the importer is run automatically.