Show Goodies page to new Chromebook users

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/lazy_instance.h"
+#include "base/task_runner_util.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/prefs/pref_service_syncable.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_tabstrip.h"
+#include "chrome/common/pref_names.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace chromeos {
+namespace first_run {
+
+namespace {
+
+// ChromeOS Goodies page for device's first New Window.
+const char kGoodiesURL[] = "https://www.google.com/chrome/devices/goodies.html";
+
+// Max days after initial login that we're willing to show Goodies.
+static const int kMaxDaysAfterOobeForGoodies = 14;
+
+base::LazyInstance<GoodiesDisplayer> g_goodies_displayer =
+    LAZY_INSTANCE_INITIALIZER;
+
+// Checks timestamp on OOBE Complete flag file.  kCanShowOobeGoodiesPage
+// defaults to |true|; we set it to |false| (return |false|) for any device over
+// kMaxDaysAfterOobeForGoodies days old, to avoid showing it after update on
+// older devices.
+bool CheckGoodiesPrefAgainstOobeTimestamp() {
+  const base::FilePath oobe_timestamp_file =
+      StartupUtils::GetOobeCompleteFlagPath();
+  base::File::Info fileInfo;
+  if (base::GetFileInfo(oobe_timestamp_file, &fileInfo)) {
+    const base::TimeDelta time_since_oobe =
+        base::Time::Now() - fileInfo.creation_time;
+    if (time_since_oobe >
+        base::TimeDelta::FromDays(kMaxDaysAfterOobeForGoodies))
+      return false;
+  }
+  return true;
+}
+
+// Callback into main thread to set pref to |false| if too long since oobe, or
+// to create GoodiesDisplayer otherwise.
+void UpdateGoodiesPrefCantShow(bool can_show_goodies) {
+  if (can_show_goodies) {
+    g_goodies_displayer.Get();
+  } else {
+    g_browser_process->local_state()->SetBoolean(prefs::kCanShowOobeGoodiesPage,
+                                                 false);
+  }
+}
+
+}  // namespace
+
+GoodiesDisplayer::GoodiesDisplayer() {
+  BrowserList::AddObserver(this);
+}
+
+// If Goodies page hasn't been shown yet, and Chromebook isn't too old, create
+// GoodiesDisplayer to observe BrowserList.
+void GoodiesDisplayer::Init() {
+  if (g_browser_process->local_state()->GetBoolean(
+          prefs::kCanShowOobeGoodiesPage))
+    base::PostTaskAndReplyWithResult(
+        content::BrowserThread::GetBlockingPool(), FROM_HERE,
+        base::Bind(&CheckGoodiesPrefAgainstOobeTimestamp),
+        base::Bind(&UpdateGoodiesPrefCantShow));
+}
+
+// If conditions enumerated below are met, this loads the Oobe Goodies page for
+// new Chromebooks; when appropriate, it uses pref to mark page as shown,
+// removes itself from BrowserListObservers, and deletes itself.

[achuithb, 2015/09/16 03:40:29]

Where does it delete itself?

Why not delete at the end of this?

[Greg Levin, 2015/09/16 16:55:13]

|> Where does it delete itself?

Currently, it doesn't; it will be deleted on program exit.  Certainly not ideal,
but as it's a memberless class that only gets created once in a device's
lifetime, not that harmful, either.

|> Why not delete at the end of this?

Wasn't sure how to delete a LazyInstance...
  g_goodies_displayer = LAZY_INSTANCE_INITIALIZER;
doesn't work, and
  delete this;
segfaults.  As I said in previous comments, LazyInstance may not be the best
choice here (still learning singletons in Chrome); it just seemed to be about
what I needed.

[achuithb, 2015/09/16 17:17:30]

delete on program exit is more commonly called a leak ;)
Yeah, I don't have experience with LAZY_INSTANCE_INITIALIZER either. I was
hoping you could simply delete it after it's single use.

[Greg Levin, 2015/09/16 20:48:23]

Done.

+void GoodiesDisplayer::OnBrowserSetLastActive(Browser* browser) {
+  // 1. Not guest or incognito session (keep observing).
+  if (browser->profile()->IsOffTheRecord())
+    return;
+
+  PrefService* local_state = g_browser_process->local_state();
+  // 2. Not previously shown, or otherwise marked as unavailable.
+  if (local_state->GetBoolean(prefs::kCanShowOobeGoodiesPage)) {
+    // 3. Device not enterprise enrolled.
+    if (!g_browser_process->platform_part()
+             ->browser_policy_connector_chromeos()
+             ->IsEnterpriseManaged())

[achuithb, 2015/09/16 03:40:29]

I believe you need {} when the if stmt is multi-line? Not sure

[Greg Levin, 2015/09/16 16:55:13]

All I found was this:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Conditionals
"In general, curly braces are not required for single-line statements, but they
are allowed if you like them; conditional or loop statements with complex
conditions or statements may be more readable with curly braces"

To me, adding the braces made it look more cluttered as opposed to more
readable.  But there may be other style conventions that apply that I'm not
aware of.  (And I could just add them if that's your preference :-)

[achuithb, 2015/09/16 17:17:30]

I believe there is no consensus on this, so let's leave it as is:
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/jPXg9Bg_kOg/33...

[Greg Levin, 2015/09/16 20:48:23]

Acknowledged.

+      chrome::AddTabAt(browser, GURL(kGoodiesURL), -1, 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.
+  BrowserList::RemoveObserver(this);
+}
+
+}  // namespace first_run
+}  // namespace chromeos