Add an outdated upgrade bubble view.

chrome/browser/upgrade_detector_impl.cc

Index: chrome/browser/upgrade_detector_impl.cc
diff --git a/chrome/browser/upgrade_detector_impl.cc b/chrome/browser/upgrade_detector_impl.cc
index 570275e391ba59c8452c3de6d6861f7f4ead8e04..03784b89fcbfce300ca53286dcad46b1ac2249f7 100644
--- a/chrome/browser/upgrade_detector_impl.cc
+++ b/chrome/browser/upgrade_detector_impl.cc
@@ -7,28 +7,33 @@
 #include <string>
 
 #include "base/bind.h"
+#include "base/build_time.h"
 #include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
+#include "base/metrics/field_trial.h"
 #include "base/path_service.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
+#include "base/version.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
-#include "chrome/installer/util/browser_distribution.h"
 #include "content/public/browser/browser_thread.h"
 #include "ui/base/resource/resource_bundle.h"
 
 #if defined(OS_WIN)
+#include "chrome/installer/util/browser_distribution.h"
+#include "chrome/installer/util/google_update_settings.h"
+#include "chrome/installer/util/helper.h"
 #include "chrome/installer/util/install_util.h"
 #elif defined(OS_MACOSX)
 #include "chrome/browser/mac/keystone_glue.h"
 #elif defined(OS_POSIX)
 #include "base/process_util.h"
-#include "base/version.h"
 #endif
 
 using content::BrowserThread;
@@ -46,6 +51,13 @@ const int kNotifyCycleTimeMs = 20 * 60 * 1000;  // 20 minutes.
 // Same as kNotifyCycleTimeMs but only used during testing.
 const int kNotifyCycleTimeForTestingMs = 500;  // Half a second.
 
+// The number of days after which we identify a build/install as outdated.
+const uint64 kOutdatedBuildAgeInDays = 12 * 7;
+
+// Finch Experiment strings to identify if we should check for outdated install.
+const char kOutdatedInstallCheckTrialName[] = "OutdatedInstallCheck";
+const char kOutdatedInstallCheck12WeeksGroupName[] = "12WeeksOutdatedIntalls";

[Finnur, 2013/02/05 10:52:19]

I assume you've filed this an experiment request with the Finch team? (they like
to keep track of these with a special form they had me submit when I was doing
mine)

[MAD, 2013/02/05 16:49:16]

Not yet, but I do know about this, I sit with the Finch team... :-) I even used
to be the TL when the team was founded... B-)

[Finnur, 2013/02/05 22:14:50]

Hah! :)

+
 std::string CmdLineInterval() {
   const CommandLine& cmd_line = *CommandLine::ForCurrentProcess();
   return cmd_line.GetSwitchValueASCII(switches::kCheckForUpdateIntervalSec);
@@ -54,7 +66,9 @@ std::string CmdLineInterval() {
 bool IsTesting() {
   const CommandLine& cmd_line = *CommandLine::ForCurrentProcess();
   return cmd_line.HasSwitch(switches::kSimulateUpgrade) ||
-      cmd_line.HasSwitch(switches::kCheckForUpdateIntervalSec);
+      cmd_line.HasSwitch(switches::kCheckForUpdateIntervalSec) ||
+      cmd_line.HasSwitch(switches::kSimulateCriticalUpdate) ||
+      cmd_line.HasSwitch(switches::kSimulateOutdated);
 }
 
 // How often to check for an upgrade.
@@ -68,12 +82,133 @@ int GetCheckForUpgradeEveryMs() {
   return kCheckForUpgradeMs;
 }
 
+bool IsUnstableChannel() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
+  return channel == chrome::VersionInfo::CHANNEL_DEV ||
+         channel == chrome::VersionInfo::CHANNEL_CANARY;
+}
+
+#if defined(OS_WIN)
+bool IsSystemInstall() {
+  FilePath exe_path;
+  if (!PathService::Get(base::DIR_EXE, &exe_path)) {
+    NOTREACHED() << "Failed to find executable path";
+    return false;
+  }
+
+  return !InstallUtil::IsPerUserInstall(exe_path.value().c_str());
+}
+
+// This task checks the update policy and calls back the task only if automatic
+// updates are allowed. It also identifies whether we are running an unstable
+// version.

[Finnur, 2013/02/04 21:30:17]

s/version/channel/

[MAD, 2013/02/05 16:49:16]

Done.

+void DetectUpdatability(const base::Closure& callback_task,
+                        bool* is_unstable_channel) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  *is_unstable_channel = IsUnstableChannel();
+
+  string16 app_guid = installer::GetAppGuidForUpdates(IsSystemInstall());
+  DCHECK(!app_guid.empty());
+  if (GoogleUpdateSettings::AUTOMATIC_UPDATES ==
+      GoogleUpdateSettings::GetAppUpdatePolicy(app_guid, NULL)) {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback_task);

[Finnur, 2013/02/05 10:52:19]

I think a direct call to ...

CheckForUnstableChannel(callback_task, is_unstable_channel);

... instead of this PostTask would consolidate the unstable check into one
location instead of two, which should make it less brittle and make it easier to
debug (because then you'll never accidentally set the breakpoint on line 109
when debugging on non-Windows platforms because you can then delete line 109). 

[MAD, 2013/02/05 16:49:16]

Done.

+  }
+}
+#endif  // defined(OS_WIN)
+
+// This task identifies whether we are running an unstable version. And then
+// it unconditionnally calls back the provided task.

[Finnur, 2013/02/04 21:30:17]

no double n in unconditionally.

[MAD, 2013/02/05 16:49:16]

Done.

+void CheckForUnstableChannel(const base::Closure& callback_task,
+                             bool* is_unstable_channel) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  *is_unstable_channel = IsUnstableChannel();
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback_task);
+}
+
+}  // namespace
+
+UpgradeDetectorImpl::UpgradeDetectorImpl()
+    : ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
+      is_unstable_channel_(false),
+      build_date_(base::GetBuildTime()) {
+  CommandLine command_line(*CommandLine::ForCurrentProcess());
+  // The different command line switches that affect testing can't be used
+  // simultaneously, if they do, here's the precedence order, based on the order
+  // of the if statements below:
+  // - kDisableBackgroundNetworking prevents any of the other command line
+  //   switch from being taken into account.
+  // - kSimulateUpgrade supersedes critical or outdated upgrade switches.
+  // - kSimulateCriticalUpdate has precedence over kSimulateOutdated.
+  // - kSimulateOutdated can work on its own, or with a specified date.
+  if (command_line.HasSwitch(switches::kDisableBackgroundNetworking))
+    return;
+  if (command_line.HasSwitch(switches::kSimulateUpgrade)) {
+    UpgradeDetected(UPGRADE_AVAILABLE_REGULAR);
+    return;
+  }
+  if (command_line.HasSwitch(switches::kSimulateCriticalUpdate)) {
+    UpgradeDetected(UPGRADE_AVAILABLE_CRITICAL);
+    return;
+  }
+  if (command_line.HasSwitch(switches::kSimulateOutdated)) {
+    // The outdated simulation can work without a value, which means outdated
+    // now, or with a value that must be a well formed date/time string that
+    // overrides the build date.
+    std::string build_date = command_line.GetSwitchValueASCII(
+        switches::kSimulateOutdated);
+    base::Time maybe_build_time;
+    bool result = base::Time::FromString(build_date.c_str(), &maybe_build_time);
+    if (result && !maybe_build_time.is_null()) {
+      // We got a valid build date simulation so use it and check for upgrades.
+      build_date_ = maybe_build_time;
+      StartTimerForUpgradeCheck();
+    } else {
+      // Without a valid date, we simulate that we are already outdated...
+      UpgradeDetected(UPGRADE_NEEDED_OUTDATED_INSTALL);
+    }
+    return;
+  }
+
+  // Windows: only enable upgrade notifications for official builds.
+  // Mac: only enable them if the updater (Keystone) is present.
+  // Linux (and other POSIX): always enable regardless of branding.
+  base::Closure start_upgrade_check_timer_task =
+      base::Bind(&UpgradeDetectorImpl::StartTimerForUpgradeCheck,
+                 weak_factory_.GetWeakPtr());
+#if defined(OS_WINDOW) && defined(GOOGLE_CHROME_BUILD)
+  // On Windows, there might be a policy preventing updates, so validate
+  // updatability, and then call StartTimerForUpgradeCheck appropriately.
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+                          base::Bind(&DetectUpdatability,
+                                     start_upgrade_check_timer_task,
+                                     &is_unstable_channel_));
+  return;
+#elif defined(OS_WINDOW) && !defined(GOOGLE_CHROME_BUILD)
+  return;  // Chromium has no upgrade channel.
+#elif defined(OS_MACOSX)
+  if (!keystone_glue::KeystoneEnabled())
+    return;  // Keystone updater not enabled.
+#elif !defined(OS_POSIX)
+  return;
+#endif
+
+  // Check whether the build is an unstable channel before starting the timer.
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+                          base::Bind(&CheckForUnstableChannel,
+                                     start_upgrade_check_timer_task,
+                                     &is_unstable_channel_));
+}
+
+UpgradeDetectorImpl::~UpgradeDetectorImpl() {
+}
+
+// Static
 // This task checks the currently running version of Chrome against the
-// installed version. If the installed version is newer, it runs the passed
-// callback task. Otherwise it just deletes the task.
-void DetectUpgradeTask(const base::Closure& upgrade_detected_task,
-                       bool* is_unstable_channel,
-                       bool* is_critical_upgrade) {
+// installed version. If the installed version is newer, it calls back
+// UpgradeDetectorImpl::UpgradeDetected using |upgrade_detector|.
+void UpgradeDetectorImpl::DetectUpgradeTask(
+    base::WeakPtr<UpgradeDetectorImpl> upgrade_detector) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   Version installed_version;
@@ -83,14 +218,7 @@ void DetectUpgradeTask(const base::Closure& upgrade_detected_task,
   // Get the version of the currently *installed* instance of Chrome,
   // which might be newer than the *running* instance if we have been
   // upgraded in the background.
-  FilePath exe_path;
-  if (!PathService::Get(base::DIR_EXE, &exe_path)) {
-    NOTREACHED() << "Failed to find executable path";
-    return;
-  }
-
-  bool system_install =
-      !InstallUtil::IsPerUserInstall(exe_path.value().c_str());
+  bool system_install = IsSystemInstall();
 
   // TODO(tommi): Check if using the default distribution is always the right
   // thing to do.
@@ -117,10 +245,6 @@ void DetectUpgradeTask(const base::Closure& upgrade_detected_task,
   installed_version = Version(reply);
 #endif
 
-  chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
-  *is_unstable_channel = channel == chrome::VersionInfo::CHANNEL_DEV ||
-                         channel == chrome::VersionInfo::CHANNEL_CANARY;
-
   // Get the version of the currently *running* instance of Chrome.
   chrome::VersionInfo version_info;
   if (!version_info.is_valid()) {
@@ -140,64 +264,77 @@ void DetectUpgradeTask(const base::Closure& upgrade_detected_task,
       (installed_version.CompareTo(running_version) > 0)) {
     // If a more recent version is available, it might be that we are lacking
     // a critical update, such as a zero-day fix.
-    *is_critical_upgrade =
-        critical_update.IsValid() &&
-        (critical_update.CompareTo(running_version) > 0);
+    UpgradeAvailable upgrade_available = UPGRADE_AVAILABLE_REGULAR;
+    if (critical_update.IsValid() &&
+        critical_update.CompareTo(running_version) > 0) {
+      upgrade_available = UPGRADE_AVAILABLE_CRITICAL;
+    }
 
     // Fire off the upgrade detected task.
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            upgrade_detected_task);
+                            base::Bind(&UpgradeDetectorImpl::UpgradeDetected,
+                                       upgrade_detector,
+                                       upgrade_available));
   }
 }
 
-}  // namespace
-
-UpgradeDetectorImpl::UpgradeDetectorImpl()
-    : ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
-      is_unstable_channel_(false) {
-  CommandLine command_line(*CommandLine::ForCurrentProcess());
-  if (command_line.HasSwitch(switches::kDisableBackgroundNetworking))
-    return;
-  if (command_line.HasSwitch(switches::kSimulateUpgrade)) {
-    UpgradeDetected();
-    return;
-  }
-  // Windows: only enable upgrade notifications for official builds.
-  // Mac: only enable them if the updater (Keystone) is present.
-  // Linux (and other POSIX): always enable regardless of branding.
-#if (defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD)) || defined(OS_POSIX)
-#if defined(OS_MACOSX)
-  if (keystone_glue::KeystoneEnabled())
-#endif
-  {
-    detect_upgrade_timer_.Start(FROM_HERE,
-        base::TimeDelta::FromMilliseconds(GetCheckForUpgradeEveryMs()),
-        this, &UpgradeDetectorImpl::CheckForUpgrade);
-  }
-#endif
-}
-
-UpgradeDetectorImpl::~UpgradeDetectorImpl() {
+void UpgradeDetectorImpl::StartTimerForUpgradeCheck() {
+  detect_upgrade_timer_.Start(FROM_HERE,
+      base::TimeDelta::FromMilliseconds(GetCheckForUpgradeEveryMs()),
+      this, &UpgradeDetectorImpl::CheckForUpgrade);
 }
 
 void UpgradeDetectorImpl::CheckForUpgrade() {
+  // Interrupt any (unlikely) unfinished business.

[Finnur, 2013/02/05 10:52:19]

Can you expand a bit on this comment (meaning: that we are talking about
unfinished *DetectUpgradeTask* business, and not unfinished
StartTimerForUpgradeCheck business). Perhaps mention the point of doing this
before checking for outdated.

[MAD, 2013/02/05 16:49:16]

Done.

   weak_factory_.InvalidateWeakPtrs();
-  base::Closure callback_task =
-      base::Bind(&UpgradeDetectorImpl::UpgradeDetected,
-                 weak_factory_.GetWeakPtr());
+
+  // No need to look for upgrades if the install is outdated.
+  if (CheckForOutdatedInstall())
+    return;
+
   // We use FILE as the thread to run the upgrade detection code on all
   // platforms. For Linux, this is because we don't want to block the UI thread
   // while launching a background process and reading its output; on the Mac and
   // on Windows checking for an upgrade requires reading a file.
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-                          base::Bind(&DetectUpgradeTask,
-                                     callback_task,
-                                     &is_unstable_channel_,
-                                     &is_critical_upgrade_));
+                          base::Bind(&UpgradeDetectorImpl::DetectUpgradeTask,
+                                     weak_factory_.GetWeakPtr()));
+}
+
+bool UpgradeDetectorImpl::CheckForOutdatedInstall() {
+  // Only enable the outdated install check if we are running the trial for it,
+  // unless we are simulating an outdated isntall.
+  static bool simulate_outdated = CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kSimulateOutdated);
+  if (base::FieldTrialList::FindFullName(kOutdatedInstallCheckTrialName) !=
+          kOutdatedInstallCheck12WeeksGroupName && !simulate_outdated) {
+    return false;
+  }
+
+  base::Time network_time(base::Time::Now());
+// TODO(mad): Uncomment when variations service NetworkTime CL gets committed.
+//  if (!g_browser_process->variations_service()->GetNetworkTime(&network_time))
+//    return;
+
+  if (network_time.is_null() || build_date_.is_null() ||
+      build_date_ > network_time) {
+    NOTREACHED();
+    return false;
+  }
+
+  if (network_time - build_date_ >
+      base::TimeDelta::FromDays(kOutdatedBuildAgeInDays)) {
+    UpgradeDetected(UPGRADE_NEEDED_OUTDATED_INSTALL);

[Finnur, 2013/02/05 10:52:19]

You can move this line to line 293 without adverse effect since that is the only
consumer of this function. That should make this function do just what it says
in its name (and the name should probably be IsOutdatedInstall, btw).

[MAD, 2013/02/05 16:49:16]

OK, sorry for not being clear, but this won't work unless I do another check for
simulate_outdated or return a tri-state from this method, i.e., 1) no outdated
detected nor simulated, so go on to check for version upgrade (the current
false), 2) an outdated was detected, so don't continue, UpgradeDetected was
already called (the current true), 3) not outdated yet, but we have a simulation
with a date that should eventually reach outdated state, so don't continue with
version upgrade check, but I didn't call UpgradeDetected yet, so call me again
(also returning true right now).

How about we rename the function to DetectOutdatedInstall? And just as
DetectUpgradeTask, it's allowed to call UpgradeDetected.

[Finnur, 2013/02/05 22:14:50]

Oh, I see. Good point. I forgot about the simulate cmd line param. Fine.

+    return true;
+  }
+  // If we simlated an outdated install with a date, we don't want to keep

[Finnur, 2013/02/05 10:52:19]

I think this is correct, but just to be sure: We will not get here if no
outdated date was simulated?

[MAD, 2013/02/05 16:49:16]

It is possible to get here when we have no current simulation and we are not
outdated yet, this is the most frequent case of returning false.

[Finnur, 2013/02/05 22:14:50]

Yes, my question was more along the lines of "I'm curious as to why the comment
specifies _with a date_". This is probably right, just making sure.

+  // checking for version upgrades, which happens on non-official builds.
+  return simulate_outdated;
 }
 
-void UpgradeDetectorImpl::UpgradeDetected() {
+void UpgradeDetectorImpl::UpgradeDetected(UpgradeAvailable upgrade_available) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  upgrade_available_ = upgrade_available;
 
   // Stop the recurring timer (that is checking for changes).
   detect_upgrade_timer_.Stop();
@@ -222,13 +359,14 @@ void UpgradeDetectorImpl::NotifyOnUpgrade() {
   bool is_testing = IsTesting();
   int64 time_passed = is_testing ? delta.InSeconds() : delta.InHours();
 
+  bool is_critical_or_outdated = upgrade_available_ > UPGRADE_AVAILABLE_REGULAR;
   if (is_unstable_channel_) {
     // There's only one threat level for unstable channels like dev and
     // canary, and it hits after one hour. During testing, it hits after one
     // minute.
     const int kUnstableThreshold = 1;
 
-    if (is_critical_upgrade_)
+    if (is_critical_or_outdated)
       set_upgrade_notification_stage(UPGRADE_ANNOYANCE_CRITICAL);
     else if (time_passed >= kUnstableThreshold) {
       set_upgrade_notification_stage(UPGRADE_ANNOYANCE_LOW);
@@ -247,10 +385,10 @@ void UpgradeDetectorImpl::NotifyOnUpgrade() {
     const int kLowThreshold = 2 * kMultiplier;
 
     // These if statements must be sorted (highest interval first).
-    if (time_passed >= kSevereThreshold || is_critical_upgrade_) {
+    if (time_passed >= kSevereThreshold || is_critical_or_outdated) {
       set_upgrade_notification_stage(
-          is_critical_upgrade_ ? UPGRADE_ANNOYANCE_CRITICAL :
-                                 UPGRADE_ANNOYANCE_SEVERE);
+          is_critical_or_outdated ? UPGRADE_ANNOYANCE_CRITICAL :
+                                    UPGRADE_ANNOYANCE_SEVERE);
 
       // We can't get any higher, baby.
       upgrade_notification_timer_.Stop();