Attempt to fix use-after-free in VersionUpdaterWin.

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 2df0593bea11fcccea45945b581d95b808f34031..5cfa86faa75a08d5962fbca0ddeb1c47856aa987 100644
--- a/chrome/browser/ui/webui/help/version_updater_win.cc
+++ b/chrome/browser/ui/webui/help/version_updater_win.cc
@@ -62,6 +62,9 @@ class VersionUpdaterWin : public VersionUpdater,
   // 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();
 
@@ -127,8 +130,7 @@ VersionUpdaterWin::VersionUpdaterWin()
 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) {
@@ -159,7 +161,7 @@ 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);
 }
 
@@ -257,10 +259,18 @@ void VersionUpdaterWin::GotInstalledVersion(const Version& version) {
 }
 
 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);