diff options
Diffstat (limited to 'chrome/browser')
21 files changed, 311 insertions, 707 deletions
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index c249c89..1d7b4f0 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -783,9 +783,7 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() { (!force_first_run && parsed_command_line().HasSwitch(switches::kNoFirstRun)); - is_first_run = - (force_first_run || first_run::IsChromeFirstRun()) && - !ProfileManager::IsImportProcess(parsed_command_line()); + is_first_run = force_first_run || first_run::IsChromeFirstRun(); #endif scoped_refptr<base::SequencedTaskRunner> local_state_task_runner = @@ -1158,19 +1156,9 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { return chrome::RESULT_CODE_PACK_EXTENSION_ERROR; } - bool pass_command_line = true; - -#if !defined(OS_MACOSX) - // In environments other than Mac OS X we support import of settings - // from other browsers. In case this process is a short-lived "import" - // process that another browser runs just to import the settings, we - // don't want to be checking for another browser process, by design. - pass_command_line = !ProfileManager::IsImportProcess(parsed_command_line()); -#endif - // If we're being launched just to check the connector policy, we are // short-lived and don't want to be passing that switch off. - pass_command_line = pass_command_line && !parsed_command_line().HasSwitch( + bool pass_command_line = !parsed_command_line().HasSwitch( switches::kCheckCloudPrintConnectorPolicy); if (pass_command_line) { @@ -1274,16 +1262,6 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { #endif // Post-profile init --------------------------------------------------------- -#if !defined(OS_MACOSX) && !defined(OS_ANDROID) - // Importing other browser settings is done in a browser-like process - // that exits when this task has finished. - // TODO(port): Port the Mac's IPC-based implementation to other platforms to - // replace this implementation. http://crbug.com/22142 - if (ProfileManager::IsImportProcess(parsed_command_line())) { - return first_run::ImportNow(profile_, parsed_command_line()); - } -#endif - #if defined(OS_WIN) // Do the tasks if chrome has been upgraded while it was last running. if (!already_running && upgrade_util::DoUpgradeTasks(parsed_command_line())) @@ -1339,7 +1317,9 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { first_run::AutoImport(profile_, master_prefs_->homepage_defined, master_prefs_->do_import_items, - master_prefs_->dont_import_items); + master_prefs_->dont_import_items, + master_prefs_->import_bookmarks_path); + // Note: this can pop the first run consent dialog on linux. first_run::DoPostImportTasks(profile_, master_prefs_->make_chrome_default); diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 8b929c9..6cfddc3 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -410,7 +410,6 @@ ExtensionService::ExtensionService(Profile* profile, g_browser_process->local_state())); if (extensions_enabled_) { - CHECK(!ProfileManager::IsImportProcess(*command_line)); extensions::ExternalProviderImpl::CreateExternalProviders( this, profile_, &external_extension_providers_); } @@ -566,9 +565,6 @@ void ExtensionService::Init() { DCHECK_EQ(extensions_.size(), 0u); const CommandLine* cmd_line = CommandLine::ForCurrentProcess(); - - CHECK(!ProfileManager::IsImportProcess(*cmd_line)); - if (cmd_line->HasSwitch(switches::kInstallFromWebstore) || cmd_line->HasSwitch(switches::kLimitedInstallFromWebstore)) { // The sole purpose of this launch is to install a new extension from CWS diff --git a/chrome/browser/extensions/extension_system.cc b/chrome/browser/extensions/extension_system.cc index 9ceb4d4..390b1b8 100644 --- a/chrome/browser/extensions/extension_system.cc +++ b/chrome/browser/extensions/extension_system.cc @@ -207,7 +207,6 @@ void ExtensionSystemImpl::Shared::Init(bool extensions_enabled) { // If import is going to run in a separate process (the profile itself is on // the main process), wait for import to finish before initializing the // routers. - CHECK(!ProfileManager::IsImportProcess(*command_line)); if (g_browser_process->profile_manager()->will_import()) { extension_service_->InitEventRoutersAfterImport(); } else { diff --git a/chrome/browser/first_run/first_run.cc b/chrome/browser/first_run/first_run.cc index 53924a3..9ff9d5b 100644 --- a/chrome/browser/first_run/first_run.cc +++ b/chrome/browser/first_run/first_run.cc @@ -27,6 +27,7 @@ #include "chrome/browser/importer/importer_list.h" #include "chrome/browser/importer/importer_progress_observer.h" #include "chrome/browser/importer/importer_type.h" +#include "chrome/browser/importer/profile_writer.h" #include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/search_engines/template_url_service.h" #include "chrome/browser/search_engines/template_url_service_factory.h" @@ -61,14 +62,49 @@ using content::UserMetricsAction; namespace { +// A bitfield formed from values in AutoImportState to record the state of +// AutoImport. This is used in testing to verify import startup actions that +// occur before an observer can be registered in the test. +uint16 g_auto_import_state = first_run::AUTO_IMPORT_NONE; + // Flags for functions of similar name. bool g_should_show_welcome_page = false; bool g_should_do_autofill_personal_data_manager_first_run = false; -// Flags indicating whether a first-run profile auto import was performed, and -// whether the importer process exited successfully. -bool did_perform_profile_import = false; -bool profile_import_exited_successfully = false; +// This class acts as an observer for the ImporterProgressObserver::ImportEnded +// callback. When the import process is started, certain errors may cause +// ImportEnded() to be called synchronously, but the typical case is that +// ImportEnded() is called asynchronously. Thus we have to handle both cases. +class ImportEndedObserver : public importer::ImporterProgressObserver { + public: + ImportEndedObserver() : ended_(false), + should_quit_message_loop_(false) {} + virtual ~ImportEndedObserver() {} + + // importer::ImporterProgressObserver: + virtual void ImportStarted() OVERRIDE {} + virtual void ImportItemStarted(importer::ImportItem item) OVERRIDE {} + virtual void ImportItemEnded(importer::ImportItem item) OVERRIDE {} + virtual void ImportEnded() OVERRIDE { + ended_ = true; + if (should_quit_message_loop_) + MessageLoop::current()->Quit(); + } + + void set_should_quit_message_loop() { + should_quit_message_loop_ = true; + } + + bool ended() const { + return ended_; + } + + private: + // Set if the import has ended. + bool ended_; + + bool should_quit_message_loop_; +}; // Helper class that performs delayed first-run tasks that need more of the // chrome infrastructure to be up and running before they can be attempted. @@ -191,28 +227,20 @@ void SetImportItem(PrefService* user_prefs, user_prefs->ClearPref(pref_path); } -// Imports bookmarks from an html file. The path to the file is provided in -// the command line. -void ImportFromFile(Profile* profile, const CommandLine& cmdline) { - base::FilePath file_path = - cmdline.GetSwitchValuePath(switches::kImportFromFile); - if (file_path.empty()) { - NOTREACHED(); - return; - } - - // Deletes itself. - ImporterHost* importer_host = new ImporterHost; - importer_host->set_headless(); - - importer::SourceProfile source_profile; - source_profile.importer_type = importer::TYPE_BOOKMARKS_FILE; - source_profile.source_path = file_path; - - first_run::internal::ImportEndedObserver observer; +// Launches the import, via |importer_host|, from |source_profile| into +// |target_profile| for the items specified in the |items_to_import| bitfield. +// This may be done in a separate process depending on the platform, but it will +// always block until done. +void ImportFromSourceProfile(ImporterHost* importer_host, + const importer::SourceProfile& source_profile, + Profile* target_profile, + uint16 items_to_import) { + ImportEndedObserver observer; importer_host->SetObserver(&observer); - importer_host->StartImportSettings( - source_profile, profile, importer::FAVORITES, new ProfileWriter(profile)); + importer_host->StartImportSettings(source_profile, + target_profile, + items_to_import, + new ProfileWriter(target_profile)); // If the import process has not errored out, block on it. if (!observer.ended()) { observer.set_should_quit_message_loop(); @@ -220,6 +248,46 @@ void ImportFromFile(Profile* profile, const CommandLine& cmdline) { } } +// Imports bookmarks from an html file whose path is provided by +// |import_bookmarks_path|. +void ImportFromFile(Profile* profile, + ImporterHost* file_importer_host, + const std::string& import_bookmarks_path) { + importer::SourceProfile source_profile; + source_profile.importer_type = importer::TYPE_BOOKMARKS_FILE; + + const base::FilePath::StringType& import_bookmarks_path_str = +#if defined(OS_WIN) + UTF8ToUTF16(import_bookmarks_path); +#else + import_bookmarks_path; +#endif + source_profile.source_path = base::FilePath(import_bookmarks_path_str); + + ImportFromSourceProfile(file_importer_host, source_profile, profile, + importer::FAVORITES); + g_auto_import_state |= first_run::AUTO_IMPORT_BOOKMARKS_FILE_IMPORTED; +} + +// Imports settings from the first profile in |importer_list|. +void ImportSettings(Profile* profile, + ImporterHost* importer_host, + scoped_refptr<ImporterList> importer_list, + int items_to_import) { + const importer::SourceProfile& source_profile = + importer_list->GetSourceProfileAt(0); + + // Ensure that importers aren't requested to import items that they do not + // support. If there is no overlap, skip. + items_to_import &= source_profile.services_supported; + if (items_to_import == 0) + return; + + ImportFromSourceProfile(importer_host, source_profile, profile, + items_to_import); + g_auto_import_state |= first_run::AUTO_IMPORT_PROFILE_IMPORTED; +} + GURL UrlFromString(const std::string& in) { return GURL(in); } @@ -241,16 +309,6 @@ FirstRunState first_run_ = FIRST_RUN_UNKNOWN; static base::LazyInstance<base::FilePath> master_prefs_path_for_testing = LAZY_INSTANCE_INITIALIZER; -// TODO(gab): This will go back inline above when it is moved to first_run.cc -// (see TODO above), but needs to be separate for now to satisfy clang error: -// "[chromium-style] virtual methods with non-empty bodies shouldn't be declared -// inline". -void ImportEndedObserver::ImportEnded() { - ended_ = true; - if (should_quit_message_loop_) - MessageLoop::current()->Quit(); -} - installer::MasterPreferences* LoadMasterPrefs(base::FilePath* master_prefs_path) { if (!master_prefs_path_for_testing.Get().empty()) @@ -354,11 +412,15 @@ void SetupMasterPrefsFromInstallPrefs( out_prefs->suppress_first_run_default_browser_prompt = true; } + install_prefs.GetString( + installer::master_preferences::kDistroImportBookmarksFromFilePref, + &out_prefs->import_bookmarks_path); + out_prefs->variations_seed = install_prefs.GetVariationsSeed(); install_prefs.GetString( - installer::master_preferences::kDistroSuppressDefaultBrowserPromptPref, - &out_prefs->suppress_default_browser_prompt_for_version); + installer::master_preferences::kDistroSuppressDefaultBrowserPromptPref, + &out_prefs->suppress_default_browser_prompt_for_version); } void SetDefaultBrowser(installer::MasterPreferences* install_prefs){ @@ -390,18 +452,6 @@ bool IsOrganicFirstRun() { } #endif -int ImportBookmarkFromFileIfNeeded(Profile* profile, - const CommandLine& cmdline) { - if (cmdline.HasSwitch(switches::kImportFromFile)) { - // Silently import preset bookmarks from file. - // This is an OEM scenario. - ImportFromFile(profile, cmdline); - } - // ImportBookmarkFromFileIfNeeded() will go away as part of - // http://crbug.com/219419, so it is fine to hardcode |true| for now. - return true; -} - } // namespace internal } // namespace first_run @@ -498,21 +548,6 @@ void LogFirstRunMetric(FirstRunBubbleMetric metric) { NUM_FIRST_RUN_BUBBLE_METRICS); } -namespace { -CommandLine* GetExtraArgumentsInstance() { - CR_DEFINE_STATIC_LOCAL(CommandLine, arguments, (CommandLine::NoProgram())); - return &arguments; -} -} // namespace - -void SetExtraArgumentsForImportProcess(const CommandLine& arguments) { - GetExtraArgumentsInstance()->AppendArguments(arguments, false); -} - -const CommandLine& GetExtraArgumentsForImportProcess() { - return *GetExtraArgumentsInstance(); -} - // static void FirstRunBubbleLauncher::ShowFirstRunBubbleSoon() { SetShowFirstRunBubblePref(FIRST_RUN_BUBBLE_SHOW); @@ -634,9 +669,6 @@ ProcessMasterPreferencesResult ProcessMasterPreferences( internal::SetupMasterPrefsFromInstallPrefs(*install_prefs, out_prefs); - internal::SetImportPreferencesAndLaunchImport(out_prefs, - install_prefs.get()); - internal::SetDefaultBrowser(install_prefs.get()); } @@ -647,15 +679,15 @@ void AutoImport( Profile* profile, bool homepage_defined, int import_items, - int dont_import_items) { + int dont_import_items, + const std::string& import_bookmarks_path) { #if !defined(USE_AURA) // Deletes itself. ImporterHost* importer_host; // TODO(csilv,mirandac): Out-of-process import has only been qualified on - // MacOS X, so we will only use it on that platform since it is required. - // Remove this conditional logic once oop import is qualified for - // Linux/Windows. http://crbug.com/22142 -#if defined(OS_MACOSX) + // MacOS X and Windows, so we will only use it on those platforms. + // Linux still uses the in-process import (http://crbug.com/56816). +#if defined(OS_MACOSX) || defined(OS_WIN) importer_host = new ExternalProcessImporterHost; #else importer_host = new ImporterHost; @@ -721,15 +753,28 @@ void AutoImport( importer::LogImporterUseToMetrics( "AutoImport", importer_list->GetSourceProfileAt(0).importer_type); - profile_import_exited_successfully = - internal::ImportSettings(profile, importer_host, importer_list, items); - DCHECK(profile_import_exited_successfully); + ImportSettings(profile, importer_host, importer_list, items); + } + + if (!import_bookmarks_path.empty()) { + // Deletes itself. + ImporterHost* file_importer_host; + // TODO(gab): Make Linux use OOP import as well (http://crbug.com/56816) and + // get rid of these ugly ifdefs. +#if defined(OS_MACOSX) || defined(OS_WIN) + file_importer_host = new ExternalProcessImporterHost; +#else + file_importer_host = new ImporterHost; +#endif + file_importer_host->set_headless(); + + ImportFromFile(profile, file_importer_host, import_bookmarks_path); } content::RecordAction(UserMetricsAction("FirstRunDef_Accept")); #endif // !defined(USE_AURA) - did_perform_profile_import = true; + g_auto_import_state |= AUTO_IMPORT_CALLED; } void DoPostImportTasks(Profile* profile, bool make_chrome_default) { @@ -750,10 +795,8 @@ void DoPostImportTasks(Profile* profile, bool make_chrome_default) { internal::DoPostImportPlatformSpecificTasks(profile); } -bool DidPerformProfileImport(bool* exited_successfully) { - if (exited_successfully) - *exited_successfully = profile_import_exited_successfully; - return did_perform_profile_import; +uint16 auto_import_state() { + return g_auto_import_state; } } // namespace first_run diff --git a/chrome/browser/first_run/first_run.h b/chrome/browser/first_run/first_run.h index 24fa69c..91e5cda 100644 --- a/chrome/browser/first_run/first_run.h +++ b/chrome/browser/first_run/first_run.h @@ -39,6 +39,13 @@ class PrefRegistrySyncable; // install work for this user. After that the sentinel file is created. namespace first_run { +enum AutoImportState { + AUTO_IMPORT_NONE = 0, + AUTO_IMPORT_CALLED = 1 << 0, + AUTO_IMPORT_PROFILE_IMPORTED = 1 << 1, + AUTO_IMPORT_BOOKMARKS_FILE_IMPORTED = 1 << 2, +}; + enum FirstRunBubbleMetric { FIRST_RUN_BUBBLE_SHOWN = 0, // The search engine bubble was shown. FIRST_RUN_BUBBLE_CHANGE_INVOKED, // The bubble's "Change" was invoked. @@ -77,6 +84,7 @@ struct MasterPrefs { bool suppress_first_run_default_browser_prompt; std::vector<GURL> new_tabs; std::vector<GURL> bookmarks; + std::string import_bookmarks_path; std::string variations_seed; std::string suppress_default_browser_prompt_for_version; }; @@ -130,35 +138,22 @@ bool ShouldDoPersonalDataManagerFirstRun(); // Log a metric for the "FirstRun.SearchEngineBubble" histogram. void LogFirstRunMetric(FirstRunBubbleMetric metric); -// Allow a test to specify additional arguments for the profile import process. -void SetExtraArgumentsForImportProcess(const CommandLine& arguments); - -// Get any extra arguments set with SetExtraArgumentsForImportProcess. -const CommandLine& GetExtraArgumentsForImportProcess(); - -// -- Platform-specific functions -- - // Automatically import history and home page (and search engine, if -// ShouldShowSearchEngineDialog is true). +// ShouldShowSearchEngineDialog is true). Also imports bookmarks from file if +// |import_bookmarks_path| is not empty. void AutoImport(Profile* profile, bool homepage_defined, int import_items, - int dont_import_items); + int dont_import_items, + const std::string& import_bookmarks_path); // Does remaining first run tasks for |profile| and makes Chrome default browser // if |make_chrome_default|. This can pop the first run consent dialog on linux. void DoPostImportTasks(Profile* profile, bool make_chrome_default); -// Whether a first-run import was triggered before the browser mainloop began. -// This is used in testing to verify import startup actions that occur before -// an observer can be registered in the test. -bool DidPerformProfileImport(bool* exited_successfully); - -// Imports bookmarks and/or browser items (depending on platform support) -// in this process. This function is paired with first_run::ImportSettings(). -// This function might or might not show a visible UI depending on the -// cmdline parameters. -int ImportNow(Profile* profile, const CommandLine& cmdline); +// Returns the current state of AutoImport as recorded in a bitfield formed from +// values in AutoImportState. +uint16 auto_import_state(); // Set a master preferences file path that overrides platform defaults. void SetMasterPrefsPathForTesting(const base::FilePath& master_prefs); diff --git a/chrome/browser/first_run/first_run_browsertest.cc b/chrome/browser/first_run/first_run_browsertest.cc index 07015a1..d2847a6 100644 --- a/chrome/browser/first_run/first_run_browsertest.cc +++ b/chrome/browser/first_run/first_run_browsertest.cc @@ -3,7 +3,9 @@ // found in the LICENSE file. #include "base/command_line.h" +#include "base/files/file_path.h" #include "base/prefs/pref_service.h" +#include "base/string_util.h" #include "base/utf_string_conversions.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/extensions/component_loader.h" @@ -64,60 +66,97 @@ IN_PROC_BROWSER_TEST_F(FirstRunBrowserTest, SetShouldShowWelcomePage) { #if !defined(OS_CHROMEOS) namespace { -class FirstRunIntegrationBrowserTest : public InProcessBrowserTest { + +// A generic test class to be subclassed by test classes testing specific +// master_preferences. All subclasses must call SetMasterPreferencesForTest() +// from their SetUp() method before deferring the remainder of Setup() to this +// class. +class FirstRunMasterPrefsBrowserTestBase : public InProcessBrowserTest { public: - FirstRunIntegrationBrowserTest() {} + FirstRunMasterPrefsBrowserTestBase() {} + protected: + virtual void SetUp() OVERRIDE { + // All users of this test class need to call SetMasterPreferencesForTest() + // before this class' SetUp() is invoked. + ASSERT_TRUE(text_.get()); + + ASSERT_TRUE(file_util::CreateTemporaryFile(&prefs_file_)); + EXPECT_TRUE(file_util::WriteFile(prefs_file_, text_->c_str(), + text_->size())); + first_run::SetMasterPrefsPathForTesting(prefs_file_); + + // This invokes BrowserMain, and does the import, so must be done last. + InProcessBrowserTest::SetUp(); + } + + virtual void TearDown() OVERRIDE { + EXPECT_TRUE(file_util::Delete(prefs_file_, false)); + InProcessBrowserTest::TearDown(); + } + virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE { InProcessBrowserTest::SetUpCommandLine(command_line); command_line->AppendSwitch(switches::kForceFirstRun); - EXPECT_FALSE(first_run::DidPerformProfileImport(NULL)); + EXPECT_EQ(first_run::AUTO_IMPORT_NONE, first_run::auto_import_state()); extensions::ComponentLoader::EnableBackgroundExtensionsForTesting(); + } - // The forked import process should run BrowserMain. - CommandLine import_arguments((CommandLine::NoProgram())); - import_arguments.AppendSwitch(content::kLaunchAsBrowser); - first_run::SetExtraArgumentsForImportProcess(import_arguments); + void SetMasterPreferencesForTest(const char text[]) { + text_.reset(new std::string(text)); } + private: - DISALLOW_COPY_AND_ASSIGN(FirstRunIntegrationBrowserTest); + base::FilePath prefs_file_; + scoped_ptr<std::string> text_; + + DISALLOW_COPY_AND_ASSIGN(FirstRunMasterPrefsBrowserTestBase); }; -class FirstRunMasterPrefsBrowserTest : public FirstRunIntegrationBrowserTest { +template<const char Text[]> +class FirstRunMasterPrefsBrowserTestT + : public FirstRunMasterPrefsBrowserTestBase { public: - FirstRunMasterPrefsBrowserTest() {} + FirstRunMasterPrefsBrowserTestT() {} protected: virtual void SetUp() OVERRIDE { - ASSERT_TRUE(file_util::CreateTemporaryFile(&prefs_file_)); - // TODO(tapted): Make this reusable. - const char text[] = - "{\n" - " \"distribution\": {\n" - " \"import_bookmarks\": false,\n" - " \"import_history\": false,\n" - " \"import_home_page\": false,\n" - " \"import_search_engine\": false\n" - " }\n" - "}\n"; - EXPECT_TRUE(file_util::WriteFile(prefs_file_, text, strlen(text))); - first_run::SetMasterPrefsPathForTesting(prefs_file_); - - // This invokes BrowserMain, and does the import, so must be done last. - FirstRunIntegrationBrowserTest::SetUp(); - } - - virtual void TearDown() OVERRIDE { - EXPECT_TRUE(file_util::Delete(prefs_file_, false)); - FirstRunIntegrationBrowserTest::TearDown(); + SetMasterPreferencesForTest(Text); + FirstRunMasterPrefsBrowserTestBase::SetUp(); } private: - base::FilePath prefs_file_; - - DISALLOW_COPY_AND_ASSIGN(FirstRunMasterPrefsBrowserTest); + DISALLOW_COPY_AND_ASSIGN(FirstRunMasterPrefsBrowserTestT); }; + +} // namespace + +// TODO(tapted): Investigate why this fails on Linux bots but does not +// reproduce locally. See http://crbug.com/178062 . +// TODO(tapted): Investigate why this fails on mac_asan flakily +// http://crbug.com/181499 . +#if defined(OS_LINUX) || (defined(OS_MACOSX) && defined(ADDRESS_SANITIZER)) +#define MAYBE_ImportDefault DISABLED_ImportDefault +#else +#define MAYBE_ImportDefault ImportDefault +#endif + +extern const char kImportDefault[] = + "{\n" + "}\n"; +typedef FirstRunMasterPrefsBrowserTestT<kImportDefault> + FirstRunMasterPrefsImportDefault; +IN_PROC_BROWSER_TEST_F(FirstRunMasterPrefsImportDefault, MAYBE_ImportDefault) { + int auto_import_state = first_run::auto_import_state(); + // Aura builds skip over the import process. +#if defined(USE_AURA) + EXPECT_EQ(first_run::AUTO_IMPORT_CALLED, auto_import_state); +#else + EXPECT_EQ(first_run::AUTO_IMPORT_CALLED | + first_run::AUTO_IMPORT_PROFILE_IMPORTED, + auto_import_state); +#endif } // TODO(tapted): Investigate why this fails on Linux bots but does not @@ -125,28 +164,52 @@ class FirstRunMasterPrefsBrowserTest : public FirstRunIntegrationBrowserTest { // TODO(tapted): Investigate why this fails on mac_asan flakily // http://crbug.com/181499 . #if defined(OS_LINUX) || (defined(OS_MACOSX) && defined(ADDRESS_SANITIZER)) -#define MAYBE_WaitForImport DISABLED_WaitForImport +#define MAYBE_ImportBookmarksFile DISABLED_ImportBookmarksFile #else -#define MAYBE_WaitForImport WaitForImport +#define MAYBE_ImportBookmarksFile ImportBookmarksFile #endif -IN_PROC_BROWSER_TEST_F(FirstRunIntegrationBrowserTest, MAYBE_WaitForImport) { - bool success = false; - EXPECT_TRUE(first_run::DidPerformProfileImport(&success)); +// The bookmarks file doesn't actually need to exist for this integration test +// to trigger the interaction being tested. +extern const char kImportBookmarksFile[] = + "{\n" + " \"distribution\": {\n" + " \"import_bookmarks_from_file\": \"/foo/doesntexists.wtv\"\n" + " }\n" + "}\n"; +typedef FirstRunMasterPrefsBrowserTestT<kImportBookmarksFile> + FirstRunMasterPrefsImportBookmarksFile; +IN_PROC_BROWSER_TEST_F(FirstRunMasterPrefsImportBookmarksFile, + MAYBE_ImportBookmarksFile) { + int auto_import_state = first_run::auto_import_state(); // Aura builds skip over the import process. #if defined(USE_AURA) - EXPECT_FALSE(success); + EXPECT_EQ(first_run::AUTO_IMPORT_CALLED, auto_import_state); #else - EXPECT_TRUE(success); + EXPECT_EQ(first_run::AUTO_IMPORT_CALLED | + first_run::AUTO_IMPORT_PROFILE_IMPORTED | + first_run::AUTO_IMPORT_BOOKMARKS_FILE_IMPORTED, + auto_import_state); #endif } // Test an import with all import options disabled. This is a regression test // for http://crbug.com/169984 where this would cause the import process to // stay running, and the NTP to be loaded with no apps. -IN_PROC_BROWSER_TEST_F(FirstRunMasterPrefsBrowserTest, +extern const char kImportNothing[] = + "{\n" + " \"distribution\": {\n" + " \"import_bookmarks\": false,\n" + " \"import_history\": false,\n" + " \"import_home_page\": false,\n" + " \"import_search_engine\": false\n" + " }\n" + "}\n"; +typedef FirstRunMasterPrefsBrowserTestT<kImportNothing> + FirstRunMasterPrefsImportNothing; +IN_PROC_BROWSER_TEST_F(FirstRunMasterPrefsImportNothing, ImportNothingAndShowNewTabPage) { - EXPECT_TRUE(first_run::DidPerformProfileImport(NULL)); + EXPECT_EQ(first_run::AUTO_IMPORT_CALLED, first_run::auto_import_state()); ui_test_utils::NavigateToURLWithDisposition( browser(), GURL(chrome::kChromeUINewTabURL), CURRENT_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); diff --git a/chrome/browser/first_run/first_run_internal.h b/chrome/browser/first_run/first_run_internal.h index 9ff23b8..ae6f152 100644 --- a/chrome/browser/first_run/first_run_internal.h +++ b/chrome/browser/first_run/first_run_internal.h @@ -11,8 +11,6 @@ #include "base/compiler_specific.h" #include "base/gtest_prod_util.h" #include "base/memory/ref_counted.h" -#include "base/message_loop.h" -#include "chrome/browser/importer/importer_progress_observer.h" #include "ui/gfx/native_widget_types.h" class CommandLine; @@ -44,41 +42,6 @@ enum FirstRunState { // This variable should only be accessed through IsChromeFirstRun(). extern FirstRunState first_run_; -// This class acts as an observer for the ImporterProgressObserver::ImportEnded -// callback. When the import process is started, certain errors may cause -// ImportEnded() to be called synchronously, but the typical case is that -// ImportEnded() is called asynchronously. Thus we have to handle both cases. -// TODO(gab): Move this to the unnamed namespace of first_run.cc as part of the -// refactoring for OOP import (http://crbug.com/219419). -class ImportEndedObserver : public importer::ImporterProgressObserver { - public: - ImportEndedObserver() : ended_(false), - should_quit_message_loop_(false) {} - virtual ~ImportEndedObserver() {} - - // importer::ImporterProgressObserver: - virtual void ImportStarted() OVERRIDE {} - virtual void ImportItemStarted(importer::ImportItem item) OVERRIDE {} - virtual void ImportItemEnded(importer::ImportItem item) OVERRIDE {} - virtual void ImportEnded() OVERRIDE; - - void set_should_quit_message_loop() { - should_quit_message_loop_ = true; - } - - bool ended() const { - return ended_; - } - - private: - // Set if the import has ended. - bool ended_; - - // Set by the client (via set_should_quit_message_loop) if, when the import - // ends, this class should quit the message loop. - bool should_quit_message_loop_; -}; - // Loads master preferences from the master preference file into the installer // master preferences. Passes the master preference file path out in // master_prefs_path. Returns the pointer to installer::MasterPreferences object @@ -111,28 +74,6 @@ bool GetFirstRunSentinelFilePath(base::FilePath* path); // a linux specific implementation. bool IsOrganicFirstRun(); -// Imports settings. This may be done in a separate process depending on the -// platform, but it will always block until done. The return value indicates -// success. -// This functions has a common implementation for OS_POSIX, and a -// windows specific implementation. -bool ImportSettings(Profile* profile, - ImporterHost* importer_host, - scoped_refptr<ImporterList> importer_list, - int items_to_import); - -// Sets import preferences and launch the import process. -void SetImportPreferencesAndLaunchImport( - MasterPrefs* out_prefs, - installer::MasterPreferences* install_prefs); - -int ImportBookmarkFromFileIfNeeded(Profile* profile, - const CommandLine& cmdline); - -#if !defined(OS_WIN) -bool ImportBookmarks(const base::FilePath& import_bookmarks_path); -#endif - // Shows the EULA dialog if required. Returns true if the EULA is accepted, // returns false if the EULA has not been accepted, in which case the browser // should exit. diff --git a/chrome/browser/first_run/first_run_linux.cc b/chrome/browser/first_run/first_run_linux.cc index cfef727..2bd99e9 100644 --- a/chrome/browser/first_run/first_run_linux.cc +++ b/chrome/browser/first_run/first_run_linux.cc @@ -31,37 +31,6 @@ bool IsOrganicFirstRun() { return true; } -// TODO(port): This is just a piece of the silent import functionality from -// ImportSettings for Windows. It would be nice to get the rest of it ported. -bool ImportBookmarks(const base::FilePath& import_bookmarks_path) { - const CommandLine& cmdline = *CommandLine::ForCurrentProcess(); - CommandLine import_cmd(cmdline.GetProgram()); - - // Propagate user data directory switch. - if (cmdline.HasSwitch(switches::kUserDataDir)) { - import_cmd.AppendSwitchPath(switches::kUserDataDir, - cmdline.GetSwitchValuePath(switches::kUserDataDir)); - } - // Since ImportSettings is called before the local state is stored on disk - // we pass the language as an argument. GetApplicationLocale checks the - // current command line as fallback. - import_cmd.AppendSwitchASCII(switches::kLang, - g_browser_process->GetApplicationLocale()); - - import_cmd.CommandLine::AppendSwitchPath(switches::kImportFromFile, - import_bookmarks_path); - - // The importer doesn't need to do any background networking tasks so disable - // them. - import_cmd.CommandLine::AppendSwitch(switches::kDisableBackgroundNetworking); - - // Time to launch the process that is going to do the import. We'll wait - // for the process to return. - base::LaunchOptions options; - options.wait = true; - return base::LaunchProcess(import_cmd, options, NULL); -} - base::FilePath MasterPrefsPath() { // The standard location of the master prefs is next to the chrome binary. base::FilePath master_prefs; diff --git a/chrome/browser/first_run/first_run_mac.mm b/chrome/browser/first_run/first_run_mac.mm index 543d3d5..f75bf5b 100644 --- a/chrome/browser/first_run/first_run_mac.mm +++ b/chrome/browser/first_run/first_run_mac.mm @@ -15,11 +15,6 @@ namespace first_run { namespace internal { -bool ImportBookmarks(const base::FilePath& import_bookmarks_path) { - // http://crbug.com/48880 - return false; -} - base::FilePath MasterPrefsPath() { return master_prefs::MasterPrefsPath(); } diff --git a/chrome/browser/first_run/first_run_posix.cc b/chrome/browser/first_run/first_run_posix.cc index e66a27e..7498a90e 100644 --- a/chrome/browser/first_run/first_run_posix.cc +++ b/chrome/browser/first_run/first_run_posix.cc @@ -6,15 +6,11 @@ #include "base/file_util.h" #include "base/files/file_path.h" -#include "base/message_loop.h" #include "base/path_service.h" #include "base/prefs/pref_service.h" -#include "base/utf_string_conversions.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/first_run/first_run_dialog.h" #include "chrome/browser/first_run/first_run_internal.h" -#include "chrome/browser/importer/importer_host.h" -#include "chrome/browser/importer/importer_list.h" #include "chrome/browser/profiles/profile_manager.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_paths.h" @@ -22,7 +18,6 @@ #include "chrome/common/startup_metric_utils.h" #include "chrome/installer/util/google_update_settings.h" #include "chrome/installer/util/master_preferences.h" -#include "chrome/installer/util/master_preferences_constants.h" namespace first_run { namespace internal { @@ -63,52 +58,6 @@ bool GetFirstRunSentinelFilePath(base::FilePath* path) { return true; } -bool ImportSettings(Profile* profile, - ImporterHost* importer_host, - scoped_refptr<ImporterList> importer_list, - int items_to_import) { - const importer::SourceProfile& source_profile = - importer_list->GetSourceProfileAt(0); - - // Ensure that importers aren't requested to import items that they do not - // support. If there is no overlap, skip. - items_to_import &= source_profile.services_supported; - if (items_to_import == 0) - return true; - - ImportEndedObserver observer; - importer_host->SetObserver(&observer); - importer_host->StartImportSettings(source_profile, - profile, - items_to_import, - new ProfileWriter(profile)); - // If the import process has not errored out, block on it. - if (!observer.ended()) { - observer.set_should_quit_message_loop(); - MessageLoop::current()->Run(); - } - - // Unfortunately there's no success/fail signal in ImporterHost. - return true; -} - -void SetImportPreferencesAndLaunchImport( - MasterPrefs* out_prefs, - installer::MasterPreferences* install_prefs) { - std::string import_bookmarks_path; - install_prefs->GetString( - installer::master_preferences::kDistroImportBookmarksFromFilePref, - &import_bookmarks_path); - if (!import_bookmarks_path.empty()) { - // There are bookmarks to import from a file. - base::FilePath path = base::FilePath::FromWStringHack(UTF8ToWide( - import_bookmarks_path)); - if (!ImportBookmarks(path)) { - LOG(WARNING) << "silent bookmark import failed"; - } - } -} - bool ShowPostInstallEULAIfNeeded(installer::MasterPreferences* install_prefs) { // The EULA is only handled on Windows. return true; @@ -116,14 +65,3 @@ bool ShowPostInstallEULAIfNeeded(installer::MasterPreferences* install_prefs) { } // namespace internal } // namespace first_run - -namespace first_run { - -// TODO(port): Import switches need to be ported to both Mac and Linux. Not all -// import switches here are implemented for Linux. None are implemented for Mac -// (as this function will not be called on Mac). -int ImportNow(Profile* profile, const CommandLine& cmdline) { - return internal::ImportBookmarkFromFileIfNeeded(profile, cmdline); -} - -} // namespace first_run diff --git a/chrome/browser/first_run/first_run_win.cc b/chrome/browser/first_run/first_run_win.cc index fd0849a..7aa0e5d 100644 --- a/chrome/browser/first_run/first_run_win.cc +++ b/chrome/browser/first_run/first_run_win.cc @@ -4,57 +4,32 @@ #include "chrome/browser/first_run/first_run.h" -#include <windows.h> #include <shellapi.h> -#include <shlobj.h> +#include "base/base_paths.h" #include "base/callback.h" -#include "base/environment.h" #include "base/file_util.h" #include "base/files/file_path.h" #include "base/path_service.h" #include "base/prefs/pref_service.h" +#include "base/process.h" #include "base/process_util.h" -#include "base/stringprintf.h" -#include "base/strings/string_number_conversions.h" -#include "base/strings/string_split.h" #include "base/threading/sequenced_worker_pool.h" #include "base/time.h" -#include "base/utf_string_conversions.h" #include "base/win/metro.h" -#include "base/win/object_watcher.h" -#include "base/win/windows_version.h" -#include "chrome/browser/browser_process.h" #include "chrome/browser/first_run/first_run_internal.h" -#include "chrome/browser/importer/importer_host.h" -#include "chrome/browser/importer/importer_list.h" -#include "chrome/browser/process_singleton.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/shell_integration.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_paths.h" -#include "chrome/common/chrome_result_codes.h" #include "chrome/common/chrome_switches.h" -#include "chrome/common/pref_names.h" -#include "chrome/common/worker_thread_ticker.h" -#include "chrome/installer/util/browser_distribution.h" #include "chrome/installer/util/google_update_settings.h" #include "chrome/installer/util/install_util.h" #include "chrome/installer/util/master_preferences.h" #include "chrome/installer/util/master_preferences_constants.h" -#include "chrome/installer/util/shell_util.h" #include "chrome/installer/util/util_constants.h" #include "content/public/browser/browser_thread.h" -#include "content/public/browser/notification_service.h" -#include "content/public/browser/user_metrics.h" #include "google_update/google_update_idl.h" -#include "grit/chromium_strings.h" -#include "grit/generated_resources.h" #include "grit/locale_settings.h" -#include "grit/theme_resources.h" #include "ui/base/l10n/l10n_util.h" -#include "ui/base/layout.h" -#include "ui/base/ui_base_switches.h" #include "ui/base/win/shell.h" namespace { @@ -172,226 +147,6 @@ bool CreateEULASentinel() { file_util::WriteFile(eula_sentinel, "", 0) != -1); } -// This class is used by first_run::ImportSettings to determine when the import -// process has ended and what was the result of the operation as reported by -// the process exit code. This class executes in the context of the main chrome -// process. -class ImportProcessRunner : public base::win::ObjectWatcher::Delegate { - public: - // The constructor takes the importer process to watch and then it does a - // message loop blocking wait until the process ends. This object now owns - // the import_process handle. - explicit ImportProcessRunner(base::ProcessHandle import_process) - : import_process_(import_process), - exit_code_(content::RESULT_CODE_NORMAL_EXIT) { - watcher_.StartWatching(import_process, this); - MessageLoop::current()->Run(); - } - virtual ~ImportProcessRunner() { - ::CloseHandle(import_process_); - } - // Returns the child process exit code. There are 2 expected values: - // NORMAL_EXIT, or IMPORTER_HUNG. - int exit_code() const { return exit_code_; } - - // The child process has terminated. Find the exit code and quit the loop. - virtual void OnObjectSignaled(HANDLE object) OVERRIDE { - DCHECK(object == import_process_); - if (!::GetExitCodeProcess(import_process_, &exit_code_)) { - NOTREACHED(); - } - MessageLoop::current()->Quit(); - } - - private: - base::win::ObjectWatcher watcher_; - base::ProcessHandle import_process_; - DWORD exit_code_; -}; - -// Check every 3 seconds if the importer UI has hung. -const int kPollHangFrequency = 3000; - -// This class specializes on finding hung 'owned' windows. Unfortunately, the -// HungWindowDetector class cannot be used here because it assumes child -// windows and not owned top-level windows. -// This code is executed in the context of the main browser process and will -// terminate the importer process if it is hung. -class HungImporterMonitor : public WorkerThreadTicker::Callback { - public: - // The ctor takes the owner popup window and the process handle of the - // process to kill in case the popup or its owned active popup become - // unresponsive. - HungImporterMonitor(HWND owner_window, base::ProcessHandle import_process) - : owner_window_(owner_window), - import_process_(import_process), - ticker_(kPollHangFrequency) { - ticker_.RegisterTickHandler(this); - ticker_.Start(); - } - virtual ~HungImporterMonitor() { - ticker_.Stop(); - ticker_.UnregisterTickHandler(this); - } - - private: - virtual void OnTick() OVERRIDE { - if (!import_process_) - return; - // We find the top active popup that we own, this will be either the - // owner_window_ itself or the dialog window of the other process. In - // both cases it is worth hung testing because both windows share the - // same message queue and at some point the other window could be gone - // while the other process still not pumping messages. - HWND active_window = ::GetLastActivePopup(owner_window_); - if (::IsHungAppWindow(active_window) || ::IsHungAppWindow(owner_window_)) { - ::TerminateProcess(import_process_, chrome::RESULT_CODE_IMPORTER_HUNG); - import_process_ = NULL; - } - } - - HWND owner_window_; - base::ProcessHandle import_process_; - WorkerThreadTicker ticker_; - DISALLOW_COPY_AND_ASSIGN(HungImporterMonitor); -}; - -std::string EncodeImportParams(int importer_type, - int options, - bool skip_first_run_ui) { - return base::StringPrintf("%d@%d@%d", importer_type, options, - skip_first_run_ui ? 1 : 0); -} - -bool DecodeImportParams(const std::string& encoded, - int* importer_type, - int* options, - bool* skip_first_run_ui) { - std::vector<std::string> parts; - base::SplitString(encoded, '@', &parts); - int skip_first_run_ui_int; - if ((parts.size() != 3) || !base::StringToInt(parts[0], importer_type) || - !base::StringToInt(parts[1], options) || - !base::StringToInt(parts[2], &skip_first_run_ui_int)) - return false; - *skip_first_run_ui = !!skip_first_run_ui_int; - return true; -} - -#if !defined(USE_AURA) -// Imports browser items in this process. The browser and the items to -// import are encoded in the command line. -int ImportFromBrowser(Profile* profile, - const CommandLine& cmdline) { - std::string import_info = cmdline.GetSwitchValueASCII(switches::kImport); - if (import_info.empty()) { - NOTREACHED(); - return false; - } - int importer_type = 0; - int items_to_import = 0; - bool skip_first_run_ui = false; - if (!DecodeImportParams(import_info, &importer_type, &items_to_import, - &skip_first_run_ui)) { - NOTREACHED(); - return false; - } - - // Deletes itself. - ImporterHost* importer_host = new ImporterHost; - - scoped_refptr<ImporterList> importer_list(new ImporterList(NULL)); - importer_list->DetectSourceProfilesHack(); - - // If |skip_first_run_ui|, we run in headless mode. This means that if - // there is user action required the import is automatically canceled. - if (skip_first_run_ui) - importer_host->set_headless(); - - first_run::internal::ImportEndedObserver observer; - importer_host->SetObserver(&observer); - importer_host->StartImportSettings( - importer_list->GetSourceProfileForImporterType(importer_type), profile, - static_cast<uint16>(items_to_import), new ProfileWriter(profile)); - // If the import process has not errored out, block on it. - if (!observer.ended()) { - observer.set_should_quit_message_loop(); - MessageLoop::current()->Run(); - } - // TODO(gab): This method will be go away as part of http://crbug.com/219419/, - // so it is fine to hardcode |RESULT_CODE_NORMAL_EXIT| here for now. - return content::RESULT_CODE_NORMAL_EXIT; -} -#endif // !defined(USE_AURA) - -bool ImportSettingsWin(Profile* profile, - int importer_type, - int items_to_import, - const base::FilePath& import_bookmarks_path, - bool skip_first_run_ui) { - if (!items_to_import && import_bookmarks_path.empty()) { - return true; - } - - const CommandLine& cmdline = *CommandLine::ForCurrentProcess(); - base::FilePath chrome_exe(cmdline.GetProgram()); - // |chrome_exe| cannot be a relative path as chrome.exe already changed its - // CWD in LoadChromeWithDirectory(), making the relative path used on the - // command-line invalid. The base name is sufficient given chrome.exe is in - // the CWD. - if (!chrome_exe.IsAbsolute()) - chrome_exe = chrome_exe.BaseName(); - CommandLine import_cmd(chrome_exe); - - const char* kSwitchNames[] = { - switches::kUserDataDir, - switches::kChromeFrame, - switches::kCountry, - }; - import_cmd.CopySwitchesFrom(cmdline, kSwitchNames, arraysize(kSwitchNames)); - - // Allow tests to introduce additional switches. - import_cmd.AppendArguments(first_run::GetExtraArgumentsForImportProcess(), - false); - - // Since ImportSettings is called before the local state is stored on disk - // we pass the language as an argument. GetApplicationLocale checks the - // current command line as fallback. - import_cmd.AppendSwitchASCII(switches::kLang, - g_browser_process->GetApplicationLocale()); - - if (items_to_import) { - import_cmd.AppendSwitchASCII(switches::kImport, - EncodeImportParams(importer_type, items_to_import, skip_first_run_ui)); - } - - if (!import_bookmarks_path.empty()) { - import_cmd.AppendSwitchPath(switches::kImportFromFile, - import_bookmarks_path); - } - - // The importer doesn't need to do any background networking tasks so disable - // them. - import_cmd.CommandLine::AppendSwitch(switches::kDisableBackgroundNetworking); - - // Time to launch the process that is going to do the import. - base::ProcessHandle import_process; - if (!base::LaunchProcess(import_cmd, base::LaunchOptions(), &import_process)) - return false; - - // We block inside the import_runner ctor, pumping messages until the - // importer process ends. This can happen either by completing the import - // or by hang_monitor killing it. - ImportProcessRunner import_runner(import_process); - - // Import process finished. Reload the prefs, because importer may set - // the pref value. - if (profile) - profile->GetPrefs()->ReloadPersistentPrefs(); - - return (import_runner.exit_code() == content::RESULT_CODE_NORMAL_EXIT); -} - } // namespace namespace first_run { @@ -415,59 +170,10 @@ void DoPostImportPlatformSpecificTasks(Profile* /* profile */) { } } -bool ImportSettings(Profile* profile, - ImporterHost* importer_host, - scoped_refptr<ImporterList> importer_list, - int items_to_import) { - return ImportSettingsWin( - profile, - importer_list->GetSourceProfileAt(0).importer_type, - items_to_import, - base::FilePath(), - false); -} - bool GetFirstRunSentinelFilePath(base::FilePath* path) { return GetSentinelFilePath(chrome::kFirstRunSentinel, path); } -void SetImportPreferencesAndLaunchImport( - MasterPrefs* out_prefs, - installer::MasterPreferences* install_prefs) { - std::string import_bookmarks_path; - install_prefs->GetString( - installer::master_preferences::kDistroImportBookmarksFromFilePref, - &import_bookmarks_path); - - if (!IsOrganicFirstRun()) { - // If search engines aren't explicitly imported, don't import. - if (!(out_prefs->do_import_items & importer::SEARCH_ENGINES)) { - out_prefs->dont_import_items |= importer::SEARCH_ENGINES; - } - // If home page isn't explicitly imported, don't import. - if (!(out_prefs->do_import_items & importer::HOME_PAGE)) { - out_prefs->dont_import_items |= importer::HOME_PAGE; - } - // If history isn't explicitly forbidden, do import. - if (!(out_prefs->dont_import_items & importer::HISTORY)) { - out_prefs->do_import_items |= importer::HISTORY; - } - } - - if (out_prefs->do_import_items || !import_bookmarks_path.empty()) { - // There is something to import from the default browser. This launches - // the importer process and blocks until done or until it fails. - scoped_refptr<ImporterList> importer_list(new ImporterList(NULL)); - importer_list->DetectSourceProfilesHack(); - if (!ImportSettingsWin( - NULL, importer_list->GetSourceProfileAt(0).importer_type, - out_prefs->do_import_items, base::FilePath::FromWStringHack(UTF8ToWide( - import_bookmarks_path)), true)) { - LOG(WARNING) << "silent import failed"; - } - } -} - bool ShowPostInstallEULAIfNeeded(installer::MasterPreferences* install_prefs) { if (IsEULANotAccepted(install_prefs)) { // Show the post-installation EULA. This is done by setup.exe and the @@ -509,17 +215,3 @@ base::FilePath MasterPrefsPath() { } // namespace internal } // namespace first_run - -namespace first_run { - -int ImportNow(Profile* profile, const CommandLine& cmdline) { - int return_code = internal::ImportBookmarkFromFileIfNeeded(profile, cmdline); -#if !defined(USE_AURA) - if (cmdline.HasSwitch(switches::kImport)) { - return_code = ImportFromBrowser(profile, cmdline); - } -#endif - return return_code; -} - -} // namespace first_run diff --git a/chrome/browser/importer/external_process_importer_bridge.cc b/chrome/browser/importer/external_process_importer_bridge.cc index a2ac59b..6271210c 100644 --- a/chrome/browser/importer/external_process_importer_bridge.cc +++ b/chrome/browser/importer/external_process_importer_bridge.cc @@ -21,12 +21,14 @@ #endif namespace { + // Rather than sending all import items over IPC at once we chunk them into // separate requests. This avoids the case of a large import causing // oversized IPC messages. const int kNumBookmarksToSend = 100; const int kNumHistoryRowsToSend = 100; const int kNumFaviconsToSend = 100; + } ExternalProcessImporterBridge::ExternalProcessImporterBridge( @@ -46,18 +48,23 @@ void ExternalProcessImporterBridge::AddBookmarks( Send(new ProfileImportProcessHostMsg_NotifyBookmarksImportStart( first_folder_name, bookmarks.size())); - std::vector<ImportedBookmarkEntry>::const_iterator it; - for (it = bookmarks.begin(); it < bookmarks.end(); - it = it + kNumBookmarksToSend) { + // |bookmarks_left| is required for the checks below as Windows has a + // Debug bounds-check which prevents pushing an iterator beyond its end() + // (i.e., |it + 2 < s.end()| crashes in debug mode if |i + 1 == s.end()|). + int bookmarks_left = bookmarks.end() - bookmarks.begin(); + for (std::vector<ImportedBookmarkEntry>::const_iterator it = + bookmarks.begin(); it < bookmarks.end();) { std::vector<ImportedBookmarkEntry> bookmark_group; std::vector<ImportedBookmarkEntry>::const_iterator end_group = - it + kNumBookmarksToSend < bookmarks.end() ? - it + kNumBookmarksToSend : bookmarks.end(); + it + std::min(bookmarks_left, kNumBookmarksToSend); bookmark_group.assign(it, end_group); Send(new ProfileImportProcessHostMsg_NotifyBookmarksImportGroup( bookmark_group)); + bookmarks_left -= end_group - it; + it = end_group; } + DCHECK_EQ(0, bookmarks_left); } void ExternalProcessImporterBridge::AddHomePage(const GURL& home_page) { @@ -76,17 +83,23 @@ void ExternalProcessImporterBridge::SetFavicons( Send(new ProfileImportProcessHostMsg_NotifyFaviconsImportStart( favicons.size())); - std::vector<ImportedFaviconUsage>::const_iterator it; - for (it = favicons.begin(); it < favicons.end(); - it = it + kNumFaviconsToSend) { + // |favicons_left| is required for the checks below as Windows has a + // Debug bounds-check which prevents pushing an iterator beyond its end() + // (i.e., |it + 2 < s.end()| crashes in debug mode if |i + 1 == s.end()|). + int favicons_left = favicons.end() - favicons.begin(); + for (std::vector<ImportedFaviconUsage>::const_iterator it = + favicons.begin(); it < favicons.end();) { std::vector<ImportedFaviconUsage> favicons_group; std::vector<ImportedFaviconUsage>::const_iterator end_group = - std::min(it + kNumFaviconsToSend, favicons.end()); + it + std::min(favicons_left, kNumFaviconsToSend); favicons_group.assign(it, end_group); - Send(new ProfileImportProcessHostMsg_NotifyFaviconsImportGroup( - favicons_group)); + Send(new ProfileImportProcessHostMsg_NotifyFaviconsImportGroup( + favicons_group)); + favicons_left -= end_group - it; + it = end_group; } + DCHECK_EQ(0, favicons_left); } void ExternalProcessImporterBridge::SetHistoryItems( @@ -94,18 +107,22 @@ void ExternalProcessImporterBridge::SetHistoryItems( history::VisitSource visit_source) { Send(new ProfileImportProcessHostMsg_NotifyHistoryImportStart(rows.size())); - history::URLRows::const_iterator it; - for (it = rows.begin(); it < rows.end(); - it = it + kNumHistoryRowsToSend) { + // |rows_left| is required for the checks below as Windows has a + // Debug bounds-check which prevents pushing an iterator beyond its end() + // (i.e., |it + 2 < s.end()| crashes in debug mode if |i + 1 == s.end()|). + int rows_left = rows.end() - rows.begin(); + for (history::URLRows::const_iterator it = rows.begin(); it < rows.end();) { history::URLRows row_group; history::URLRows::const_iterator end_group = - it + kNumHistoryRowsToSend < rows.end() ? - it + kNumHistoryRowsToSend : rows.end(); + it + std::min(rows_left, kNumHistoryRowsToSend); row_group.assign(it, end_group); - Send(new ProfileImportProcessHostMsg_NotifyHistoryImportGroup(row_group, - visit_source)); + Send(new ProfileImportProcessHostMsg_NotifyHistoryImportGroup( + row_group, visit_source)); + rows_left -= end_group - it; + it = end_group; } + DCHECK_EQ(0, rows_left); } void ExternalProcessImporterBridge::SetKeywords( diff --git a/chrome/browser/importer/firefox_importer_browsertest.cc b/chrome/browser/importer/firefox_importer_browsertest.cc index 1a6dbec..fc87222 100644 --- a/chrome/browser/importer/firefox_importer_browsertest.cc +++ b/chrome/browser/importer/firefox_importer_browsertest.cc @@ -163,7 +163,6 @@ class Firefox3Observer : public ProfileWriter, virtual void AddBookmarks(const std::vector<ImportedBookmarkEntry>& bookmarks, const string16& top_level_folder_name) OVERRIDE { - ASSERT_LE(bookmark_count_ + bookmarks.size(), arraysize(kFirefox3Bookmarks)); // Importer should import the FF favorites the same as the list, in the same @@ -269,9 +268,9 @@ class FirefoxProfileImporterBrowserTest : public InProcessBrowserTest { items = items | importer::SEARCH_ENGINES; // Deletes itself. - // TODO(gab): Use ExternalProcessImporterHost on both Windows and Linux. + // TODO(gab): Use ExternalProcessImporterHost on Linux as well. ImporterHost* host; -#if defined(OS_MACOSX) +#if defined(OS_MACOSX) || defined(OS_WIN) host = new ExternalProcessImporterHost; #else host = new ImporterHost; diff --git a/chrome/browser/importer/ie_importer_browsertest_win.cc b/chrome/browser/importer/ie_importer_browsertest_win.cc index 5b51894..ac04365 100644 --- a/chrome/browser/importer/ie_importer_browsertest_win.cc +++ b/chrome/browser/importer/ie_importer_browsertest_win.cc @@ -30,6 +30,7 @@ #include "base/win/windows_version.h" #include "chrome/browser/bookmarks/imported_bookmark_entry.h" #include "chrome/browser/favicon/imported_favicon_usage.h" +#include "chrome/browser/importer/external_process_importer_host.h" #include "chrome/browser/importer/ie_importer.h" #include "chrome/browser/importer/ie_importer_utils_win.h" #include "chrome/browser/importer/ie_importer_test_registry_overrider_win.h" @@ -484,8 +485,7 @@ IN_PROC_BROWSER_TEST_F(IEImporterBrowserTest, IEImporter) { // Starts to import the above settings. // Deletes itself. - // TODO(gab): Use ExternalProcessImporterHost on Windows. - ImporterHost* host = new ImporterHost; + ImporterHost* host = new ExternalProcessImporterHost; TestObserver* observer = new TestObserver(); host->SetObserver(observer); @@ -561,8 +561,7 @@ IN_PROC_BROWSER_TEST_F(IEImporterBrowserTest, // Starts to import the above settings. // Deletes itself. - // TODO(gab): Use ExternalProcessImporterHost on Windows. - ImporterHost* host = new ImporterHost; + ImporterHost* host = new ExternalProcessImporterHost; MalformedFavoritesRegistryTestObserver* observer = new MalformedFavoritesRegistryTestObserver(); host->SetObserver(observer); diff --git a/chrome/browser/policy/cloud/user_policy_signin_service.cc b/chrome/browser/policy/cloud/user_policy_signin_service.cc index f93bf21..11400a7 100644 --- a/chrome/browser/policy/cloud/user_policy_signin_service.cc +++ b/chrome/browser/policy/cloud/user_policy_signin_service.cc @@ -226,11 +226,8 @@ UserPolicySigninService::UserPolicySigninService( Profile* profile) : profile_(profile), weak_factory_(this) { - CommandLine* cmd_line = CommandLine::ForCurrentProcess(); - if (profile_->GetPrefs()->GetBoolean(prefs::kDisableCloudPolicyOnSignin) || - ProfileManager::IsImportProcess(*cmd_line)) { + if (profile_->GetPrefs()->GetBoolean(prefs::kDisableCloudPolicyOnSignin)) return; - } // Initialize/shutdown the UserCloudPolicyManager when the user signs out. registrar_.Add(this, diff --git a/chrome/browser/profiles/profile.cc b/chrome/browser/profiles/profile.cc index be0bafd..9c3b7113 100644 --- a/chrome/browser/profiles/profile.cc +++ b/chrome/browser/profiles/profile.cc @@ -167,8 +167,7 @@ bool Profile::IsNewProfile() { // TODO(dconnelly): revisit this when crbug.com/22142 (unifying the profile // import code) is fixed. return GetOriginalProfile()->GetPrefs()->GetInitializationStatus() == - PrefService::INITIALIZATION_STATUS_CREATED_NEW_PREF_STORE || - first_run::DidPerformProfileImport(NULL); + PrefService::INITIALIZATION_STATUS_CREATED_NEW_PREF_STORE; } bool Profile::IsSyncAccessible() { diff --git a/chrome/browser/profiles/profile_manager.cc b/chrome/browser/profiles/profile_manager.cc index ca55479..ddf53e7 100644 --- a/chrome/browser/profiles/profile_manager.cc +++ b/chrome/browser/profiles/profile_manager.cc @@ -713,12 +713,6 @@ void ProfileManager::Observe( } } -// static -bool ProfileManager::IsImportProcess(const CommandLine& command_line) { - return (command_line.HasSwitch(switches::kImport) || - command_line.HasSwitch(switches::kImportFromFile)); -} - void ProfileManager::SetWillImport() { will_import_ = true; } @@ -794,16 +788,13 @@ void ProfileManager::DoFinalInit(Profile* profile, bool go_off_the_record) { void ProfileManager::DoFinalInitForServices(Profile* profile, bool go_off_the_record) { #if defined(ENABLE_EXTENSIONS) - const CommandLine& command_line = *CommandLine::ForCurrentProcess(); - if (!IsImportProcess(command_line)) { - extensions::ExtensionSystem::Get(profile)->InitForRegularProfile( - !go_off_the_record); - // During tests, when |profile| is an instance of TestingProfile, - // ExtensionSystem might not create an ExtensionService. - if (extensions::ExtensionSystem::Get(profile)->extension_service()) { - profile->GetHostContentSettingsMap()->RegisterExtensionService( - extensions::ExtensionSystem::Get(profile)->extension_service()); - } + extensions::ExtensionSystem::Get(profile)->InitForRegularProfile( + !go_off_the_record); + // During tests, when |profile| is an instance of TestingProfile, + // ExtensionSystem might not create an ExtensionService. + if (extensions::ExtensionSystem::Get(profile)->extension_service()) { + profile->GetHostContentSettingsMap()->RegisterExtensionService( + extensions::ExtensionSystem::Get(profile)->extension_service()); } #endif #if defined(ENABLE_MANAGED_USERS) diff --git a/chrome/browser/profiles/profile_manager.h b/chrome/browser/profiles/profile_manager.h index 05babfa..5b55baa 100644 --- a/chrome/browser/profiles/profile_manager.h +++ b/chrome/browser/profiles/profile_manager.h @@ -127,10 +127,6 @@ class ProfileManager : public base::NonThreadSafe, const content::NotificationSource& source, const content::NotificationDetails& details) OVERRIDE; - // Returns true if the given command line indicates that this is a short-lived - // profile import process. - static bool IsImportProcess(const CommandLine& command_line); - // Indicate that an import process will run for the next created Profile. void SetWillImport(); bool will_import() { return will_import_; } diff --git a/chrome/browser/sync/profile_sync_service_factory.cc b/chrome/browser/sync/profile_sync_service_factory.cc index 9a5718a..f43c988 100644 --- a/chrome/browser/sync/profile_sync_service_factory.cc +++ b/chrome/browser/sync/profile_sync_service_factory.cc @@ -39,10 +39,6 @@ ProfileSyncService* ProfileSyncServiceFactory::GetForProfile( if (!ProfileSyncService::IsSyncEnabled()) return NULL; - // Do not start sync on the import process. - if (ProfileManager::IsImportProcess(*CommandLine::ForCurrentProcess())) - return NULL; - return static_cast<ProfileSyncService*>( GetInstance()->GetServiceForBrowserContext(profile, true)); } diff --git a/chrome/browser/ui/sync/profile_signin_confirmation_helper_browsertest.cc b/chrome/browser/ui/sync/profile_signin_confirmation_helper_browsertest.cc index f00a907..c250511 100644 --- a/chrome/browser/ui/sync/profile_signin_confirmation_helper_browsertest.cc +++ b/chrome/browser/ui/sync/profile_signin_confirmation_helper_browsertest.cc @@ -22,11 +22,6 @@ class ProfileSigninConfirmationHelperBrowserTest : public InProcessBrowserTest { // Force the first-run flow to trigger autoimport. InProcessBrowserTest::SetUpCommandLine(command_line); command_line->AppendSwitch(switches::kForceFirstRun); - - // The forked import process should run BrowserMain. - CommandLine import_arguments((CommandLine::NoProgram())); - import_arguments.AppendSwitch(content::kLaunchAsBrowser); - first_run::SetExtraArgumentsForImportProcess(import_arguments); } private: @@ -36,7 +31,7 @@ class ProfileSigninConfirmationHelperBrowserTest : public InProcessBrowserTest { IN_PROC_BROWSER_TEST_F(ProfileSigninConfirmationHelperBrowserTest, HasNotBeenShutdown) { #if !defined(OS_CHROMEOS) - EXPECT_TRUE(first_run::DidPerformProfileImport(NULL)); + EXPECT_TRUE(first_run::auto_import_state() & first_run::AUTO_IMPORT_CALLED); #endif EXPECT_FALSE(ui::HasBeenShutdown(browser()->profile())); } diff --git a/chrome/browser/ui/webui/options/import_data_handler.cc b/chrome/browser/ui/webui/options/import_data_handler.cc index 5b2d449..eefe4fe 100644 --- a/chrome/browser/ui/webui/options/import_data_handler.cc +++ b/chrome/browser/ui/webui/options/import_data_handler.cc @@ -109,12 +109,16 @@ void ImportDataHandler::ImportData(const ListValue* args) { state); import_did_succeed_ = false; - // TODO(csilv): Out-of-process import has only been qualified on MacOS X, - // so we will only use it on that platform since it is required. Remove this - // conditional logic once oop import is qualified for Linux/Windows. - // http://crbug.com/22142 -#if defined(OS_MACOSX) - importer_host_ = new ExternalProcessImporterHost; + // TODO(gab): Make Linux use OOP import as well (http://crbug.com/56816) and + // get rid of these ugly ifdefs. +#if defined(OS_MACOSX) || defined(OS_WIN) + // The Google Toolbar importer doesn't work for the out-of-process import. + // This is the only entry point for this importer (it is never used on first + // run). See discussion on http://crbug.com/219419 for details. + if (source_profile.importer_type == importer::TYPE_GOOGLE_TOOLBAR5) + importer_host_ = new ImporterHost; + else + importer_host_ = new ExternalProcessImporterHost; #else importer_host_ = new ImporterHost; #endif |