Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "--upgrade-from-muon" flag to auto-import from Muon #729

Merged
merged 1 commit into from
Oct 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 1 addition & 14 deletions browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,6 @@ source_set("browser_process") {
"geolocation/brave_geolocation_permission_context.h",
"guest_window_search_engine_provider_controller.cc",
"guest_window_search_engine_provider_controller.h",
"importer/brave_external_process_importer_client.cc",
"importer/brave_external_process_importer_client.h",
"importer/brave_external_process_importer_host.cc",
"importer/brave_external_process_importer_host.h",
"importer/brave_importer_lock_dialog.h",
"importer/brave_in_process_importer_bridge.cc",
"importer/brave_in_process_importer_bridge.h",
"importer/brave_profile_writer.cc",
"importer/brave_profile_writer.h",
"importer/brave_profile_lock.cc",
"importer/brave_profile_lock.h",
"importer/browser_profile_lock.h",
"importer/chrome_profile_lock.cc",
"importer/chrome_profile_lock.h",
"mac/sparkle_glue.mm",
"mac/sparkle_glue.h",
"mac/su_updater.h",
Expand Down Expand Up @@ -123,6 +109,7 @@ source_set("browser") {
"//chrome/browser",
"download",
"extensions",
"importer",
"net",
"permissions",
"profiles",
Expand Down
5 changes: 5 additions & 0 deletions browser/brave_browser_main_extra_parts.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "brave/browser/brave_browser_main_extra_parts.h"

#include "brave/common/resource_bundle_helper.h"
#include "chrome/browser/first_run/first_run.h"

BraveBrowserMainExtraParts::BraveBrowserMainExtraParts() {
}
Expand All @@ -15,3 +16,7 @@ BraveBrowserMainExtraParts::~BraveBrowserMainExtraParts() {
void BraveBrowserMainExtraParts::PreCreateThreads() {
brave::InitializeResourceBundle();
}

void BraveBrowserMainExtraParts::PreMainMessageLoopRun() {
Copy link
Collaborator

@bridiver bridiver Oct 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fine because the profile is initialized in PreMainMessageLoopRun and this will run after the chrome extra parts, but I think we want to guard this with local_state pref that is set after import instead of using a command line flag

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think guarding with a local_state pref would be preferable to conditioning auto-import on first run, e.g. via first_run::IsChromeFirstRun?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of using a command line flag

We need a command line flag to distinguish "Brave-Core is running for the first time after being installed by the Muon upgrader" and "Brave-Core is running for the first time after being downloaded and installed de novo". See brave/brave-browser#1545 (comment) for discussion.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we just upgrade by default if we detect a previous browser-laptop install? Then no command line flag is necessary and a pref would work fine

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we just upgrade by default if we detect a previous browser-laptop install?

We discussed this in a meeting last week and consensus was that auto-import should only happen in the case where the user is running browser-laptop and performs the update that migrates them to brave-core. We need a way to distinguish this from the case where the user downloads and installs brave-core separately, but may also have an existing browser-laptop installation (which could be their current/default browser, or defunct and unused and therefore probably not desirable to auto-import from). We agreed that passing a command line argument to the brave-core binary from the auto-update was the best way to distinguish these cases.

cc @bsclifton @davidtemkin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bridiver What do you think of the local state pref I added in 51c68d4? With this addition,

  1. auto-import from Muon only occurs if triggered by passing --upgrade-from-muon command line flag
  2. auto-import from Muon status is tracked in a local_state pref, so auto-import can occur at most once
  3. the local_state pref may be useful to other components (e.g. if Brave Rewards wants to convert pins -> donations, it can check the pref to see if there are any imported pins that need conversion)

brave::AutoImportMuon();
}
1 change: 1 addition & 0 deletions browser/brave_browser_main_extra_parts.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class BraveBrowserMainExtraParts : public ChromeBrowserMainExtraParts {

// ChromeBrowserMainExtraParts overrides.
void PreCreateThreads() override;
void PreMainMessageLoopRun() override;

private:
DISALLOW_COPY_AND_ASSIGN(BraveBrowserMainExtraParts);
Expand Down
2 changes: 2 additions & 0 deletions browser/brave_local_state_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "brave/browser/brave_stats_updater.h"
#include "brave/browser/referrals/brave_referrals_service.h"
#include "brave/browser/tor/tor_profile_service.h"
#include "chrome/browser/first_run/first_run.h"
#include "chrome/common/pref_names.h"
#include "components/prefs/pref_registry_simple.h"

Expand All @@ -22,6 +23,7 @@ void RegisterLocalStatePrefs(PrefRegistrySimple* registry) {
base::Value(false));
#endif
tor::TorProfileService::RegisterLocalStatePrefs(registry);
RegisterPrefsForMuonMigration(registry);
}

} // namespace brave
24 changes: 24 additions & 0 deletions browser/importer/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import("//build/config/features.gni")

source_set("importer") {
sources = [
"brave_external_process_importer_client.cc",
"brave_external_process_importer_client.h",
"brave_external_process_importer_host.cc",
"brave_external_process_importer_host.h",
"brave_importer_lock_dialog.h",
"brave_in_process_importer_bridge.cc",
"brave_in_process_importer_bridge.h",
"brave_profile_writer.cc",
"brave_profile_writer.h",
"brave_profile_lock.cc",
"brave_profile_lock.h",
"browser_profile_lock.h",
"chrome_profile_lock.cc",
"chrome_profile_lock.h",
]

deps = [
"//chrome/browser",
]
}
82 changes: 82 additions & 0 deletions chromium_src/chrome/browser/first_run/first_run.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/browser/brave_browser_process_impl.h"
#include "brave/common/brave_switches.h"
#include "brave/common/pref_names.h"

#include "base/command_line.h"
#include "base/logging.h"
#include "base/run_loop.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/importer/external_process_importer_host.h"
#include "chrome/browser/importer/importer_list.h"
#include "chrome/browser/importer/importer_progress_observer.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/common/importer/importer_data_types.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"

#include "../../../../../chrome/browser/first_run/first_run.cc"

namespace brave {

void AutoImportMuon() {
base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess();
if (!command_line.HasSwitch(switches::kUpgradeFromMuon))
return;

PrefService* local_state = g_browser_process->local_state();
if (local_state->GetBoolean(kMigratedMuonProfile)) {
LOG(WARNING) << "Muon profile already migrated, ignoring --upgrade-from-muon";
return;
}

LOG(INFO) << "Auto-importing Muon profile";

base::RunLoop run_loop;
auto importer_list = std::make_unique<ImporterList>();
importer_list->DetectSourceProfiles(
g_browser_process->GetApplicationLocale(),
false, // include_interactive_profiles
run_loop.QuitClosure());
run_loop.Run();

bool brave_profile_found = false;
size_t brave_profile_index = 0;
for (size_t i = 0; i < importer_list->count(); i++) {
const auto& source_profile = importer_list->GetSourceProfileAt(i);
if (source_profile.importer_type == importer::TYPE_BRAVE) {
brave_profile_found = true;
brave_profile_index = i;
break;
}
}
if (!brave_profile_found) {
LOG(INFO) << "Muon profile not found";
return;
}

const importer::SourceProfile& source_profile =
importer_list->GetSourceProfileAt(brave_profile_index);

// Import every possible type of data from the Muon profile.
uint16_t items_to_import = 0;
items_to_import |= source_profile.services_supported;

ProfileManager* profile_manager = g_browser_process->profile_manager();
Profile* target_profile = profile_manager->GetLastUsedProfile();

ImportFromSourceProfile(source_profile, target_profile, items_to_import);

// Mark Muon profile as migrated so we don't attempt to import it again.
local_state->SetBoolean(kMigratedMuonProfile, true);
}

void RegisterPrefsForMuonMigration(PrefRegistrySimple* registry) {
registry->RegisterBooleanPref(kMigratedMuonProfile, false);
}

} // namespace brave
21 changes: 21 additions & 0 deletions chromium_src/chrome/browser/first_run/first_run.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_FIRST_RUN_FIRST_RUN_H_
#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_FIRST_RUN_FIRST_RUN_H_

#include "../../../../../chrome/browser/first_run/first_run.h"

class PrefRegistrySimple;

namespace brave {

void AutoImportMuon();

// Registers the preferences used to track the state of migration from Muon
void RegisterPrefsForMuonMigration(PrefRegistrySimple* registry);

} // namespace brave

#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_FIRST_RUN_FIRST_RUN_H_
4 changes: 4 additions & 0 deletions common/brave_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,8 @@ const char kRewardsReconcileInterval[] = "rewards-reconcile-interval";
// Valid values are: "dark" | "light".
const char kUiMode[] = "ui-mode";

// Triggers auto-import of profile data from Brave browser-laptop/Muon, if
// available.
const char kUpgradeFromMuon[] = "upgrade-from-muon";

} // namespace switches
2 changes: 2 additions & 0 deletions common/brave_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ extern const char kRewardsReconcileInterval[];

extern const char kUiMode[];

extern const char kUpgradeFromMuon[];

} // namespace switches

#endif // BRAVE_COMMON_BRAVE_SWITCHES_H_
Expand Down
1 change: 1 addition & 0 deletions common/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@ const char kReferralCheckedForPromoCodeFile[] = "brave.referral.checked_for_prom
const char kHTTPSEVerywhereControlType[] = "brave.https_everywhere_default";
const char kNoScriptControlType[] = "brave.no_script_default";
const char kRewardsNotifications[] = "brave.rewards.notifications";
const char kMigratedMuonProfile[] = "brave.muon.migrated_profile";
1 change: 1 addition & 0 deletions common/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,6 @@ extern const char kReferralCheckedForPromoCodeFile[];
extern const char kHTTPSEVerywhereControlType[];
extern const char kNoScriptControlType[];
extern const char kRewardsNotifications[];
extern const char kMigratedMuonProfile[];

#endif // BRAVE_COMMON_PREF_NAMES_H_