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.
+    const bool no_first_run = base::CommandLine::ForCurrentProcess()->HasSwitch(
+        switches::kNoFirstRun);

[Greg Levin, 2016/11/22 00:13:40]

I'm not sure about all the use cases of this flag, and this may cover all cases
of interest as is, but did you look at DetermineFirstRunState() :

https://cs.chromium.org/chromium/src/chrome/browser/first_run/first_run.cc?l=652

There, --force-first-run supersedes --no-first-run.  I haven't had a chance to
play with the FirstRunSentinel (for example, I don't know what it does on
off-the-record sessions), so I'm not sure if it (and thus
DetermineFirstRunState()) is more appropriate here.

[achuithb, 2016/11/22 00:38:47]

Done.

[Greg Levin, 2016/11/22 16:57:29]

Sorry, didn't mean to imply that you *should* switch to IsChromeFirstRun(), only
that you should investigate the option.  I tested it this morning, and it
doesn't seem to be the right choice for Goodies (at least on Linux desktop debug
version).  The "First Run" flag file gets written even before the OOBE screen is
shown.  If I shut down and restart before doing a proper login (i.e. during
OOBE, or after only doing a guest login), the flag file exists, the system
doesn't think it's a first run, and no Goodies are ever shown.  The rest of the
Goodies code tries to ensure that they will be shown even after such incomplete
startups, so IsChromeFirstRun()'s dependence on the flag file seems like the
wrong choice after all.  I think a better solution would be something like

    const bool no_first_run =
        !base::CommandLine::ForCurrentProcess()->HasSwitch(
            switches::kForceFirstRun) &&
        base::CommandLine::ForCurrentProcess()->HasSwitch(
            switches::kNoFirstRun);

This is consistent with the flag logic in DetermineFirstRunState(), but doesn't
depend on the First Run flag file.

[achuithb, 2016/11/22 21:37:43]

Done.

+
+    if (!enterprise_managed && !no_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