Disable AutoImport: experiment and histogram for analysis.

chrome/browser/chrome_browser_main.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chrome_browser_main.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 15 matching lines @@
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/sys_info.h"
 #include "base/task_scheduler/scheduler_worker_pool_params.h"
 #include "base/task_scheduler/switches.h"
 #include "base/task_scheduler/task_scheduler.h"
 #include "base/task_scheduler/task_traits.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/default_tick_clock.h"
 #include "base/time/time.h"
@@ skipping 152 matching lines @@
 #include "chromeos/settings/cros_settings_names.h"
 #endif  // defined(OS_CHROMEOS)
 
 // TODO(port): several win-only methods have been pulled out of this, but
 // BrowserMain() as a whole needs to be broken apart so that it's usable by
 // other platforms. For now, it's just a stub. This is a serious work in
 // progress and should not be taken as an indication of a real refactoring.
 
 #if defined(OS_WIN)
 #include "base/trace_event/trace_event_etw_export_win.h"
+#include "base/win/win_util.h"
 #include "base/win/windows_version.h"
 #include "chrome/app/file_pre_reader_win.h"
 #include "chrome/browser/chrome_browser_main_win.h"
 #include "chrome/browser/component_updater/caps_installer_win.h"
 #include "chrome/browser/component_updater/sw_reporter_installer_win.h"
 #include "chrome/browser/downgrade/user_data_downgrade.h"
 #include "chrome/browser/first_run/try_chrome_dialog_view.h"
 #include "chrome/browser/first_run/upgrade_util_win.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/ui/network_profile_bubble.h"
@@ skipping 65 matching lines @@
 
 #if defined(OS_WIN) || defined(OS_MACOSX) || \
     (defined(OS_LINUX) && !defined(OS_CHROMEOS))
 #include "chrome/browser/metrics/desktop_engagement/desktop_engagement_service.h"
 #endif
 
 using content::BrowserThread;
 
 namespace {
 
+// feature used to remove AutoImport on first run is default-off
+const base::Feature kDisableFirstRunAutoImport{
+    "DisableFirstRunAutoImport", base::FEATURE_DISABLED_BY_DEFAULT};

[Ilya Sherman, 2016/08/22 21:10:45]

Please define this feature in
https://cs.chromium.org/chromium/src/chrome/common/chrome_features.h

[gcomanici, 2016/08/23 16:31:59]

Done.

+
 // A provider of Geolocation services to override AccessTokenStore.
 class ChromeGeolocationDelegate : public device::GeolocationDelegate {
  public:
   ChromeGeolocationDelegate() = default;
 
   scoped_refptr<device::AccessTokenStore> CreateAccessTokenStore() final {
     return new ChromeAccessTokenStore();
   }
 
  private:
@@ skipping 1488 matching lines @@
   // (requires supporting early exit).
   PostProfileInit();
 
 #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
   // Show the First Run UI if this is the first time Chrome has been run on
   // this computer, or we're being compelled to do so by a command line flag.
   // Note that this be done _after_ the PrefService is initialized and all
   // preferences are registered, since some of the code that the importer
   // touches reads preferences.
   if (first_run::IsChromeFirstRun()) {
-    first_run::AutoImport(profile_,
-                          master_prefs_->homepage_defined,
-                          master_prefs_->do_import_items,
-                          master_prefs_->dont_import_items,
-                          master_prefs_->import_bookmarks_path);
+    // By default, autoImport is performed on first run.

[Ilya Sherman, 2016/08/22 21:10:45]

nit: Please try to be consist about how autoImport vs. "AutoImport" vs.
"auto-import" (vs. any other variants) is spelled.

+    bool perform_autoImport_on_first_run = true;

[Ilya Sherman, 2016/08/22 21:10:45]

nit: s/autoImport/auto_import

[Ilya Sherman, 2016/08/22 21:10:45]

nit: I'd drop the "_on_first_run" suffix, since this code is already within the
body of an IsChromeFirstRun() if-stmt.

[gcomanici, 2016/08/23 16:31:59]

Done.

+  // Part of a field trial that is designed to turn off the AutoImport feature
+  // for Windows users (on first run).
+  // The feature keeps running for enterprise.
+  #if defined(OS_WIN)

[Ilya Sherman, 2016/08/22 21:10:45]

Please control the platform via the field trial's server-side config, rather
than via a #define, unless you have a really compelling reason to use a #define.
 If you do, please document it here =)

[gcomanici, 2016/08/23 16:31:59]

Done.

+  base::FieldTrial* trial =
+          base::FieldTrialList::Find("DisableFirstRunAutoImport");

[Ilya Sherman, 2016/08/22 21:10:45]

There's no need to call this when using the Feature API.

[gcomanici, 2016/08/23 16:31:59]

Acknowledged.

+  perform_autoImport_on_first_run = !trial ||
+        !base::FeatureList::IsEnabled(kDisableFirstRunAutoImport) ||
+        base::win::IsEnrolledToDomain();

[Ilya Sherman, 2016/08/22 21:10:45]

Please make the IsEnrolledToDomain() check first.  That way, users who are in
enterprise domains won't be considered as "active" in the field trial, which
will reduce noise when viewing the metrics.

[gcomanici, 2016/08/23 16:31:59]

Done.

+  #endif

[Ilya Sherman, 2016/08/22 21:10:45]

The indentation looks off for these lines.  Please run git cl format over this
CL -- it's a handy tool =)  (Apologies if you've already run it, and I'm
misunderstanding something.)

[gcomanici, 2016/08/23 16:31:59]

Acknowledged.

+    if (perform_autoImport_on_first_run) {
+      first_run::AutoImport(profile_, master_prefs_->homepage_defined,
+                            master_prefs_->do_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_for_user);
 
     if (!master_prefs_->suppress_first_run_default_browser_prompt) {
       browser_creator_->set_show_main_browser_window(
           !chrome::ShowFirstRunDefaultBrowserPrompt(profile_));
     } else {
       browser_creator_->set_is_default_browser_dialog_suppressed(true);
@@ skipping 393 matching lines @@
   chromeos::CrosSettings::Shutdown();
 #endif  // defined(OS_CHROMEOS)
 #endif  // defined(OS_ANDROID)
 }
 
 // Public members:
 
 void ChromeBrowserMainParts::AddParts(ChromeBrowserMainExtraParts* parts) {
   chrome_extra_parts_.push_back(parts);
 }