Prevent duplicate concurrent installs of the same extension

chrome/browser/extensions/api/webstore_private/webstore_private_api.cc

Index: chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
diff --git a/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc b/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
index ed224121f0d8b55f293f3558542bdd04d869458f..1a16a47b0784e383d03ce4edd2d86a7625727654 100644
--- a/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
+++ b/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
@@ -21,6 +21,7 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/install_tracker.h"
 #include "chrome/browser/extensions/webstore_installer.h"
 #include "chrome/browser/gpu/gpu_feature_checker.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -116,65 +117,8 @@ scoped_ptr<WebstoreInstaller::Approval> PendingApprovals::PopApproval(
   return scoped_ptr<WebstoreInstaller::Approval>();
 }
 
-// Uniquely holds the profile and extension id of an install between the time we
-// prompt and complete the installs.
-class PendingInstalls {
- public:
-  PendingInstalls();
-  ~PendingInstalls();
-
-  bool InsertInstall(Profile* profile, const std::string& id);
-  void EraseInstall(Profile* profile, const std::string& id);
-  bool ContainsInstall(Profile* profile, const std::string& id);
- private:
-  typedef std::pair<Profile*, std::string> ProfileAndExtensionId;
-  typedef std::vector<ProfileAndExtensionId> InstallList;
-
-  InstallList::iterator FindInstall(Profile* profile, const std::string& id);
-
-  InstallList installs_;
-
-  DISALLOW_COPY_AND_ASSIGN(PendingInstalls);
-};
-
-PendingInstalls::PendingInstalls() {}
-PendingInstalls::~PendingInstalls() {}
-
-// Returns true and inserts the profile/id pair if it is not present. Otherwise
-// returns false.
-bool PendingInstalls::InsertInstall(Profile* profile, const std::string& id) {
-  if (FindInstall(profile, id) != installs_.end())
-    return false;
-  installs_.push_back(make_pair(profile, id));
-  return true;
-}
-
-// Removes the given profile/id pair.
-void PendingInstalls::EraseInstall(Profile* profile, const std::string& id) {
-  InstallList::iterator it = FindInstall(profile, id);
-  if (it != installs_.end())
-    installs_.erase(it);
-}
-
-bool PendingInstalls::ContainsInstall(Profile* profile, const std::string& id) {
-  return FindInstall(profile, id) != installs_.end();
-}
-
-PendingInstalls::InstallList::iterator PendingInstalls::FindInstall(
-    Profile* profile,
-    const std::string& id) {
-  for (size_t i = 0; i < installs_.size(); ++i) {
-    ProfileAndExtensionId install = installs_[i];
-    if (install.second == id && profile->IsSameProfile(install.first))
-      return (installs_.begin() + i);
-  }
-  return installs_.end();
-}
-
 static base::LazyInstance<PendingApprovals> g_pending_approvals =
     LAZY_INSTANCE_INITIALIZER;
-static base::LazyInstance<PendingInstalls> g_pending_installs =
-    LAZY_INSTANCE_INITIALIZER;
 
 // A preference set by the web store to indicate login information for
 // purchased apps.
@@ -210,6 +154,12 @@ void RecordWebstoreExtensionInstallResult(bool success) {
   UMA_HISTOGRAM_BOOLEAN("Webstore.ExtensionInstallResult", success);
 }
 
+void RemoveActiveInstall(Profile* profile, const std::string& extension_id) {
+  InstallTracker* tracker = InstallTracker::Get(profile);
+  DCHECK(tracker);
+  tracker->RemoveActiveInstall(extension_id);
+}
+
 }  // namespace
 
 // static
@@ -326,12 +276,16 @@ bool WebstorePrivateBeginInstallWithManifest3Function::RunAsync() {
       *params_->details.icon_data : std::string();
 
   Profile* profile = GetProfile();
+  InstallTracker* tracker = InstallTracker::Get(profile);
+  DCHECK(tracker);
   if (util::IsExtensionInstalledPermanently(params_->details.id, profile) ||
-      !g_pending_installs.Get().InsertInstall(profile, params_->details.id)) {
+      tracker->GetActiveInstall(params_->details.id)) {
     SetResultCode(ALREADY_INSTALLED);
     error_ = kAlreadyInstalledError;
     return false;
   }
+  InstallTracker::InstallProgressData install_data(params_->details.id);
+  tracker->AddActiveInstall(install_data);
 
   net::URLRequestContextGetter* context_getter = NULL;
   if (!icon_url.is_empty())
@@ -450,7 +404,7 @@ void WebstorePrivateBeginInstallWithManifest3Function::OnWebstoreParseFailure(
       CHECK(false);
   }
   error_ = error_message;
-  g_pending_installs.Get().EraseInstall(GetProfile(), id);
+  RemoveActiveInstall(GetProfile(), id);
   SendResponse(false);
 
   // Matches the AddRef in RunAsync().
@@ -463,7 +417,7 @@ void WebstorePrivateBeginInstallWithManifest3Function::SigninFailed(
 
   SetResultCode(SIGNIN_FAILED);
   error_ = error.ToString();
-  g_pending_installs.Get().EraseInstall(GetProfile(), params_->details.id);
+  RemoveActiveInstall(GetProfile(), params_->details.id);

[asargent_no_longer_on_chrome, 2014/07/16 18:49:07]

It would be cool if we had something like a "ScopedActiveInstall" object for the
install tracker, so that we wouldn't have to remember to call
RemoveActiveInstall everywhere - it would automatically get removed when the
ScopedActiveInstall was deleted. 

[tmdiep, 2014/07/17 01:01:51]

That's a great idea. Done.

The WebstorePrivateAPI install is split between beginInstall and
completeInstall, so that explains ScopedActiveInstall::CancelDeregister().

   SendResponse(false);
 
   // Matches the AddRef in RunAsync().
@@ -531,7 +485,7 @@ void WebstorePrivateBeginInstallWithManifest3Function::InstallUIAbort(
     bool user_initiated) {
   error_ = kUserCancelledError;
   SetResultCode(USER_CANCELLED);
-  g_pending_installs.Get().EraseInstall(GetProfile(), params_->details.id);
+  RemoveActiveInstall(GetProfile(), params_->details.id);
   SendResponse(false);
 
   // The web store install histograms are a subset of the install histograms.
@@ -646,7 +600,7 @@ void WebstorePrivateCompleteInstallFunction::OnExtensionInstallFailure(
 
   error_ = error;
   VLOG(1) << "Install failed, sending response";
-  g_pending_installs.Get().EraseInstall(GetProfile(), id);
+  RemoveActiveInstall(GetProfile(), id);
   SendResponse(false);
 
   RecordWebstoreExtensionInstallResult(false);
@@ -661,7 +615,7 @@ void WebstorePrivateCompleteInstallFunction::OnInstallSuccess(
     test_webstore_installer_delegate->OnExtensionInstallSuccess(id);
 
   VLOG(1) << "Install success, sending response";
-  g_pending_installs.Get().EraseInstall(GetProfile(), id);
+  RemoveActiveInstall(GetProfile(), id);
   SendResponse(true);
 }
 
@@ -856,13 +810,6 @@ bool WebstorePrivateLaunchEphemeralAppFunction::RunAsync() {
       LaunchEphemeralApp::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params);
 
-  // If a full install is in progress, do not install ephemerally.
-  if (g_pending_installs.Get().ContainsInstall(GetProfile(), params->id)) {
-    SetResult(LaunchEphemeralAppResult::RESULT_INSTALL_IN_PROGRESS,
-              "An install is already in progress");
-    return false;
-  }
-
   AddRef();  // Balanced in OnLaunchComplete()
 
   scoped_refptr<EphemeralAppLauncher> launcher =
@@ -919,6 +866,12 @@ void WebstorePrivateLaunchEphemeralAppFunction::OnLaunchComplete(
     case webstore_install::LAUNCH_UNSUPPORTED_EXTENSION_TYPE:
       api_result = LaunchEphemeralAppResult::RESULT_UNSUPPORTED_EXTENSION_TYPE;
       break;
+    case webstore_install::INSTALL_IN_PROGRESS:
+      api_result = LaunchEphemeralAppResult::RESULT_INSTALL_IN_PROGRESS;
+      break;
+    case webstore_install::LAUNCH_IN_PROGRESS:
+      api_result = LaunchEphemeralAppResult::RESULT_LAUNCH_IN_PROGRESS;
+      break;
     default:
       NOTREACHED();
       break;