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

chrome/browser/chromeos/first_run/goodies_displayer.cc

Index: chrome/browser/chromeos/first_run/goodies_displayer.cc
diff --git a/chrome/browser/chromeos/first_run/goodies_displayer.cc b/chrome/browser/chromeos/first_run/goodies_displayer.cc
index 94c1d44eb35305189fcc24068bdf380b18fd3be3..3b3aca6593e14af7a200eb925673b1bc6f033dbf 100644
--- a/chrome/browser/chromeos/first_run/goodies_displayer.cc
+++ b/chrome/browser/chromeos/first_run/goodies_displayer.cc
@@ -4,6 +4,7 @@
 
 #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"
@@ -14,6 +15,7 @@
 #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"
@@ -116,9 +118,14 @@ void GoodiesDisplayer::OnBrowserSetLastActive(Browser* browser) {
   // 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.