Attempt to fix use-after-free in VersionUpdaterWin.

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 "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/string16.h"
 #include "base/version.h"
 #include "base/win/windows_version.h"
@@ skipping 44 matching lines @@
                     GoogleUpdateErrorCode error_code,
                     const 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);
 
   // Little helper function to reset google_updater_.
   void SetGoogleUpdater();
 
+  // Helper function to clear google_updater_.
+  void ClearGoogleUpdater();
+
   // Returns a window that can be used for elevation.
   HWND GetElevationParent();
 
   // The class that communicates with Google Update to find out if an update is
   // available and asks it to start an upgrade.
   scoped_refptr<GoogleUpdate> google_updater_;
 
   // Used for callbacks.
   base::WeakPtrFactory<VersionUpdaterWin> weak_factory_;
 
@@ skipping 45 matching lines @@
 };
 
 VersionUpdaterWin::VersionUpdaterWin()
     : weak_factory_(this) {
   SetGoogleUpdater();
 }
 
 VersionUpdaterWin::~VersionUpdaterWin() {
   // The Google Updater will hold a pointer to the listener until it reports
   // status, so that pointer must be cleared when the listener is destoyed.
-  if (google_updater_)
-    google_updater_->set_status_listener(NULL);
+  ClearGoogleUpdater();
 }
 
 void VersionUpdaterWin::CheckForUpdate(const StatusCallback& callback) {
   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.
@@ skipping 10 matching lines @@
 }
 
 void VersionUpdaterWin::RelaunchBrowser() const {
   chrome::AttemptRestart();
 }
 
 void VersionUpdaterWin::OnReportResults(
     GoogleUpdateUpgradeResult result, GoogleUpdateErrorCode error_code,
     const string16& error_message, const string16& version) {
   // Drop the last reference to the object so that it gets cleaned up here.
-  google_updater_ = NULL;
+  ClearGoogleUpdater();
   UpdateStatus(result, error_code, error_message);
 }
 
 void VersionUpdaterWin::UpdateStatus(GoogleUpdateUpgradeResult result,
                                      GoogleUpdateErrorCode error_code,
                                      const 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)
@@ skipping 76 matching lines @@
   if (!version.IsValid() || version.CompareTo(running_version) <= 0) {
     content::RecordAction(
         UserMetricsAction("UpgradeCheck_AlreadyUpToDate"));
     callback_.Run(UPDATED, 0, string16());
   } else {
     content::RecordAction(UserMetricsAction("UpgradeCheck_AlreadyUpgraded"));
     callback_.Run(NEARLY_UPDATED, 0, string16());
   }
 }
 
 void VersionUpdaterWin::SetGoogleUpdater() {

[MAD, 2013/05/28 19:37:06]

While you are there, maybe we should rename this to ResetGoogleUpdater.

Or maybe we should move in here the DCHECK that is done before calling this
method at one point. Because this method is only called when google_updater_ is
actually NULL or DCHECK'd to be NULL.

[grt (UTC plus 2), 2013/05/28 19:49:09]

Nice idea. I went for "CreateGoogleUpdater". SGTY?
I prefer leaving it as-is so that this method is safe to be called at any time.
The caller(s) can DCHECK that the world is as they expect it when they make the
call.

+  ClearGoogleUpdater();
   google_updater_ = new GoogleUpdate();
   google_updater_->set_status_listener(this);
 }
 
+void VersionUpdaterWin::ClearGoogleUpdater() {
+  if (google_updater_) {
+    google_updater_->set_status_listener(NULL);
+    google_updater_ = NULL;
+  }
+}
+
 BOOL CALLBACK WindowEnumeration(HWND window, LPARAM param) {
   if (IsWindowVisible(window)) {
     HWND* returned_window = reinterpret_cast<HWND*>(param);
     *returned_window = window;
     return FALSE;
   }
   return TRUE;
 }
 
 HWND VersionUpdaterWin::GetElevationParent() {
@@ skipping 10 matching lines @@
                          << GetCurrentThreadId();
 #endif
   return window;
 }
 
 }  // namespace
 
 VersionUpdater* VersionUpdater::Create() {
   return new VersionUpdaterWin;
 }