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

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

Index: chrome/browser/ui/webui/help/version_updater_win.cc
diff --git a/chrome/browser/ui/webui/help/version_updater_win.cc b/chrome/browser/ui/webui/help/version_updater_win.cc
index 0d9b5286675d75f8dfc92423467e1988d77aec95..2c59ece83bbb6fd19c57e5e1eec0b674df7846e3 100644
--- a/chrome/browser/ui/webui/help/version_updater_win.cc
+++ b/chrome/browser/ui/webui/help/version_updater_win.cc
@@ -2,65 +2,55 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#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/first_run/upgrade_util_win.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"
+#include "content/public/browser/web_contents.h"
+#include "ui/aura/window.h"
+#include "ui/aura/window_tree_host.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/native_widget_types.h"
-#include "ui/views/widget/widget.h"
-
-using content::BrowserThread;
 
 namespace {
 
 // Windows implementation of version update functionality, used by the WebUI
 // About/Help page.
-class VersionUpdaterWin : public VersionUpdater {
- private:
-  friend class VersionReader;
-  friend class VersionUpdater;
-
-  // Clients must use VersionUpdater::Create().
-  VersionUpdaterWin();
+class VersionUpdaterWin : public VersionUpdater, public UpdateCheckDelegate {
+ public:
+  // |owner_widget| is the parent widget hosting the update check UI. Any UI
+  // needed to install an update (e.g., a UAC prompt for a system-level install)
+  // will be parented to this widget.
+  explicit VersionUpdaterWin(gfx::AcceleratedWidget owner_widget);
   ~VersionUpdaterWin() override;
 
-  // VersionUpdater implementation.
+  // VersionUpdater:
   void CheckForUpdate(const StatusCallback& callback) override;
   void RelaunchBrowser() const override;
 
-  // chrome::UpdateCheckCallback.
-  void OnUpdateCheckResults(GoogleUpdateUpgradeResult result,
-                            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,
-                    GoogleUpdateErrorCode error_code,
-                    const base::string16& error_message);
+  // UpdateCheckDelegate:
+  void OnUpdateCheckComplete(const base::string16& new_version) override;
+  void OnUpgradeProgress(int progress,
+                         const base::string16& new_version) override;
+  void OnUpgradeComplete(const base::string16& new_version) override;
+  void OnError(GoogleUpdateErrorCode error_code,
+               const base::string16& error_message,
+               const base::string16& new_version) override;
 
-  // 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();
+ private:
+#if defined(GOOGLE_CHROME_BUILD)
+  void BeginUpdateCheckOnFileThread(bool install_update_if_possible);
+#endif  // GOOGLE_CHROME_BUILD
 
-  void BeginUpdateCheckOnFileThread(bool install_if_newer);
+  // The widget owning the UI for the update check.
+  gfx::AcceleratedWidget owner_widget_;
 
   // Callback used to communicate update status to the client.
   StatusCallback callback_;
@@ -71,57 +61,16 @@ class VersionUpdaterWin : public VersionUpdater {
   DISALLOW_COPY_AND_ASSIGN(VersionUpdaterWin);
 };
 
-// This class is used to read the version on the FILE thread and then call back
-// the version updater in the UI thread. Using a class helps better control
-// the lifespan of the Version independently of the lifespan of the version
-// updater, which may die while asynchonicity is happening, thus the usage of
-// the WeakPtr, which can only be used from the thread that created it.
-class VersionReader
-    : public base::RefCountedThreadSafe<VersionReader> {
- public:
-  explicit VersionReader(
-      const base::WeakPtr<VersionUpdaterWin>& version_updater)
-      : version_updater_(version_updater) {
-  }
-
-  void GetVersionFromFileThread() {
-    BrowserDistribution* dist = BrowserDistribution::GetDistribution();
-    InstallUtil::GetChromeVersion(dist, false, &installed_version_);
-    if (!installed_version_.IsValid()) {
-      // User-level Chrome is not installed, check system-level.
-      InstallUtil::GetChromeVersion(dist, true, &installed_version_);
-    }
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(
-          &VersionReader::SetVersionInUIThread, this));
-  }
-
-  void SetVersionInUIThread() {
-    if (version_updater_.get() != NULL)
-      version_updater_->GotInstalledVersion(installed_version_);
-  }
-
- private:
-  friend class base::RefCountedThreadSafe<VersionReader>;
-
-  ~VersionReader() {}
-
-  // The version updater that must be called back when we are done.
-  // We use a weak pointer in case the updater gets destroyed while waiting.
-  base::WeakPtr<VersionUpdaterWin> version_updater_;
-
-  // This is the version that gets read in the FILE thread and set on the
-  // the updater in the UI thread.
-  Version installed_version_;
-};
-
-VersionUpdaterWin::VersionUpdaterWin()
-    : weak_factory_(this) {
+VersionUpdaterWin::VersionUpdaterWin(gfx::AcceleratedWidget owner_widget)
+    : owner_widget_(owner_widget), weak_factory_(this) {
 }
 
 VersionUpdaterWin::~VersionUpdaterWin() {
 }
 
 void VersionUpdaterWin::CheckForUpdate(const StatusCallback& callback) {
+  // There is no supported integration with Google Update for Chromium.
+#if defined(GOOGLE_CHROME_BUILD)
   callback_ = callback;
 
   // On-demand updates for Chrome don't work in Vista RTM when UAC is turned
@@ -132,141 +81,93 @@ void VersionUpdaterWin::CheckForUpdate(const StatusCallback& callback) {
   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,
-                 base::string16());
-    // Specify false to not upgrade yet.
-    BeginUpdateCheckOnFileThread(false);
+    callback_.Run(CHECKING, 0, base::string16());
+    BeginUpdateCheckOnFileThread(false /* !install_update_if_possible */);
   }
+#endif
 }
 
 void VersionUpdaterWin::RelaunchBrowser() const {
   chrome::AttemptRestart();
 }
 
-void VersionUpdaterWin::OnUpdateCheckResults(
-    GoogleUpdateUpgradeResult result,
-    GoogleUpdateErrorCode error_code,
-    const base::string16& error_message,
-    const base::string16& version) {
-  UpdateStatus(result, error_code, error_message);
+void VersionUpdaterWin::OnUpdateCheckComplete(
+    const base::string16& new_version) {
+#if defined(GOOGLE_CHROME_BUILD)

[Peter Kasting, 2015/05/12 22:04:34]

Do we need to #if out the contents of these next functions?  Seems like on
Chromium they'll just never be called.  The #ifs are kind of noisy.

[grt (UTC plus 2), 2015/05/13 13:04:05]

Agreed. I'm going to land this as-is. I think a better solution (which will
require more hacking, so I'll do in a folloup CL) is to have a simplified
implementation of the VersionUpdater interface for Win Chromium that only
implements RelaunchBrowser. With that change, we can cleanly omit the
google_update_win.* files from Win Chromium builds altogether. I imagine this
will be a small improvement for bot cycle times. The google_update_win* files
could still be build into unit_tests so that we'd spot regressions in Chromium
builds.

+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  Status status = CHECKING;
+  if (new_version.empty()) {
+    // Google Update says that no new version is available. Check to see if a
+    // restart is needed for a previously-applied update to take effect.
+    status = upgrade_util::IsRunningOldChrome() ? NEARLY_UPDATED : UPDATED;
+  } else {
+    // Notify the caller that the update is now beginning and initiate it.
+    status = UPDATING;
+    BeginUpdateCheckOnFileThread(true /* install_update_if_possible */);
+  }
+  callback_.Run(status, 0, base::string16());
+#endif  // GOOGLE_CHROME_BUILD
 }
 
-void VersionUpdaterWin::UpdateStatus(GoogleUpdateUpgradeResult result,
-                                     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.
+void VersionUpdaterWin::OnUpgradeProgress(int progress,
+                                          const base::string16& new_version) {
 #if defined(GOOGLE_CHROME_BUILD)
-  Status status = UPDATED;
-  base::string16 message;
-
-  switch (result) {
-    case UPGRADE_CHECK_STARTED: {
-      status = CHECKING;
-      break;
-    }
-    case UPGRADE_STARTED: {
-      status = UPDATING;
-      break;
-    }
-    case UPGRADE_IS_AVAILABLE: {
-      UpdateStatus(UPGRADE_STARTED, 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 {
-        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;
-    }
-  }
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  callback_.Run(UPDATING, progress, base::string16());
+#endif  // GOOGLE_CHROME_BUILD
+}
 
-  // TODO(mad): Get proper progress value instead of passing 0.
-  // http://crbug.com/136117
-  callback_.Run(status, 0, message);
-#endif  //  defined(GOOGLE_CHROME_BUILD)
+void VersionUpdaterWin::OnUpgradeComplete(const base::string16& new_version) {
+#if defined(GOOGLE_CHROME_BUILD)
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  callback_.Run(NEARLY_UPDATED, 0, base::string16());
+#endif  // 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);
+void VersionUpdaterWin::OnError(GoogleUpdateErrorCode error_code,
+                                const base::string16& error_message,
+                                const base::string16& new_version) {
+#if defined(GOOGLE_CHROME_BUILD)
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  base::string16 message;
 
-  // Make sure that the latest version is running and if not,
-  // notify the user by setting the status to NEARLY_UPDATED.
-  //
-  // The extra version check is necessary on Windows because the application
-  // may be already up to date on disk though the running app is still
-  // out of date.
-  chrome::VersionInfo version_info;
-  Version running_version(version_info.Version());
-  callback_.Run((version.IsValid() && version.CompareTo(running_version) > 0)
-                    ? NEARLY_UPDATED
-                    : UPDATED,
-                0,
-                base::string16());
-}
+  // Current versions of Google Update provide a nice message for the policy
+  // case. Use this generic error for the policy case only if no message from
+  // Google Update is present.
+  if (error_code != GOOGLE_UPDATE_DISABLED_BY_POLICY || error_message.empty())
+    message = l10n_util::GetStringFUTF16Int(IDS_UPGRADE_ERROR, error_code);
 
-BOOL CALLBACK WindowEnumeration(HWND window, LPARAM param) {
-  if (IsWindowVisible(window)) {
-    HWND* returned_window = reinterpret_cast<HWND*>(param);
-    *returned_window = window;
-    return FALSE;
+  if (!error_message.empty()) {
+    message += l10n_util::GetStringFUTF16(
+        IDS_ABOUT_BOX_ERROR_DURING_UPDATE_CHECK, error_message);
   }
-  return TRUE;
-}
-
-gfx::AcceleratedWidget VersionUpdaterWin::GetElevationParent() {
-  // Look for a visible window belonging to the UI thread.
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  HWND window = NULL;
-  EnumThreadWindows(GetCurrentThreadId(),
-                    WindowEnumeration,
-                    reinterpret_cast<LPARAM>(&window));
-  return window;
+  callback_.Run(FAILED, 0, message);
+#endif  // GOOGLE_CHROME_BUILD
 }
 
-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(),
-                   base::Bind(&VersionUpdaterWin::OnUpdateCheckResults,
-                              weak_factory_.GetWeakPtr()));
+#if defined(GOOGLE_CHROME_BUILD)
+void VersionUpdaterWin::BeginUpdateCheckOnFileThread(
+    bool install_update_if_possible) {
+  BeginUpdateCheck(content::BrowserThread::GetMessageLoopProxyForThread(
+                       content::BrowserThread::FILE),

[Peter Kasting, 2015/05/12 22:04:34]

Perhaps outside the scope of this CL, but with my Code Purple hat on: prefer to
use the blocking pool over the FILE thread wherever possible.

[grt (UTC plus 2), 2015/05/13 13:04:05]

I tried that early on in this change and found that it can't be done in the
blocking pool as it exists today. I'll start a thread on chrome-fast so that the
Purplers are aware of the issues and can design accordingly.

[Sorin Jianu, 2015/05/18 20:32:45]

re: COM on FILE thread, I am interested in the solution to this problem too.
Greg, please let me know if you have an answer and when you are changing this
code. The component update service uses a similar implementation. We need an
single-thread COM apartment to make COM calls to BITS. We use the FILE thread to
do this as the FILE thread is initialized for COM already and we thought is
impractical to use or create a blocking pool for this reason alone.

+                   g_browser_process->GetApplicationLocale(),
+                   install_update_if_possible, owner_widget_,
+                   weak_factory_.GetWeakPtr());
 }
+#endif  // GOOGLE_CHROME_BUILD
 
 }  // namespace
 
-VersionUpdater* VersionUpdater::Create(content::BrowserContext* /* context */) {
-  return new VersionUpdaterWin;
+VersionUpdater* VersionUpdater::Create(content::WebContents* web_contents) {
+  // Retrieve the HWND for the browser window that is hosting the update check.
+  // This will be used as the parent for a UAC prompt, if needed. It's possible
+  // this this window will no longer have focus by the time UAC is needed. In
+  // that case, the UAC prompt will appear in the taskbar and will require a
+  // user click. This is the least surprising thing we can do for the user, and
+  // is the intended behavior for Windows applications. It's also possible that
+  // the browser window hosting the update check will have been closed by the
+  // time the UAC prompt is needed. This will behave similarly.
+  return new VersionUpdaterWin(web_contents->GetTopLevelNativeWindow()
+                                   ->GetHost()
+                                   ->GetAcceleratedWidget());
 }