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..8e2ec93035ad1d0e7e56cb0338dc84f847f25119 100644
--- a/chrome/browser/upgrade_detector_impl.cc
+++ b/chrome/browser/upgrade_detector_impl.cc
@@ -7,6 +7,7 @@
 #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"
@@ -14,21 +15,28 @@
 #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 "googleurl/src/gurl.h"
+#include "net/base/load_flags.h"
+#include "net/http/http_response_headers.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_status.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 +54,10 @@ const int kNotifyCycleTimeMs = 20 * 60 * 1000;  // 20 minutes.
 // Same as kNotifyCycleTimeMs but only used during testing.
 const int kNotifyCycleTimeForTestingMs = 500;  // Half a second.
 
+// Default server of Variations seed info.
+const char kTimeServerURL[] = "https://www.google.com";

[Finnur, 2013/01/25 11:39:22]

This worries me. Chrome's user base is, as you know, extremely large and I doubt
this is an approved way of getting the accurate time for such a large number of
users.

You should look up the thread "Accurate clocks and Chromium" on chromium-dev,
because at a glance it looks like akalin was coming up with a more general
purpose solution to this problem back in August of last year.

[MAD, 2013/01/29 21:13:10]

I had looked it up and all I found was the following command line app:
http://src.chromium.org/viewvc/chrome/trunk/src/net/tools/get_server_time/get...

I don't think the general purpose util is available yet, though I could check
with akalin to get more accurate status on this..

Also,thanks for mentioning this because my initial goal was to request the sane
time only once per session, and then I forgot about it... Bad MAD, no cookie!
:-)

+const int kMaxRetryTimeFetch = 5;
+
 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,6 +82,18 @@ int GetCheckForUpgradeEveryMs() {
   return kCheckForUpgradeMs;
 }
 
+#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());
+}
+#endif
+
 // 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.
@@ -83,14 +109,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.
@@ -150,11 +169,35 @@ void DetectUpgradeTask(const base::Closure& upgrade_detected_task,
   }
 }
 
+#if defined(OS_WIN)
+// This task checks the update policy and calls back the task only if automatic
+// updates are allowed.
+void DetectUpdatability(const base::Closure& updates_enabled) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+  CommandLine command_line(*CommandLine::ForCurrentProcess());
+  if (command_line.HasSwitch(switches::kSimulateOutdated)) {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            updates_enabled);
+    return;
+  }
+
+  string16 app_guid = installer::GetAppGuidForUpdates(IsSystemInstall());
+  DCHECK(!app_guid.empty());
+  if (GoogleUpdateSettings::GetAppUpdatePolicy(app_guid, NULL) ==
+      GoogleUpdateSettings::AUTOMATIC_UPDATES) {

[Finnur, 2013/01/25 11:39:22]

I didn't think that the 'prevent any updates' enterprise policy was Windows
only. Is that the case?

[MAD, 2013/01/29 21:13:10]

I couldn't find an implementation for other platforms. Except for Chrome OS, for
which we don't want to show the outdated bubble anyway.

+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            updates_enabled);
+  }
+}
+#endif  // defined(OS_WIN)
+
 }  // namespace
 
 UpgradeDetectorImpl::UpgradeDetectorImpl()
     : ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
-      is_unstable_channel_(false) {
+      is_unstable_channel_(false),
+      build_date_(base::GetBuildTime()) {
   CommandLine command_line(*CommandLine::ForCurrentProcess());
   if (command_line.HasSwitch(switches::kDisableBackgroundNetworking))
     return;
@@ -162,6 +205,27 @@ UpgradeDetectorImpl::UpgradeDetectorImpl()
     UpgradeDetected();
     return;
   }
+  if (command_line.HasSwitch(switches::kSimulateCriticalUpdate)) {
+    is_critical_upgrade_ = true;
+    UpgradeDetected();
+    return;
+  }
+  if (command_line.HasSwitch(switches::kSimulateOutdated)) {
+    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()) {
+      build_date_ = maybe_build_time;
+      detect_upgrade_timer_.Start(FROM_HERE,
+          base::TimeDelta::FromMilliseconds(GetCheckForUpgradeEveryMs()),
+          this, &UpgradeDetectorImpl::CheckForUpgrade);
+    } else {
+      is_outdated_install_ = true;
+      UpgradeDetected();

[Finnur, 2013/01/25 11:39:22]

I don't quite grok this...

If we fail to parse the simulated build date from the command line (result ==
false), we assume the build is outdated and trigger the notification. Huh?

Is this so you can either specify a time (and wait for it to prompt you) or no
time (and have it prompt you immediately)? Can you perhaps comment what you are
after here with the outer if clause (line 213) and the inner if-else clause
(line 218)?

[MAD, 2013/01/29 21:13:10]

Done.

+    }
+    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.
@@ -194,6 +258,35 @@ void UpgradeDetectorImpl::CheckForUpgrade() {
                                      callback_task,
                                      &is_unstable_channel_,
                                      &is_critical_upgrade_));
+// On Windows, there might be a policy preventing updates, so validate
+// updatability before comparing build time to sane time.
+#if defined(OS_WIN)
+  if (!is_outdated_install_ && !pending_time_request_.get()) {
+    base::Closure callback_task =
+        base::Bind(&UpgradeDetectorImpl::CompareBuildTimeToSaneTime,
+                   weak_factory_.GetWeakPtr());
+    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+                            base::Bind(&DetectUpdatability,
+                                       callback_task));
+  }
+#else  // defined(OS_WIN)

[Finnur, 2013/01/25 11:39:22]

nit: shouldn't this just be on the endif?

[MAD, 2013/01/29 21:13:10]

Done.

+  CompareBuildTimeToSaneTime();
+#endif
+}
+
+void UpgradeDetectorImpl::CompareBuildTimeToSaneTime() {
+  // Check again, in case we got posted more than once by CheckForUpgrade.
+  if (!is_outdated_install_ && !pending_time_request_.get()) {
+    // Now get the current time from a URL request to ensure we get a sane time.
+    pending_time_request_.reset(net::URLFetcher::Create(
+        GURL(kTimeServerURL), net::URLFetcher::GET, this));
+    pending_time_request_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
+                                        net::LOAD_DO_NOT_SAVE_COOKIES);
+    pending_time_request_->SetRequestContext(
+        g_browser_process->system_request_context());
+    pending_time_request_->SetMaxRetriesOn5xx(kMaxRetryTimeFetch);
+    pending_time_request_->Start();
+  }
 }
 
 void UpgradeDetectorImpl::UpgradeDetected() {
@@ -222,13 +315,14 @@ void UpgradeDetectorImpl::NotifyOnUpgrade() {
   bool is_testing = IsTesting();
   int64 time_passed = is_testing ? delta.InSeconds() : delta.InHours();
 
+  bool is_critical_or_oudated = is_critical_upgrade_ || is_outdated_install_;
   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_oudated)
       set_upgrade_notification_stage(UPGRADE_ANNOYANCE_CRITICAL);
     else if (time_passed >= kUnstableThreshold) {
       set_upgrade_notification_stage(UPGRADE_ANNOYANCE_LOW);
@@ -247,10 +341,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_oudated) {
       set_upgrade_notification_stage(
-          is_critical_upgrade_ ? UPGRADE_ANNOYANCE_CRITICAL :
-                                 UPGRADE_ANNOYANCE_SEVERE);
+          is_critical_or_oudated ? UPGRADE_ANNOYANCE_CRITICAL :
+                                   UPGRADE_ANNOYANCE_SEVERE);
 
       // We can't get any higher, baby.
       upgrade_notification_timer_.Stop();
@@ -268,6 +362,24 @@ void UpgradeDetectorImpl::NotifyOnUpgrade() {
   NotifyUpgradeRecommended();
 }
 
+void UpgradeDetectorImpl::OnURLFetchComplete(const net::URLFetcher* source) {
+  // The fetcher will be deleted when the request is handled.
+  scoped_ptr<const net::URLFetcher> request(
+      pending_time_request_.release());
+  if (request->GetStatus().status() != net::URLRequestStatus::SUCCESS) {
+    DVLOG(1) << "Variations server request failed.";

[Finnur, 2013/01/25 11:39:22]

Of course, this might be a moot point (if you switch away from this method) but
why is it called Variations server?
And is there no error code you want to output as well?

[MAD, 2013/01/29 21:13:10]

Copy/Paste error... Cleared... Not really needed...

+    return;
+  }
+
+  base::Time response_date;
+  bool success = request->GetResponseHeaders()->GetDateValue(&response_date);
+  if (success) {
+    DCHECK(!response_date.is_null() && response_date >= build_date_);
+    if (response_date - build_date_ > base::TimeDelta::FromDays(12 * 7))

[Finnur, 2013/01/25 11:39:22]

Chrome has issued 1 major stable release per quarter, if I'm not mistaken. If
you are twelve months behind from the release date, then you are more than one
major revision behind, are you not?

Or did we relax this restriction? By the way, CL description says "on major
revision" (not one major revision).

[MAD, 2013/01/29 21:13:10]

This is the spec as debated with JeffC and ALaforge, it's 12 weeks, when we have
a 6 weeks release cycle.

+      is_outdated_install_ = true;
+  }
+}

[Finnur, 2013/01/25 11:39:22]

Essentially you are setting the |is_outdated_install_| flag and patiently
waiting for the next update check to happen? Correct? It might help the
uninitiated to document that.

[MAD, 2013/01/29 21:13:10]

Ho... Actually, I needed to add a call to UpgradeDetected here, thanks for
catching it... It's weird that it worked anyway with the command line switch to
test with an outdated time, in that case, UpgradeDetected should not have been
called...

+
 // static
 UpgradeDetectorImpl* UpgradeDetectorImpl::GetInstance() {
   return Singleton<UpgradeDetectorImpl>::get();