Commit e263a6b6 authored by tmdiep@chromium.org's avatar tmdiep@chromium.org

Fixed leaks of WebstoreStandaloneInstallers in some code paths

Simplifies the sequence of AddRef() and Release() to avoid leaks
due to unbalanced Releases.

BUG=378606
TEST=browser_tests
Manual: Test installing apps via the app launcher and inline
installation to ensure there are no regressions.

Review URL: https://codereview.chromium.org/298133007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275270 0039d316-1c4b-4281-b951-d872f2087c98
parent 7131920e
...@@ -98,11 +98,14 @@ WebstoreStandaloneInstaller::CreateApproval() const { ...@@ -98,11 +98,14 @@ WebstoreStandaloneInstaller::CreateApproval() const {
} }
void WebstoreStandaloneInstaller::OnWebstoreRequestFailure() { void WebstoreStandaloneInstaller::OnWebstoreRequestFailure() {
OnWebStoreDataFetcherDone();
CompleteInstall(kWebstoreRequestError); CompleteInstall(kWebstoreRequestError);
} }
void WebstoreStandaloneInstaller::OnWebstoreResponseParseSuccess( void WebstoreStandaloneInstaller::OnWebstoreResponseParseSuccess(
scoped_ptr<base::DictionaryValue> webstore_data) { scoped_ptr<base::DictionaryValue> webstore_data) {
OnWebStoreDataFetcherDone();
if (!CheckRequestorAlive()) { if (!CheckRequestorAlive()) {
CompleteInstall(std::string()); CompleteInstall(std::string());
return; return;
...@@ -183,6 +186,7 @@ void WebstoreStandaloneInstaller::OnWebstoreResponseParseSuccess( ...@@ -183,6 +186,7 @@ void WebstoreStandaloneInstaller::OnWebstoreResponseParseSuccess(
void WebstoreStandaloneInstaller::OnWebstoreResponseParseFailure( void WebstoreStandaloneInstaller::OnWebstoreResponseParseFailure(
const std::string& error) { const std::string& error) {
OnWebStoreDataFetcherDone();
CompleteInstall(error); CompleteInstall(error);
} }
...@@ -212,9 +216,6 @@ void WebstoreStandaloneInstaller::OnWebstoreParseSuccess( ...@@ -212,9 +216,6 @@ void WebstoreStandaloneInstaller::OnWebstoreParseSuccess(
ShowInstallUI(); ShowInstallUI();
// Control flow finishes up in InstallUIProceed or InstallUIAbort. // Control flow finishes up in InstallUIProceed or InstallUIAbort.
} else { } else {
// Balanced in InstallUIAbort or indirectly in InstallUIProceed via
// OnExtensionInstallSuccess or OnExtensionInstallFailure.
AddRef();
InstallUIProceed(); InstallUIProceed();
} }
} }
...@@ -272,14 +273,12 @@ void WebstoreStandaloneInstaller::InstallUIProceed() { ...@@ -272,14 +273,12 @@ void WebstoreStandaloneInstaller::InstallUIProceed() {
void WebstoreStandaloneInstaller::InstallUIAbort(bool user_initiated) { void WebstoreStandaloneInstaller::InstallUIAbort(bool user_initiated) {
CompleteInstall(kUserCancelledError); CompleteInstall(kUserCancelledError);
Release(); // Balanced in ShowInstallUI.
} }
void WebstoreStandaloneInstaller::OnExtensionInstallSuccess( void WebstoreStandaloneInstaller::OnExtensionInstallSuccess(
const std::string& id) { const std::string& id) {
CHECK_EQ(id_, id); CHECK_EQ(id_, id);
CompleteInstall(std::string()); CompleteInstall(std::string());
Release(); // Balanced in ShowInstallUI.
} }
void WebstoreStandaloneInstaller::OnExtensionInstallFailure( void WebstoreStandaloneInstaller::OnExtensionInstallFailure(
...@@ -288,7 +287,6 @@ void WebstoreStandaloneInstaller::OnExtensionInstallFailure( ...@@ -288,7 +287,6 @@ void WebstoreStandaloneInstaller::OnExtensionInstallFailure(
WebstoreInstaller::FailureReason cancelled) { WebstoreInstaller::FailureReason cancelled) {
CHECK_EQ(id_, id); CHECK_EQ(id_, id);
CompleteInstall(error); CompleteInstall(error);
Release(); // Balanced in ShowInstallUI.
} }
void WebstoreStandaloneInstaller::AbortInstall() { void WebstoreStandaloneInstaller::AbortInstall() {
...@@ -310,8 +308,7 @@ void WebstoreStandaloneInstaller::CompleteInstall(const std::string& error) { ...@@ -310,8 +308,7 @@ void WebstoreStandaloneInstaller::CompleteInstall(const std::string& error) {
Release(); // Matches the AddRef in BeginInstall. Release(); // Matches the AddRef in BeginInstall.
} }
void void WebstoreStandaloneInstaller::ShowInstallUI() {
WebstoreStandaloneInstaller::ShowInstallUI() {
std::string error; std::string error;
localized_extension_for_display_ = localized_extension_for_display_ =
ExtensionInstallPrompt::GetLocalizedExtensionForDisplay( ExtensionInstallPrompt::GetLocalizedExtensionForDisplay(
...@@ -326,14 +323,18 @@ WebstoreStandaloneInstaller::ShowInstallUI() { ...@@ -326,14 +323,18 @@ WebstoreStandaloneInstaller::ShowInstallUI() {
return; 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_ = CreateInstallUI();
install_ui_->ConfirmStandaloneInstall( install_ui_->ConfirmStandaloneInstall(
this, localized_extension_for_display_.get(), &icon_, *install_prompt_); this, localized_extension_for_display_.get(), &icon_, *install_prompt_);
} }
void WebstoreStandaloneInstaller::OnWebStoreDataFetcherDone() {
// An instance of this class is passed in as a delegate for the
// WebstoreInstallHelper, ExtensionInstallPrompt and WebstoreInstaller, and
// therefore needs to remain alive until they are done. Clear the webstore
// data fetcher to avoid calling Release in AbortInstall while any of these
// operations are in progress.
webstore_data_fetcher_.reset();
}
} // namespace extensions } // namespace extensions
...@@ -186,6 +186,7 @@ class WebstoreStandaloneInstaller ...@@ -186,6 +186,7 @@ class WebstoreStandaloneInstaller
WebstoreInstaller::FailureReason reason) OVERRIDE; WebstoreInstaller::FailureReason reason) OVERRIDE;
void ShowInstallUI(); void ShowInstallUI();
void OnWebStoreDataFetcherDone();
// Input configuration. // Input configuration.
std::string id_; std::string id_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment