Do not display goodies with --no-first-run.

chrome/browser/chromeos/first_run/goodies_displayer.cc

 // Copyright 2015 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/chromeos/first_run/goodies_displayer.h"
 
+#include "base/command_line.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/login/startup_utils.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace chromeos {
 namespace first_run {
 
 namespace {
 
 GoodiesDisplayer* g_goodies_displayer = nullptr;
@@ skipping 82 matching lines @@
     return;
 
   // 2. Not guest or incognito session (keep observing).
   if (browser->profile()->IsOffTheRecord())
     return;
 
   PrefService* local_state = g_browser_process->local_state();
   // 3. Not previously shown, or otherwise marked as unavailable.
   if (local_state->GetBoolean(prefs::kCanShowOobeGoodiesPage)) {
     // 4. Device not enterprise enrolled.
-    if (!g_browser_process->platform_part()
-             ->browser_policy_connector_chromeos()
-             ->IsEnterpriseManaged())
+    const bool enterprise_managed = g_browser_process->platform_part()
+                                        ->browser_policy_connector_chromeos()
+                                        ->IsEnterpriseManaged();
+    // 5. --no-first-run not specified, as it is for tests. --force-run takes
+    // precedence over --no-first-run.
+    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+    const bool is_first_run =

[Greg Levin, 2016/11/22 21:49:21]

Looks good.  One last optional nit, totally at your discretion:
"is_first_run" is misleading, as that's not really what this variable means at
all.  Maybe something like "switches_allow_first_run" or "first_run_permitted"?

[achuithb, 2016/11/22 22:08:25]

I've renamed this. This variable exists purely so the if stmt below is readable,
so I don't think we lose much by being sloppy with the meaning.

+        command_line->HasSwitch(switches::kForceFirstRun) ||
+        !command_line->HasSwitch(switches::kNoFirstRun);
+
+    if (!enterprise_managed && is_first_run)
       chrome::AddTabAt(browser, GURL(kGoodiesURL), 2, false);
 
     // Set to |false| whether enterprise enrolled or Goodies shown.
     local_state->SetBoolean(prefs::kCanShowOobeGoodiesPage, false);
   }
 
   // Regardless of how we got here, we don't henceforth need to show Goodies.
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::Bind(&Delete));
 }
 
 GoodiesDisplayerTestInfo::GoodiesDisplayerTestInfo()
     : days_since_oobe(0), setup_complete(false) {}
 
 GoodiesDisplayerTestInfo::~GoodiesDisplayerTestInfo() {}
 
 }  // namespace first_run
 }  // namespace chromeos