Switch on-demand update checks to the less-old GoogleUpdate3 API.

chrome/browser/ui/webui/help/version_updater_win.cc

 // Copyright (c) 2012 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 <algorithm>
+
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
 #include "base/version.h"
 #include "base/win/win_util.h"
 #include "base/win/windows_version.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/google/google_update_win.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/webui/help/version_updater.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/install_util.h"
 #include "content/public/browser/browser_thread.h"
@@ skipping 15 matching lines @@
   // Clients must use VersionUpdater::Create().
   VersionUpdaterWin();
   ~VersionUpdaterWin() override;
 
   // VersionUpdater implementation.
   void CheckForUpdate(const StatusCallback& callback) override;
   void RelaunchBrowser() const override;
 
   // chrome::UpdateCheckCallback.
   void OnUpdateCheckResults(GoogleUpdateUpgradeResult result,
+                            double progress,
                             GoogleUpdateErrorCode error_code,
                             const base::string16& error_message,
                             const base::string16& version);
 
   // Update the UI to show the status of the upgrade.
   void UpdateStatus(GoogleUpdateUpgradeResult result,
+                    double update_progress,
                     GoogleUpdateErrorCode error_code,
                     const base::string16& error_message);
 
   // Got the intalled version so the handling of the UPGRADE_ALREADY_UP_TO_DATE
   // result case can now be completeb on the UI thread.
   void GotInstalledVersion(const Version& version);
 
   // Returns a window that can be used for elevation.
   gfx::AcceleratedWidget GetElevationParent();
 
@@ skipping 62 matching lines @@
   callback_ = callback;
 
   // On-demand updates for Chrome don't work in Vista RTM when UAC is turned
   // off. So, in this case, the version updater must not mention
   // on-demand updates. Silent updates (in the background) should still
   // work as before - enabling UAC or installing the latest service pack
   // for Vista is another option.
   if (!(base::win::GetVersion() == base::win::VERSION_VISTA &&
         (base::win::OSInfo::GetInstance()->service_pack().major == 0) &&
         !base::win::UserAccountControlIsEnabled())) {
-    UpdateStatus(UPGRADE_CHECK_STARTED, GOOGLE_UPDATE_NO_ERROR,
+    UpdateStatus(UPGRADE_CHECK_STARTED, 0.0, GOOGLE_UPDATE_NO_ERROR,
                  base::string16());
     // Specify false to not upgrade yet.
     BeginUpdateCheckOnFileThread(false);
   }
 }
 
 void VersionUpdaterWin::RelaunchBrowser() const {
   chrome::AttemptRestart();
 }
 
 void VersionUpdaterWin::OnUpdateCheckResults(
     GoogleUpdateUpgradeResult result,
+    double progress,
     GoogleUpdateErrorCode error_code,
     const base::string16& error_message,
     const base::string16& version) {
-  UpdateStatus(result, error_code, error_message);
+  UpdateStatus(result, progress, error_code, error_message);
 }
 
 void VersionUpdaterWin::UpdateStatus(GoogleUpdateUpgradeResult result,
+                                     double update_progress,
                                      GoogleUpdateErrorCode error_code,
                                      const base::string16& error_message) {
   // For Chromium builds it would show an error message.
   // But it looks weird because in fact there is no error,
   // just the update server is not available for non-official builds.
 #if defined(GOOGLE_CHROME_BUILD)
   Status status = UPDATED;
+  int progress = 0;
   base::string16 message;
 
   switch (result) {
     case UPGRADE_CHECK_STARTED: {
       status = CHECKING;
       break;
     }
     case UPGRADE_STARTED: {
       status = UPDATING;
+      progress = static_cast<int>((update_progress * 100.0) + 0.5);

[Peter Kasting, 2015/05/07 01:12:28]

Technically, things like this should probably use ToFlooredInt() from
safe_integer_conversions.h.

However, looking at the code here to convert this double to an int, and the code
in UpdateCheckDriver::IsIntermediateState() that actually computes the value, I
wonder whether making this a double is really the right thing to do.

It seems like we could use an int everywhere and save a bit of complexity here,
while not really making the code there worse: the "downloading" case basically
changes from "divide by 2" to "multiply by 50" and the "installing" case just
becomes "50 + install_progress / 2".

It doesn't really seem more arbitrary to me to pass an int ranging from 0-100
than a double ranging from 0-1.  WDYT?

(I'd probably just ditch the min() call too, this code is tightly tied to the
other code and it seems OK to rely on that code giving back a valid number.  If
there is a clamp needed anywhere, I'd suggest it in the "installing" case that
fetches the original LONG that's from 0-100 -- if one is even needed there. 
This would allow ditching the <algorithm> include.)

[grt (UTC plus 2), 2015/05/08 18:51:51]

Awesome, I didn't know that was a thing.
SGTM. I was inspired to use a double by the CrOS case, but I don't think there's
any compelling reason to use it here.
No longer needed.

+      progress = std::min(progress, 100);
       break;
     }
     case UPGRADE_IS_AVAILABLE: {
-      UpdateStatus(UPGRADE_STARTED, GOOGLE_UPDATE_NO_ERROR, base::string16());
+      UpdateStatus(UPGRADE_STARTED, 0.0, GOOGLE_UPDATE_NO_ERROR,
+                   base::string16());
       // Specify true to upgrade now.
       BeginUpdateCheckOnFileThread(true);
       return;
     }
     case UPGRADE_ALREADY_UP_TO_DATE: {
       // Google Update reported that Chrome is up-to-date.
       // To confirm the updated version is running, the reading
       // must be done on the file thread. The rest of this case
       // will be handled within GotInstalledVersion.
       BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind(
             &VersionReader::GetVersionFromFileThread,
             new VersionReader(weak_factory_.GetWeakPtr())));
       return;
     }
     case UPGRADE_SUCCESSFUL: {
       status = NEARLY_UPDATED;
       break;
     }
     case UPGRADE_ERROR: {
       status = FAILED;
-      if (error_code == GOOGLE_UPDATE_DISABLED_BY_POLICY) {
-        message =
-            l10n_util::GetStringUTF16(IDS_UPGRADE_DISABLED_BY_POLICY);
-      } else if (error_code == GOOGLE_UPDATE_DISABLED_BY_POLICY_AUTO_ONLY) {
-        message =
-            l10n_util::GetStringUTF16(IDS_UPGRADE_DISABLED_BY_POLICY_MANUAL);
-      } else {
+      // Google Update provides a nice message for the policy case. Use this
+      // generic error for the policy case only if no message from Google Update
+      // is missing.

[Peter Kasting, 2015/05/07 01:12:28]

"if no message is missing"?  Is there a double negative there?

[grt (UTC plus 2), 2015/05/08 18:51:51]

Done.

+      if (error_code != GOOGLE_UPDATE_DISABLED_BY_POLICY ||
+          error_message.empty()) {
         message =
             l10n_util::GetStringFUTF16Int(IDS_UPGRADE_ERROR, error_code);
       }
 
       if (!error_message.empty()) {
         message +=
             l10n_util::GetStringFUTF16(IDS_ABOUT_BOX_ERROR_DURING_UPDATE_CHECK,
                                        error_message);
       }
       break;
     }
   }
 
-  // TODO(mad): Get proper progress value instead of passing 0.
-  // http://crbug.com/136117
-  callback_.Run(status, 0, message);
+  callback_.Run(status, progress, message);
 #endif  //  defined(GOOGLE_CHROME_BUILD)
 }
 
 void VersionUpdaterWin::GotInstalledVersion(const Version& version) {
   // This must be called on the UI thread so that callback_ can be called.
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // Make sure that the latest version is running and if not,
   // notify the user by setting the status to NEARLY_UPDATED.
   //
@@ skipping 25 matching lines @@
   EnumThreadWindows(GetCurrentThreadId(),
                     WindowEnumeration,
                     reinterpret_cast<LPARAM>(&window));
   return window;
 }
 
 void VersionUpdaterWin::BeginUpdateCheckOnFileThread(bool install_if_newer) {
   scoped_refptr<base::TaskRunner> task_runner(
       content::BrowserThread::GetMessageLoopProxyForThread(
           content::BrowserThread::FILE));
-  BeginUpdateCheck(task_runner, install_if_newer, GetElevationParent(),
+  BeginUpdateCheck(task_runner, g_browser_process->GetApplicationLocale(),
+                   install_if_newer, GetElevationParent(),
                    base::Bind(&VersionUpdaterWin::OnUpdateCheckResults,
                               weak_factory_.GetWeakPtr()));
 }
 
 }  // namespace
 
 VersionUpdater* VersionUpdater::Create(content::BrowserContext* /* context */) {
   return new VersionUpdaterWin;
 }