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

chrome/browser/google/google_update_win.h

Index: chrome/browser/google/google_update_win.h
diff --git a/chrome/browser/google/google_update_win.h b/chrome/browser/google/google_update_win.h
index 75119e841281b836a81f55f1ea6ad5e7b4cb6165..9df5b9426e200fbf671fb9e748d59da3ac17fb4f 100644
--- a/chrome/browser/google/google_update_win.h
+++ b/chrome/browser/google/google_update_win.h
@@ -25,13 +25,13 @@ enum GoogleUpdateUpgradeResult {
   UPGRADE_STARTED = 0,
   // A check for upgrade has been initiated.
   UPGRADE_CHECK_STARTED = 1,
-  // An update is available.
+  // An update is available (terminal condition when not installing if newer).

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

Nit: I'm slightly confused by "when not installing if newer" as I don't know
whether to parse as "terminal condition when (not (installing if newer))" or
"(terminal condition when not installing) if newer" and frankly I don't know
what either one of those means anyway.  Maybe add just a little more verbosity
so there are some nouns/pronouns?

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

All values represent terminal conditions now, so I've removed the confusing
comments.

   UPGRADE_IS_AVAILABLE = 2,
-  // The upgrade happened successfully.
+  // The upgrade happened successfully (terminal condition).
   UPGRADE_SUCCESSFUL = 3,
-  // No need to upgrade, Chrome is up to date.
+  // No need to upgrade, Chrome is up to date (terminal condition).
   UPGRADE_ALREADY_UP_TO_DATE = 4,
-  // An error occurred.
+  // An error occurred (terminal condition).
   UPGRADE_ERROR = 5,
   NUM_UPGRADE_RESULTS
 };
@@ -44,8 +44,8 @@ enum GoogleUpdateErrorCode {
   // location. This error will appear for developer builds and with
   // installations unzipped to random locations.
   CANNOT_UPGRADE_CHROME_IN_THIS_DIRECTORY = 1,
-  // Failed to create Google Update JobServer COM class.
-  GOOGLE_UPDATE_JOB_SERVER_CREATION_FAILED = 2,
+  // Failed to create Google Update JobServer COM class. DEPRECATED.
+  // GOOGLE_UPDATE_JOB_SERVER_CREATION_FAILED = 2,
   // Failed to create Google Update OnDemand COM class.
   GOOGLE_UPDATE_ONDEMAND_CLASS_NOT_FOUND = 3,
   // Google Update OnDemand COM class reported an error during a check for
@@ -63,16 +63,20 @@ enum GoogleUpdateErrorCode {
   GOOGLE_UPDATE_DISABLED_BY_POLICY = 8,
   // Updates can not be downloaded because the administrator has disabled
   // manual (on-demand) updates.  Automatic background updates are allowed.
-  GOOGLE_UPDATE_DISABLED_BY_POLICY_AUTO_ONLY = 9,
+  // DEPRECATED.
+  // GOOGLE_UPDATE_DISABLED_BY_POLICY_AUTO_ONLY = 9,
   NUM_ERROR_CODES
 };
 
-// A callback run when a check for updates has completed. |result| indicates the
-// end state of the operation. When |result| is UPGRADE_ERROR, |error_code| and
-// |error_message| indicate the the nature of the error. When |result| is
-// UPGRADE_IS_AVAILABLE or UPGRADE_SUCCESSFUL, |version| may indicate the new
-// version Google Update detected (it may, however, be empty).
+// A callback run once or more while an update check is in progress and once

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

Nit: "once or more" -> "at least once"?

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

Callback removed.

+// when it completes. |result| indicates the current state of the
+// operation. When |result| is UPGRADE_STARTED, |progress| (a value between 0.0
+// and 1.0) indicates the progress of the update. |version|, if not empty,
+// indicates the version discovered by Google Update. When |result| is
+// UPGRADE_ERROR, |error_code| and |error_message| indicate the the nature of
+// the error.

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

We have several parameters that only have meaning in different states.

What if we supplied a couple of callbacks, a la:

OnUpgradeStarted(progress)
OnUpgradeError(code, message)
OnOtherUpgradeStatus(status)

This would be a little more verbose but clearer and harder to misuse.

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

That makes sense. I left it as a single callback initially since this is
somewhat similar to the CrOS case, which uses a single method to report
everything. I've introduced a delegate with a few methods that seem pretty clear
from the consumer's point of view.

 typedef base::Callback<void(GoogleUpdateUpgradeResult result,
+                            double progress,
                             GoogleUpdateErrorCode error_code,
                             const base::string16& error_message,
                             const base::string16& version)> UpdateCheckCallback;
@@ -80,21 +84,22 @@ typedef base::Callback<void(GoogleUpdateUpgradeResult result,
 // Begins an asynchronous update check on |task_runner|, which must run a
 // TYPE_UI message loop. If |install_if_newer| is true, an update will be
 // applied. |elevation_window| is the window which should own any necessary

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

Nit: "If |install_if_newer| is true, any available update will automatically be
applied."?

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

Done.

-// elevation UI. |callback| will be run on the caller's thread when the check
-// completes.
+// elevation UI. |callback| will be run on the caller's thread to provide status
+// during the update check and the final results when the check completes.
 void BeginUpdateCheck(const scoped_refptr<base::TaskRunner>& task_runner,
+                      const std::string& locale,
                       bool install_if_newer,
                       gfx::AcceleratedWidget elevation_window,
                       const UpdateCheckCallback& callback);
 
-// A type of callback supplied by tests to provide a custom IGoogleUpdate
+// A type of callback supplied by tests to provide a custom IGoogleUpdate3

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

Nit: Perhaps note where a reader can find out more about IGoogleUpdate3?

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

I've put a pointer to the idl. Unfortunately, I was unable to find any good
documentation published along with Google Update.

 // implementation.
-typedef base::Callback<HRESULT(base::win::ScopedComPtr<IGoogleUpdate>*)>
-    OnDemandAppsClassFactory;
+typedef base::Callback<HRESULT(base::win::ScopedComPtr<IGoogleUpdate3>*)>
+    GoogleUpdate3ClassFactory;
 
 // For use by tests that wish to provide a custom IGoogleUpdate implementation
 // independent of Google Update's.
 void SetGoogleUpdateFactoryForTesting(
-    const OnDemandAppsClassFactory& google_update_factory);
+    const GoogleUpdate3ClassFactory& google_update_factory);
 
 #endif  // CHROME_BROWSER_GOOGLE_GOOGLE_UPDATE_WIN_H_