Fixed leaks of WebstoreStandaloneInstallers in some code paths

chrome/browser/extensions/webstore_standalone_installer.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 "chrome/browser/extensions/webstore_standalone_installer.h"
 
 #include "base/values.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_install_prompt.h"
 #include "chrome/browser/extensions/extension_install_ui.h"
@@ skipping 194 matching lines @@
     DCHECK(!error.empty());
     CompleteInstall(error);
     return;
   }
 
   install_prompt_ = CreateInstallPrompt();
   if (install_prompt_) {
     ShowInstallUI();
     // Control flow finishes up in InstallUIProceed or InstallUIAbort.
   } else {
-    // Balanced in InstallUIAbort or indirectly in InstallUIProceed via
-    // OnExtensionInstallSuccess or OnExtensionInstallFailure.
-    AddRef();
     InstallUIProceed();
   }
 }
 
 void WebstoreStandaloneInstaller::OnWebstoreParseFailure(
     const std::string& id,
     InstallHelperResultCode result_code,
     const std::string& error_message) {
   CompleteInstall(error_message);
 }
@@ skipping 27 matching lines @@
       // enable it.
       extension_service->EnableExtension(id_);
     }  // else extension is installed and enabled; no work to be done.
 
     CompleteInstall(install_result);
     return;
   }
 
   scoped_refptr<WebstoreInstaller> installer = new WebstoreInstaller(
       profile_,
       this,

[asargent_no_longer_on_chrome, 2014/05/30 05:03:03]

Inside OnWebstoreParseSuccess, we can end up calling into InstallUIProceed here,
and it looks like we then pass a naked pointer to ourself as the delegate to
this new WebstoreInstaller. Since you removed the AddRef on line 217, doesn't
that mean that after exiting this method at line 268 that the reference count
will reach 0 and the WebstoreInstaller will be left with a stale pointer?

[tmdiep, 2014/05/30 05:34:32]

WebstoreStandaloneInstaller::BeginInstall() starts with AddRef() and I traced
all the paths of execution to CompleteInstall(). As long as use of this object
always starts with BeginInstall() and ends in CompleteInstall(), the ref count
should always be 1 in between. Let me know if I've missed something.

I can add a DCHECK(HasOneRef()) at key locations to ensure this is the case.

[tmdiep, 2014/05/30 06:25:26]

I added this dcheck before creation of the install prompt, WebstoreInstallHelper
and WebstoreInstaller, and before the Release in CompleteInstall(). It always
holds. I can't check this in though, in case clients or subclasses call
additional AddRefs and the ref count is not exactly one.

       GetWebContents(),
       id_,
       approval.Pass(),
       install_source_);
   installer->Start();
 }
 
 void WebstoreStandaloneInstaller::InstallUIAbort(bool user_initiated) {
   CompleteInstall(kUserCancelledError);
-  Release();  // Balanced in ShowInstallUI.
 }
 
 void WebstoreStandaloneInstaller::OnExtensionInstallSuccess(
     const std::string& id) {
   CHECK_EQ(id_, id);
   CompleteInstall(std::string());
-  Release();  // Balanced in ShowInstallUI.
 }
 
 void WebstoreStandaloneInstaller::OnExtensionInstallFailure(
     const std::string& id,
     const std::string& error,
     WebstoreInstaller::FailureReason cancelled) {
   CHECK_EQ(id_, id);
   CompleteInstall(error);
-  Release();  // Balanced in ShowInstallUI.
 }
 
 void WebstoreStandaloneInstaller::AbortInstall() {
   callback_.Reset();
+
   // Abort any in-progress fetches.
-  if (webstore_data_fetcher_) {
+  if (webstore_data_fetcher_)
     webstore_data_fetcher_.reset();
-    Release();  // Matches the AddRef in BeginInstall.
-  }
+
+  Release();  // Matches the AddRef in BeginInstall.
 }
 
 void WebstoreStandaloneInstaller::InvokeCallback(const std::string& error) {
   if (!callback_.is_null())
     callback_.Run(error.empty(), error);
 }
 
 void WebstoreStandaloneInstaller::CompleteInstall(const std::string& error) {
   InvokeCallback(error);
   Release();  // Matches the AddRef in BeginInstall.
 }
 
-void
-WebstoreStandaloneInstaller::ShowInstallUI() {
+void WebstoreStandaloneInstaller::ShowInstallUI() {
   std::string error;
   localized_extension_for_display_ =
       ExtensionInstallPrompt::GetLocalizedExtensionForDisplay(
           manifest_.get(),
           Extension::REQUIRE_KEY | Extension::FROM_WEBSTORE,
           id_,
           localized_name_,
           localized_description_,
           &error);
   if (!localized_extension_for_display_.get()) {
     CompleteInstall(kInvalidManifestError);
     return;
   }
 
-  // Keep this alive as long as the install prompt lives.
-  // Balanced in InstallUIAbort or indirectly in InstallUIProceed via
-  // OnExtensionInstallSuccess or OnExtensionInstallFailure.
-  AddRef();
-
   install_ui_ = CreateInstallUI();
   install_ui_->ConfirmStandaloneInstall(
       this, localized_extension_for_display_.get(), &icon_, *install_prompt_);
 }
 
 }  // namespace extensions