From fd84d4ce8c5cceba8565709cc09a71a58fd31cd4 Mon Sep 17 00:00:00 2001 From: sergei Date: Mon, 9 Jan 2023 17:04:46 +0300 Subject: [PATCH 1/5] Import source profiles to separate profiles --- browser/ui/BUILD.gn | 2 + browser/ui/webui/brave_welcome_ui.cc | 4 +- .../brave_import_bulk_data_handler.cc | 138 ++++++++++++++++++ .../settings/brave_import_bulk_data_handler.h | 60 ++++++++ .../settings/brave_import_data_handler.cc | 31 ++-- .../settings/brave_import_data_handler.h | 19 ++- .../api/import_data_browser_proxy.ts | 14 ++ .../components/select-profile/index.tsx | 9 +- 8 files changed, 258 insertions(+), 19 deletions(-) create mode 100644 browser/ui/webui/settings/brave_import_bulk_data_handler.cc create mode 100644 browser/ui/webui/settings/brave_import_bulk_data_handler.h diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn index 5d3fe5cdbcbb..fcee5b662c6f 100644 --- a/browser/ui/BUILD.gn +++ b/browser/ui/BUILD.gn @@ -148,6 +148,8 @@ source_set("ui") { "webui/settings/brave_adblock_handler.h", "webui/settings/brave_appearance_handler.cc", "webui/settings/brave_appearance_handler.h", + "webui/settings/brave_import_bulk_data_handler.cc", + "webui/settings/brave_import_bulk_data_handler.h", "webui/settings/brave_import_data_handler.cc", "webui/settings/brave_import_data_handler.h", "webui/settings/brave_importer_observer.cc", diff --git a/browser/ui/webui/brave_welcome_ui.cc b/browser/ui/webui/brave_welcome_ui.cc index 1a5f70e603aa..9a6584e4c56d 100644 --- a/browser/ui/webui/brave_welcome_ui.cc +++ b/browser/ui/webui/brave_welcome_ui.cc @@ -12,7 +12,7 @@ #include "base/feature_list.h" #include "base/metrics/histogram_macros.h" #include "brave/browser/ui/webui/brave_webui_source.h" -#include "brave/browser/ui/webui/settings/brave_import_data_handler.h" +#include "brave/browser/ui/webui/settings/brave_import_bulk_data_handler.h" #include "brave/browser/ui/webui/settings/brave_search_engines_handler.h" #include "brave/components/brave_welcome/common/features.h" #include "brave/components/brave_welcome/resources/grit/brave_welcome_generated_map.h" @@ -251,7 +251,7 @@ BraveWelcomeUI::BraveWelcomeUI(content::WebUI* web_ui, const std::string& name) web_ui->AddMessageHandler( std::make_unique(Profile::FromWebUI(web_ui))); web_ui->AddMessageHandler( - std::make_unique()); + std::make_unique()); web_ui->AddMessageHandler( std::make_unique()); // set default // browser diff --git a/browser/ui/webui/settings/brave_import_bulk_data_handler.cc b/browser/ui/webui/settings/brave_import_bulk_data_handler.cc new file mode 100644 index 000000000000..2f9be8b1eac1 --- /dev/null +++ b/browser/ui/webui/settings/brave_import_bulk_data_handler.cc @@ -0,0 +1,138 @@ +/* Copyright (c) 2023 The Brave Authors. All rights reserved. + * 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 https://mozilla.org/MPL/2.0/. */ + +#include "brave/browser/ui/webui/settings/brave_import_bulk_data_handler.h" + +#include +#include +#include +#include + +#include "base/bind.h" +#include "base/files/file_path.h" +#include "base/files/file_util.h" +#include "base/json/json_reader.h" +#include "base/rand_util.h" +#include "base/strings/utf_string_conversions.h" +#include "brave/browser/ui/webui/settings/import_feature.h" +#include "chrome/browser/browser_process.h" +#include "chrome/browser/profiles/profile_avatar_icon_util.h" +#include "chrome/browser/profiles/profile_manager.h" +#include "chrome/common/pref_names.h" +#include "components/prefs/pref_service.h" + +namespace settings { + +BraveImportBulkDataHandler::BraveImportBulkDataHandler() = default; +BraveImportBulkDataHandler::~BraveImportBulkDataHandler() = default; + +void BraveImportBulkDataHandler::RegisterMessages() { + BraveImportDataHandler::RegisterMessages(); + web_ui()->RegisterMessageCallback( + "importDataBulk", + base::BindRepeating(&BraveImportBulkDataHandler::HandleImportDataBulk, + base::Unretained(this))); +} + +void BraveImportBulkDataHandler::PrepareProfile( + const std::u16string& name, + ProfileReadyCallback profile_ready_callback) { + ProfileManager* profile_manager = g_browser_process->profile_manager(); + Profile* profile = + profile_manager->GetProfileByPath(profile_manager->user_data_dir().Append( + base::FilePath::FromASCII(base::UTF16ToUTF8(name)))); + + if (profile) { + std::move(profile_ready_callback).Run(profile); + return; + } + + auto avatar_index = base::RandInt(profiles::GetModernAvatarIconStartIndex(), + profiles::GetDefaultAvatarIconCount()); + ProfileManager::CreateMultiProfileAsync( + name, avatar_index, false, + base::BindOnce( + [](ProfileReadyCallback initialized_callback, Profile* profile) { + CHECK(profile); + // Migrate welcome page flag to new profiles. + profile->GetPrefs()->SetBoolean(prefs::kHasSeenWelcomePage, true); + std::move(initialized_callback).Run(profile); + }, + std::move(profile_ready_callback))); +} + +void BraveImportBulkDataHandler::ProfileReadyForImport( + const importer::SourceProfile& source_profile, + uint16_t imported_items, + Profile* profile) { +#if BUILDFLAG(IS_MAC) + CheckDiskAccess(source_profile, imported_items, profile); +#else + StartImportImpl(source_profile, imported_items, profile); +#endif +} + +void BraveImportBulkDataHandler::HandleImportDataBulk( + const base::Value::List& args) { + CHECK_GE(args.size(), 2u); + const auto& list = args[0].GetList(); + // Bulk profiles import assumes new profiles will be created on our side if + // they do not exist. + const base::Value& types = args[1]; + for (const auto& it : list) { + int browser_index = it.GetInt(); + importing_profiles_.insert(browser_index); + base::Value::List single_profile_args; + single_profile_args.Append(base::Value(browser_index)); + single_profile_args.Append(types.Clone()); + BraveImportDataHandler::HandleImportData(single_profile_args); + } +} + +absl::optional BraveImportBulkDataHandler::GetProfileIndex( + const importer::SourceProfile& source_profile) { + for (auto index : importing_profiles_) { + const auto& profile = GetSourceProfileAt(index); + if (profile.source_path == source_profile.source_path) { + return index; + } + } + return absl::nullopt; +} + +void BraveImportBulkDataHandler::StartImport( + const importer::SourceProfile& source_profile, + uint16_t imported_items) { + // If profile is not from the bulk import request fallback to single profile + // import. + if (!GetProfileIndex(source_profile).has_value()) { + BraveImportDataHandler::StartImport(source_profile, imported_items); + return; + } + if (!imported_items) + return; + PrepareProfile( + source_profile.profile.empty() ? source_profile.importer_name + : source_profile.profile, + base::BindOnce(&BraveImportBulkDataHandler::ProfileReadyForImport, + weak_factory_.GetWeakPtr(), source_profile, + imported_items)); +} + +void BraveImportBulkDataHandler::NotifyImportProgress( + const importer::SourceProfile& source_profile, + const base::Value& info) { + BraveImportDataHandler::NotifyImportProgress(source_profile, info); + + const std::string* event = info.FindStringKey("event"); + if (!event || *event != "ImportEnded") + return; + auto index = GetProfileIndex(source_profile); + if (index.has_value()) { + importing_profiles_.erase(index.value()); + } +} + +} // namespace settings diff --git a/browser/ui/webui/settings/brave_import_bulk_data_handler.h b/browser/ui/webui/settings/brave_import_bulk_data_handler.h new file mode 100644 index 000000000000..e7fbbc4a3e24 --- /dev/null +++ b/browser/ui/webui/settings/brave_import_bulk_data_handler.h @@ -0,0 +1,60 @@ +/* Copyright (c) 2023 The Brave Authors. All rights reserved. + * 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 https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_BROWSER_UI_WEBUI_SETTINGS_BRAVE_IMPORT_BULK_DATA_HANDLER_H_ +#define BRAVE_BROWSER_UI_WEBUI_SETTINGS_BRAVE_IMPORT_BULK_DATA_HANDLER_H_ + +#include + +#include "base/callback.h" +#include "base/containers/flat_set.h" +#include "base/memory/weak_ptr.h" +#include "brave/browser/ui/webui/settings/brave_import_data_handler.h" +#include "brave/browser/ui/webui/settings/brave_importer_observer.h" +#include "build/build_config.h" +#include "third_party/abseil-cpp/absl/types/optional.h" + +namespace settings { + +// This class handles bulk requests to import multiple profiles to +// new target Brave profiles. +class BraveImportBulkDataHandler : public BraveImportDataHandler { + public: + BraveImportBulkDataHandler(); + ~BraveImportBulkDataHandler() override; + + using ProfileReadyCallback = base::OnceCallback; + + BraveImportBulkDataHandler(const BraveImportBulkDataHandler&) = delete; + BraveImportBulkDataHandler& operator=(const BraveImportBulkDataHandler&) = + delete; + + void PrepareProfile(const std::u16string& name, + ProfileReadyCallback callback); + + void ProfileReadyForImport(const importer::SourceProfile& source_profile, + uint16_t imported_items, + Profile* profile); + void NotifyImportProgress(const importer::SourceProfile& source_profile, + const base::Value& info) override; + + // SettingsPageUIHandler + void RegisterMessages() override; + + private: + absl::optional GetProfileIndex( + const importer::SourceProfile& source_profile); + + void HandleImportDataBulk(const base::Value::List& args); + // ImportDataHandler overrides: + void StartImport(const importer::SourceProfile& source_profile, + uint16_t imported_items) override; + base::flat_set importing_profiles_; + base::WeakPtrFactory weak_factory_{this}; +}; + +} // namespace settings + +#endif // BRAVE_BROWSER_UI_WEBUI_SETTINGS_BRAVE_IMPORT_BULK_DATA_HANDLER_H_ diff --git a/browser/ui/webui/settings/brave_import_data_handler.cc b/browser/ui/webui/settings/brave_import_data_handler.cc index 834dc1cf6ac3..e4855493c5f9 100644 --- a/browser/ui/webui/settings/brave_import_data_handler.cc +++ b/browser/ui/webui/settings/brave_import_data_handler.cc @@ -11,6 +11,7 @@ #include "brave/browser/importer/brave_external_process_importer_host.h" #include "brave/browser/ui/webui/settings/import_feature.h" +#include "chrome/browser/importer/importer_list.h" #include "chrome/browser/importer/profile_writer.h" #include "chrome/browser/profiles/profile.h" #include "content/public/browser/browser_task_traits.h" @@ -68,17 +69,18 @@ void BraveImportDataHandler::StartImport( uint16_t imported_items) { if (!imported_items) return; - + Profile* profile = Profile::FromWebUI(web_ui()); #if BUILDFLAG(IS_MAC) - CheckDiskAccess(source_profile, imported_items); + CheckDiskAccess(source_profile, imported_items, profile); #else - StartImportImpl(source_profile, imported_items); + StartImportImpl(source_profile, imported_items, profile); #endif } void BraveImportDataHandler::StartImportImpl( const importer::SourceProfile& source_profile, - uint16_t imported_items) { + uint16_t imported_items, + Profile* profile) { // Temporary flag to keep old way until // https://github.com/brave/brave-core/pull/15637 landed. // Should be removed in that PR. @@ -97,7 +99,7 @@ void BraveImportDataHandler::StartImportImpl( importer_host, source_profile, imported_items, base::BindRepeating(&BraveImportDataHandler::NotifyImportProgress, weak_factory_.GetWeakPtr())); - Profile* profile = Profile::FromWebUI(web_ui()); + importer_host->StartImportSettings(source_profile, profile, imported_items, new ProfileWriter(profile)); } @@ -115,14 +117,24 @@ void BraveImportDataHandler::NotifyImportProgress( } } +void BraveImportDataHandler::HandleImportData(const base::Value::List& args) { + ImportDataHandler::HandleImportData(args); +} + void BraveImportDataHandler::OnImportEnded(const base::FilePath& source_path) { import_observers_.erase(source_path); } +const importer::SourceProfile& BraveImportDataHandler::GetSourceProfileAt( + int browser_index) { + return importer_list_->GetSourceProfileAt(browser_index); +} + #if BUILDFLAG(IS_MAC) void BraveImportDataHandler::CheckDiskAccess( const importer::SourceProfile& source_profile, - uint16_t imported_items) { + uint16_t imported_items, + Profile* profile) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); guide_dialog_is_requested_ = false; @@ -138,16 +150,17 @@ void BraveImportDataHandler::CheckDiskAccess( base::BindOnce(&HasProperDiskAccessPermission, imported_items), base::BindOnce(&BraveImportDataHandler::OnGetDiskAccessPermission, weak_factory_.GetWeakPtr(), source_profile, - imported_items)); + imported_items, profile)); return; } - StartImportImpl(source_profile, imported_items); + StartImportImpl(source_profile, imported_items, profile); } void BraveImportDataHandler::OnGetDiskAccessPermission( const importer::SourceProfile& source_profile, uint16_t imported_items, + Profile* profile, bool allowed) { if (!allowed) { // Notify to webui to finish import process and launch tab modal dialog @@ -165,7 +178,7 @@ void BraveImportDataHandler::OnGetDiskAccessPermission( return; } - StartImportImpl(source_profile, imported_items); + StartImportImpl(source_profile, imported_items, profile); } void BraveImportDataHandler::DidStopLoading() { diff --git a/browser/ui/webui/settings/brave_import_data_handler.h b/browser/ui/webui/settings/brave_import_data_handler.h index e1e55da44028..3aa162fe150c 100644 --- a/browser/ui/webui/settings/brave_import_data_handler.h +++ b/browser/ui/webui/settings/brave_import_data_handler.h @@ -38,21 +38,28 @@ class BraveImportDataHandler : public ImportDataHandler, BraveImportDataHandler(const BraveImportDataHandler&) = delete; BraveImportDataHandler& operator=(const BraveImportDataHandler&) = delete; - private: + protected: + const importer::SourceProfile& GetSourceProfileAt(int browser_index); + void HandleImportData(const base::Value::List& args); // ImportDataHandler overrides: void StartImport(const importer::SourceProfile& source_profile, uint16_t imported_items) override; void StartImportImpl(const importer::SourceProfile& source_profile, - uint16_t imported_items); - void NotifyImportProgress(const importer::SourceProfile& source_profile, - const base::Value& info); + uint16_t imported_items, + Profile* profile); + virtual void NotifyImportProgress( + const importer::SourceProfile& source_profile, + const base::Value& info); void OnImportEnded(const base::FilePath& source_path); + #if BUILDFLAG(IS_MAC) void CheckDiskAccess(const importer::SourceProfile& source_profile, - uint16_t imported_items); + uint16_t imported_items, + Profile* profile); void OnGetDiskAccessPermission(const importer::SourceProfile& source_profile, uint16_t imported_items, + Profile* profile, bool allowed); // content::WebContentsObserver overrides: @@ -60,7 +67,7 @@ class BraveImportDataHandler : public ImportDataHandler, bool guide_dialog_is_requested_ = false; #endif - + private: std::unordered_map> import_observers_; base::WeakPtrFactory weak_factory_{this}; diff --git a/components/brave_welcome_ui/api/import_data_browser_proxy.ts b/components/brave_welcome_ui/api/import_data_browser_proxy.ts index 083cce85e445..9c12e90cdd9b 100644 --- a/components/brave_welcome_ui/api/import_data_browser_proxy.ts +++ b/components/brave_welcome_ui/api/import_data_browser_proxy.ts @@ -49,6 +49,15 @@ export interface ImportDataBrowserProxy { sourceBrowserProfileIndex: number, types: {[type: string]: boolean}) => void + /** + * Starts importing data for the specified source browser profiles. The C++ + * responds with the 'import-data-status-changed' WebUIListener event. + * @param types Which types of data to import. + */ + importDataBulk: ( + sourceBrowserProfileIndex: number[], + types: {[type: string]: boolean}) => void + /** * Prompts the user to choose a bookmarks file to import bookmarks from. */ @@ -65,6 +74,11 @@ export class ImportDataBrowserProxyImpl implements ImportDataBrowserProxy { chrome.send('importData', [sourceBrowserProfileIndex, types]) } + importDataBulk ( + sourceBrowserProfileIndex: number[], types: {[type: string]: boolean}) { + chrome.send('importDataBulk', [sourceBrowserProfileIndex, types]) + } + importFromBookmarksFile () { chrome.send('importFromBookmarksFile') } diff --git a/components/brave_welcome_ui/components/select-profile/index.tsx b/components/brave_welcome_ui/components/select-profile/index.tsx index b11f539ca3d4..e88fdf94970d 100644 --- a/components/brave_welcome_ui/components/select-profile/index.tsx +++ b/components/brave_welcome_ui/components/select-profile/index.tsx @@ -72,12 +72,17 @@ function SelectProfile () { const handleImportProfiles = () => { if (selectedProfiles.size <= 0) return - + let entries: number[] = [] selectedProfiles.forEach((entry) => { - ImportDataBrowserProxyImpl.getInstance().importData(entry, defaultImportTypes) + entries.push(entry) incrementCount() }) + if (entries.length === 1) { + ImportDataBrowserProxyImpl.getInstance().importData(entries[0], defaultImportTypes) + } else { + ImportDataBrowserProxyImpl.getInstance().importDataBulk(entries, defaultImportTypes) + } WelcomeBrowserProxyImpl.getInstance().recordP3A({ currentScreen: ViewType.ImportSelectProfile, isFinished: false, isSkipped: false }) } const getImportEntryName = (entry: any) => { From 9df874a373ea7d1679be62c6dbf5d17235539eb5 Mon Sep 17 00:00:00 2001 From: sergei Date: Mon, 16 Jan 2023 15:58:35 +0300 Subject: [PATCH 2/5] Remove parallel import feature --- browser/ui/BUILD.gn | 2 -- .../brave_import_bulk_data_handler.cc | 1 - .../settings/brave_import_data_handler.cc | 13 ++-------- browser/ui/webui/settings/import_feature.cc | 26 ------------------- browser/ui/webui/settings/import_feature.h | 17 ------------ 5 files changed, 2 insertions(+), 57 deletions(-) delete mode 100644 browser/ui/webui/settings/import_feature.cc delete mode 100644 browser/ui/webui/settings/import_feature.h diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn index fcee5b662c6f..223e6d90d442 100644 --- a/browser/ui/BUILD.gn +++ b/browser/ui/BUILD.gn @@ -168,8 +168,6 @@ source_set("ui") { "webui/settings/brave_wallet_handler.h", "webui/settings/default_brave_shields_handler.cc", "webui/settings/default_brave_shields_handler.h", - "webui/settings/import_feature.cc", - "webui/settings/import_feature.h", "webui/speedreader/speedreader_panel_data_handler_impl.cc", "webui/speedreader/speedreader_panel_data_handler_impl.h", "webui/speedreader/speedreader_panel_handler_impl.cc", diff --git a/browser/ui/webui/settings/brave_import_bulk_data_handler.cc b/browser/ui/webui/settings/brave_import_bulk_data_handler.cc index 2f9be8b1eac1..811d20290559 100644 --- a/browser/ui/webui/settings/brave_import_bulk_data_handler.cc +++ b/browser/ui/webui/settings/brave_import_bulk_data_handler.cc @@ -16,7 +16,6 @@ #include "base/json/json_reader.h" #include "base/rand_util.h" #include "base/strings/utf_string_conversions.h" -#include "brave/browser/ui/webui/settings/import_feature.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/profiles/profile_avatar_icon_util.h" #include "chrome/browser/profiles/profile_manager.h" diff --git a/browser/ui/webui/settings/brave_import_data_handler.cc b/browser/ui/webui/settings/brave_import_data_handler.cc index e4855493c5f9..eb82298a622e 100644 --- a/browser/ui/webui/settings/brave_import_data_handler.cc +++ b/browser/ui/webui/settings/brave_import_data_handler.cc @@ -7,10 +7,9 @@ #include #include -#include +#include #include "brave/browser/importer/brave_external_process_importer_host.h" -#include "brave/browser/ui/webui/settings/import_feature.h" #include "chrome/browser/importer/importer_list.h" #include "chrome/browser/importer/profile_writer.h" #include "chrome/browser/profiles/profile.h" @@ -81,13 +80,6 @@ void BraveImportDataHandler::StartImportImpl( const importer::SourceProfile& source_profile, uint16_t imported_items, Profile* profile) { - // Temporary flag to keep old way until - // https://github.com/brave/brave-core/pull/15637 landed. - // Should be removed in that PR. - if (!IsParallelImportEnabled(web_ui()->GetWebContents()->GetVisibleURL())) { - ImportDataHandler::StartImport(source_profile, imported_items); - return; - } // If another import is already ongoing, let it finish silently. if (import_observers_.count(source_profile.source_path)) import_observers_.erase(source_profile.source_path); @@ -167,9 +159,8 @@ void BraveImportDataHandler::OnGetDiskAccessPermission( // to guide full disk access information to users. // Guide dialog will be opened after import dialog is closed. FireWebUIListener("import-data-status-changed", base::Value("failed")); - if (IsParallelImportEnabled(web_ui()->GetWebContents()->GetVisibleURL())) { + if (import_observers_.count(source_profile.source_path)) import_observers_[source_profile.source_path]->ImportEnded(); - } // Observing web_contents is started here to know the closing timing of // import dialog. Observe(web_ui()->GetWebContents()); diff --git a/browser/ui/webui/settings/import_feature.cc b/browser/ui/webui/settings/import_feature.cc deleted file mode 100644 index e938e7a61ad7..000000000000 --- a/browser/ui/webui/settings/import_feature.cc +++ /dev/null @@ -1,26 +0,0 @@ -/* Copyright (c) 2022 The Brave Authors. All rights reserved. - * 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/ui/webui/settings/import_feature.h" - -#include "base/feature_list.h" -#include "base/metrics/field_trial_params.h" -#include "brave/components/constants/webui_url_constants.h" -#include "url/gurl.h" - -namespace settings { - -BASE_FEATURE(kParallelImports, - "ParallelImports", - base::FEATURE_ENABLED_BY_DEFAULT); - -// Temporary flag to keep old way until -// https://github.com/brave/brave-core/pull/15637 landed. -bool IsParallelImportEnabled(const GURL& url) { - return base::FeatureList::IsEnabled(kParallelImports) && - url.DomainIs(kWelcomeHost); -} - -} // namespace settings diff --git a/browser/ui/webui/settings/import_feature.h b/browser/ui/webui/settings/import_feature.h deleted file mode 100644 index 42070d28ec6b..000000000000 --- a/browser/ui/webui/settings/import_feature.h +++ /dev/null @@ -1,17 +0,0 @@ -/* Copyright (c) 2022 The Brave Authors. All rights reserved. - * 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_BROWSER_UI_WEBUI_SETTINGS_IMPORT_FEATURE_H_ -#define BRAVE_BROWSER_UI_WEBUI_SETTINGS_IMPORT_FEATURE_H_ - -#include "base/feature_list.h" - -class GURL; - -namespace settings { -bool IsParallelImportEnabled(const GURL& url); -} // namespace settings - -#endif // BRAVE_BROWSER_UI_WEBUI_SETTINGS_IMPORT_FEATURE_H_ From b12fde443f5eaa8af8d59ae95dd938d15f27ea7b Mon Sep 17 00:00:00 2001 From: Sergey P Date: Tue, 17 Jan 2023 17:03:14 +0300 Subject: [PATCH 3/5] Check disk access before creating target profile --- .../brave_import_bulk_data_handler.cc | 40 +++++++------ .../settings/brave_import_bulk_data_handler.h | 15 +++-- .../settings/brave_import_data_handler.cc | 56 +++++++++++-------- .../settings/brave_import_data_handler.h | 18 +++--- 4 files changed, 72 insertions(+), 57 deletions(-) diff --git a/browser/ui/webui/settings/brave_import_bulk_data_handler.cc b/browser/ui/webui/settings/brave_import_bulk_data_handler.cc index 811d20290559..b784966b4a57 100644 --- a/browser/ui/webui/settings/brave_import_bulk_data_handler.cc +++ b/browser/ui/webui/settings/brave_import_bulk_data_handler.cc @@ -62,17 +62,6 @@ void BraveImportBulkDataHandler::PrepareProfile( std::move(profile_ready_callback))); } -void BraveImportBulkDataHandler::ProfileReadyForImport( - const importer::SourceProfile& source_profile, - uint16_t imported_items, - Profile* profile) { -#if BUILDFLAG(IS_MAC) - CheckDiskAccess(source_profile, imported_items, profile); -#else - StartImportImpl(source_profile, imported_items, profile); -#endif -} - void BraveImportBulkDataHandler::HandleImportDataBulk( const base::Value::List& args) { CHECK_GE(args.size(), 2u); @@ -112,22 +101,31 @@ void BraveImportBulkDataHandler::StartImport( } if (!imported_items) return; - PrepareProfile( - source_profile.profile.empty() ? source_profile.importer_name - : source_profile.profile, - base::BindOnce(&BraveImportBulkDataHandler::ProfileReadyForImport, - weak_factory_.GetWeakPtr(), source_profile, - imported_items)); + auto profile_name = source_profile.profile.empty() + ? source_profile.importer_name + : source_profile.profile; + auto import_callback = base::BindOnce( + &BraveImportBulkDataHandler::StartImportImpl, weak_factory_.GetWeakPtr(), + source_profile, imported_items); +#if BUILDFLAG(IS_MAC) + CheckDiskAccess(imported_items, source_profile.source_path, + source_profile.importer_type, + base::BindOnce(&BraveImportBulkDataHandler::PrepareProfile, + weak_factory_.GetWeakPtr(), profile_name, + std::move(import_callback))); +#else + PrepareProfile(profile_name, std::move(import_callback)); +#endif } void BraveImportBulkDataHandler::NotifyImportProgress( const importer::SourceProfile& source_profile, const base::Value& info) { - BraveImportDataHandler::NotifyImportProgress(source_profile, info); + FireWebUIListener("brave-import-data-status-changed", info); +} - const std::string* event = info.FindStringKey("event"); - if (!event || *event != "ImportEnded") - return; +void BraveImportBulkDataHandler::OnImportEnded( + const importer::SourceProfile& source_profile) { auto index = GetProfileIndex(source_profile); if (index.has_value()) { importing_profiles_.erase(index.value()); diff --git a/browser/ui/webui/settings/brave_import_bulk_data_handler.h b/browser/ui/webui/settings/brave_import_bulk_data_handler.h index e7fbbc4a3e24..98bf7c2465e5 100644 --- a/browser/ui/webui/settings/brave_import_bulk_data_handler.h +++ b/browser/ui/webui/settings/brave_import_bulk_data_handler.h @@ -31,26 +31,31 @@ class BraveImportBulkDataHandler : public BraveImportDataHandler { BraveImportBulkDataHandler& operator=(const BraveImportBulkDataHandler&) = delete; + protected: + void HandleImportDataBulk(const base::Value::List& args); + + absl::optional GetProfileIndex( + const importer::SourceProfile& source_profile); + void PrepareProfile(const std::u16string& name, ProfileReadyCallback callback); void ProfileReadyForImport(const importer::SourceProfile& source_profile, uint16_t imported_items, Profile* profile); + // BraveImportDataHandler void NotifyImportProgress(const importer::SourceProfile& source_profile, const base::Value& info) override; + void OnImportEnded(const importer::SourceProfile& source_profile) override; // SettingsPageUIHandler void RegisterMessages() override; - private: - absl::optional GetProfileIndex( - const importer::SourceProfile& source_profile); - - void HandleImportDataBulk(const base::Value::List& args); // ImportDataHandler overrides: void StartImport(const importer::SourceProfile& source_profile, uint16_t imported_items) override; + + private: base::flat_set importing_profiles_; base::WeakPtrFactory weak_factory_{this}; }; diff --git a/browser/ui/webui/settings/brave_import_data_handler.cc b/browser/ui/webui/settings/brave_import_data_handler.cc index eb82298a622e..81be03483f4e 100644 --- a/browser/ui/webui/settings/brave_import_data_handler.cc +++ b/browser/ui/webui/settings/brave_import_data_handler.cc @@ -56,6 +56,8 @@ bool HasProperDiskAccessPermission(uint16_t imported_items) { return true; } #endif // BUILDFLAG(IS_MAC) +const char kImportStatusSucceeded[] = "succeeded"; +const char kImportStatusFailed[] = "failed"; } // namespace namespace settings { @@ -70,7 +72,11 @@ void BraveImportDataHandler::StartImport( return; Profile* profile = Profile::FromWebUI(web_ui()); #if BUILDFLAG(IS_MAC) - CheckDiskAccess(source_profile, imported_items, profile); + CheckDiskAccess(imported_items, source_profile.source_path, + source_profile.importer_type, + base::BindOnce(&BraveImportDataHandler::StartImportImpl, + weak_factory_.GetWeakPtr(), source_profile, + imported_items, profile)); #else StartImportImpl(source_profile, imported_items, profile); #endif @@ -99,13 +105,15 @@ void BraveImportDataHandler::StartImportImpl( void BraveImportDataHandler::NotifyImportProgress( const importer::SourceProfile& source_profile, const base::Value& info) { - FireWebUIListener("brave-import-data-status-changed", info); const std::string* event = info.FindStringKey("event"); - if (event && *event == "ImportEnded") { + if (!event) + return; + if (*event == "ImportItemEnded") { + import_did_succeed_ = true; + } else if (*event == "ImportEnded") { content::GetUIThreadTaskRunner({})->PostTask( - FROM_HERE, - base::BindOnce(&BraveImportDataHandler::OnImportEnded, - weak_factory_.GetWeakPtr(), source_profile.source_path)); + FROM_HERE, base::BindOnce(&BraveImportDataHandler::OnImportEnded, + weak_factory_.GetWeakPtr(), source_profile)); } } @@ -113,8 +121,12 @@ void BraveImportDataHandler::HandleImportData(const base::Value::List& args) { ImportDataHandler::HandleImportData(args); } -void BraveImportDataHandler::OnImportEnded(const base::FilePath& source_path) { - import_observers_.erase(source_path); +void BraveImportDataHandler::OnImportEnded( + const importer::SourceProfile& source_profile) { + import_observers_.erase(source_profile.source_path); + FireWebUIListener("import-data-status-changed", + base::Value(import_did_succeed_ ? kImportStatusSucceeded + : kImportStatusFailed)); } const importer::SourceProfile& BraveImportDataHandler::GetSourceProfileAt( @@ -124,43 +136,39 @@ const importer::SourceProfile& BraveImportDataHandler::GetSourceProfileAt( #if BUILDFLAG(IS_MAC) void BraveImportDataHandler::CheckDiskAccess( - const importer::SourceProfile& source_profile, uint16_t imported_items, - Profile* profile) { + base::FilePath source_path, + importer::ImporterType importer_type, + ContinueImportCallback callback) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); guide_dialog_is_requested_ = false; - if (!imported_items) - return; - - if (source_profile.importer_type == importer::TYPE_SAFARI) { + if (importer_type == importer::TYPE_SAFARI) { // Start import if Brave has full disk access permission. // If not, show dialog that has infos about that permission. base::ThreadPool::PostTaskAndReplyWithResult( FROM_HERE, {base::MayBlock()}, base::BindOnce(&HasProperDiskAccessPermission, imported_items), base::BindOnce(&BraveImportDataHandler::OnGetDiskAccessPermission, - weak_factory_.GetWeakPtr(), source_profile, - imported_items, profile)); + weak_factory_.GetWeakPtr(), std::move(callback), + source_path)); return; } - - StartImportImpl(source_profile, imported_items, profile); + std::move(callback).Run(); } void BraveImportDataHandler::OnGetDiskAccessPermission( - const importer::SourceProfile& source_profile, - uint16_t imported_items, - Profile* profile, + ContinueImportCallback callback, + base::FilePath source_path, bool allowed) { if (!allowed) { // Notify to webui to finish import process and launch tab modal dialog // to guide full disk access information to users. // Guide dialog will be opened after import dialog is closed. FireWebUIListener("import-data-status-changed", base::Value("failed")); - if (import_observers_.count(source_profile.source_path)) - import_observers_[source_profile.source_path]->ImportEnded(); + if (import_observers_.count(source_path)) + import_observers_[source_path]->ImportEnded(); // Observing web_contents is started here to know the closing timing of // import dialog. Observe(web_ui()->GetWebContents()); @@ -169,7 +177,7 @@ void BraveImportDataHandler::OnGetDiskAccessPermission( return; } - StartImportImpl(source_profile, imported_items, profile); + std::move(callback).Run(); } void BraveImportDataHandler::DidStopLoading() { diff --git a/browser/ui/webui/settings/brave_import_data_handler.h b/browser/ui/webui/settings/brave_import_data_handler.h index 3aa162fe150c..b679ee5fc21e 100644 --- a/browser/ui/webui/settings/brave_import_data_handler.h +++ b/browser/ui/webui/settings/brave_import_data_handler.h @@ -39,6 +39,8 @@ class BraveImportDataHandler : public ImportDataHandler, BraveImportDataHandler& operator=(const BraveImportDataHandler&) = delete; protected: + using ContinueImportCallback = base::OnceCallback; + const importer::SourceProfile& GetSourceProfileAt(int browser_index); void HandleImportData(const base::Value::List& args); // ImportDataHandler overrides: @@ -51,15 +53,17 @@ class BraveImportDataHandler : public ImportDataHandler, virtual void NotifyImportProgress( const importer::SourceProfile& source_profile, const base::Value& info); - void OnImportEnded(const base::FilePath& source_path); + virtual void OnImportEnded(const importer::SourceProfile& source_profile); + void OnStartImport(const importer::SourceProfile& source_profile, + uint16_t imported_items); #if BUILDFLAG(IS_MAC) - void CheckDiskAccess(const importer::SourceProfile& source_profile, - uint16_t imported_items, - Profile* profile); - void OnGetDiskAccessPermission(const importer::SourceProfile& source_profile, - uint16_t imported_items, - Profile* profile, + void CheckDiskAccess(uint16_t imported_items, + base::FilePath source_path, + importer::ImporterType importer_type, + ContinueImportCallback callback); + void OnGetDiskAccessPermission(ContinueImportCallback callback, + base::FilePath source_path, bool allowed); // content::WebContentsObserver overrides: From 2dce8efcccc2797c53b4cf9aae3bbfd2fe0332cd Mon Sep 17 00:00:00 2001 From: sergei Date: Thu, 19 Jan 2023 13:27:05 +0300 Subject: [PATCH 4/5] reuse existing profile --- .../brave_import_bulk_data_handler.cc | 73 ++++++++++++++----- 1 file changed, 56 insertions(+), 17 deletions(-) diff --git a/browser/ui/webui/settings/brave_import_bulk_data_handler.cc b/browser/ui/webui/settings/brave_import_bulk_data_handler.cc index b784966b4a57..e4e863961702 100644 --- a/browser/ui/webui/settings/brave_import_bulk_data_handler.cc +++ b/browser/ui/webui/settings/brave_import_bulk_data_handler.cc @@ -9,6 +9,7 @@ #include #include #include +#include #include "base/bind.h" #include "base/files/file_path.h" @@ -17,6 +18,8 @@ #include "base/rand_util.h" #include "base/strings/utf_string_conversions.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/profiles/profile_attributes_entry.h" +#include "chrome/browser/profiles/profile_attributes_storage.h" #include "chrome/browser/profiles/profile_avatar_icon_util.h" #include "chrome/browser/profiles/profile_manager.h" #include "chrome/common/pref_names.h" @@ -24,6 +27,21 @@ namespace settings { +namespace { +base::FilePath GetProfilePathByName(const std::u16string& name) { + std::vector entries = + g_browser_process->profile_manager() + ->GetProfileAttributesStorage() + .GetAllProfilesAttributesSortedByName(); + for (auto* it : entries) { + if (it->GetName() == name) { + return it->GetPath(); + } + } + return base::FilePath(); +} +} // namespace + BraveImportBulkDataHandler::BraveImportBulkDataHandler() = default; BraveImportBulkDataHandler::~BraveImportBulkDataHandler() = default; @@ -39,27 +57,48 @@ void BraveImportBulkDataHandler::PrepareProfile( const std::u16string& name, ProfileReadyCallback profile_ready_callback) { ProfileManager* profile_manager = g_browser_process->profile_manager(); - Profile* profile = - profile_manager->GetProfileByPath(profile_manager->user_data_dir().Append( - base::FilePath::FromASCII(base::UTF16ToUTF8(name)))); - - if (profile) { - std::move(profile_ready_callback).Run(profile); + auto profile_path = GetProfilePathByName(name); + // If a profile with the given path is currently managed by this object and + // fully initialized, return a pointer to the corresponding Profile object. + Profile* loaded_profile = profile_manager->GetProfileByPath(profile_path); + if (loaded_profile) { + std::move(profile_ready_callback).Run(loaded_profile); return; } - - auto avatar_index = base::RandInt(profiles::GetModernAvatarIconStartIndex(), - profiles::GetDefaultAvatarIconCount()); - ProfileManager::CreateMultiProfileAsync( - name, avatar_index, false, + // Asynchronously loads an existing profile given its |profile_base_name| + // (which is the directory name within the user data directory), optionally in + // |incognito| mode. The |callback| will be called with the Profile when it + // has been loaded, or with a nullptr otherwise. + profile_manager->LoadProfileByPath( + profile_path, false, base::BindOnce( - [](ProfileReadyCallback initialized_callback, Profile* profile) { - CHECK(profile); - // Migrate welcome page flag to new profiles. - profile->GetPrefs()->SetBoolean(prefs::kHasSeenWelcomePage, true); - std::move(initialized_callback).Run(profile); + [](ProfileReadyCallback profile_callback, const std::u16string& name, + Profile* existing_profile) { + // Existing profile loaded, reuse it. + if (existing_profile) { + std::move(profile_callback).Run(existing_profile); + return; + } + // Asynchronously creates a new profile in the next available + // multiprofile directory. Directories are named "profile_1", + // "profile_2", etc., in sequence of creation. + auto avatar_index = + base::RandInt(profiles::GetModernAvatarIconStartIndex(), + profiles::GetDefaultAvatarIconCount()); + ProfileManager::CreateMultiProfileAsync( + name, avatar_index, false, + base::BindOnce( + [](ProfileReadyCallback initialized_callback, + Profile* created_profile) { + CHECK(created_profile); + // Migrate welcome page flag to new profiles. + created_profile->GetPrefs()->SetBoolean( + prefs::kHasSeenWelcomePage, true); + std::move(initialized_callback).Run(created_profile); + }, + std::move(profile_callback))); }, - std::move(profile_ready_callback))); + std::move(profile_ready_callback), name)); } void BraveImportBulkDataHandler::HandleImportDataBulk( From cb5bb3536bc852734d3f37584525b38bd8d7e4cd Mon Sep 17 00:00:00 2001 From: sergei Date: Fri, 20 Jan 2023 17:43:28 +0300 Subject: [PATCH 5/5] Nits from review --- .../ui/webui/settings/brave_import_bulk_data_handler.cc | 9 +++++---- browser/ui/webui/settings/brave_import_data_handler.cc | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/browser/ui/webui/settings/brave_import_bulk_data_handler.cc b/browser/ui/webui/settings/brave_import_bulk_data_handler.cc index e4e863961702..cafd30ec7170 100644 --- a/browser/ui/webui/settings/brave_import_bulk_data_handler.cc +++ b/browser/ui/webui/settings/brave_import_bulk_data_handler.cc @@ -107,13 +107,12 @@ void BraveImportBulkDataHandler::HandleImportDataBulk( const auto& list = args[0].GetList(); // Bulk profiles import assumes new profiles will be created on our side if // they do not exist. - const base::Value& types = args[1]; for (const auto& it : list) { int browser_index = it.GetInt(); importing_profiles_.insert(browser_index); base::Value::List single_profile_args; single_profile_args.Append(base::Value(browser_index)); - single_profile_args.Append(types.Clone()); + single_profile_args.Append(args[1].Clone()); BraveImportDataHandler::HandleImportData(single_profile_args); } } @@ -132,14 +131,14 @@ absl::optional BraveImportBulkDataHandler::GetProfileIndex( void BraveImportBulkDataHandler::StartImport( const importer::SourceProfile& source_profile, uint16_t imported_items) { + if (!imported_items) + return; // If profile is not from the bulk import request fallback to single profile // import. if (!GetProfileIndex(source_profile).has_value()) { BraveImportDataHandler::StartImport(source_profile, imported_items); return; } - if (!imported_items) - return; auto profile_name = source_profile.profile.empty() ? source_profile.importer_name : source_profile.profile; @@ -168,7 +167,9 @@ void BraveImportBulkDataHandler::OnImportEnded( auto index = GetProfileIndex(source_profile); if (index.has_value()) { importing_profiles_.erase(index.value()); + return; } + BraveImportDataHandler::OnImportEnded(source_profile); } } // namespace settings diff --git a/browser/ui/webui/settings/brave_import_data_handler.cc b/browser/ui/webui/settings/brave_import_data_handler.cc index 81be03483f4e..6daf67123824 100644 --- a/browser/ui/webui/settings/brave_import_data_handler.cc +++ b/browser/ui/webui/settings/brave_import_data_handler.cc @@ -56,8 +56,8 @@ bool HasProperDiskAccessPermission(uint16_t imported_items) { return true; } #endif // BUILDFLAG(IS_MAC) -const char kImportStatusSucceeded[] = "succeeded"; -const char kImportStatusFailed[] = "failed"; +constexpr char kImportStatusSucceeded[] = "succeeded"; +constexpr char kImportStatusFailed[] = "failed"; } // namespace namespace settings {