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

chrome/browser/google/google_update_win.h

 // 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.
 
 #ifndef CHROME_BROWSER_GOOGLE_GOOGLE_UPDATE_WIN_H_
 #define CHROME_BROWSER_GOOGLE_GOOGLE_UPDATE_WIN_H_
 
 #include "base/basictypes.h"
 #include "base/callback_forward.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string16.h"
 #include "base/win/scoped_comptr.h"
 #include "google_update/google_update_idl.h"
 #include "ui/gfx/native_widget_types.h"
 
 namespace base {
 class TaskRunner;
 }  // namespace base
 
 // The status of the upgrade. UPGRADE_STARTED and UPGRADE_CHECK_STARTED are
 // internal states and will not be reported as results to the listener.
 // These values are used for a histogram. Do not reorder.
 enum GoogleUpdateUpgradeResult {

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

Nit: "Status" would be a better word than "Result" in the name of this.

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

Indeed. With clients receiving this value during the check/upgrade, "Result" no
longer makes sense. During the change from a single callback to a delegate, the
need for clients to see this value has vanished (as have the first two values),
so I've moved it out of the header and kept the name as-is. The remaining values
are still logged via UMA.

   // The upgrade has started.
   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
 };
 
 // These values are used for a histogram. Do not reorder.
 enum GoogleUpdateErrorCode {
   // The upgrade completed successfully (or hasn't been started yet).
   GOOGLE_UPDATE_NO_ERROR = 0,
   // Google Update only supports upgrading if Chrome is installed in the default
   // 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
   // update (or while upgrading).
   GOOGLE_UPDATE_ONDEMAND_CLASS_REPORTED_ERROR = 4,
   // A call to GetResults failed. DEPRECATED.
   // GOOGLE_UPDATE_GET_RESULT_CALL_FAILED = 5,
   // A call to GetVersionInfo failed. DEPRECATED
   // GOOGLE_UPDATE_GET_VERSION_INFO_FAILED = 6,
   // An error occurred while upgrading (or while checking for update).
   // Check the Google Update log in %TEMP% for more details.
   GOOGLE_UPDATE_ERROR_UPDATING = 7,
   // Updates can not be downloaded because the administrator has disabled all
   // types of updating.
   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;
 
 // 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_