Prevent duplicate webstore install requests being serviced.

webstore_private_api returns "already_installed" when an install request specifies an extension id that is either already installed or is in the process of installing.

BUG=222735

Review URL: https://chromiumcodereview.appspot.com/15292011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@203070 0039d316-1c4b-4281-b951-d872f2087c98

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

@@ -52,17 +52,20 @@ namespace extensions {
 namespace {
 
 // Holds the Approvals between the time we prompt and start the installs.
-struct PendingApprovals {
-  typedef ScopedVector<WebstoreInstaller::Approval> ApprovalList;
-
+class PendingApprovals {
+ public:
   PendingApprovals();
   ~PendingApprovals();
 
   void PushApproval(scoped_ptr<WebstoreInstaller::Approval> approval);
   scoped_ptr<WebstoreInstaller::Approval> PopApproval(
       Profile* profile, const std::string& id);
+ private:
+  typedef ScopedVector<WebstoreInstaller::Approval> ApprovalList;
+
+  ApprovalList approvals_;
 
-  ApprovalList approvals;
+  DISALLOW_COPY_AND_ASSIGN(PendingApprovals);
 };
 
 PendingApprovals::PendingApprovals() {}
@@ -70,24 +73,76 @@ PendingApprovals::~PendingApprovals() {}
 
 void PendingApprovals::PushApproval(
     scoped_ptr<WebstoreInstaller::Approval> approval) {
-  approvals.push_back(approval.release());
+  approvals_.push_back(approval.release());
 }
 
 scoped_ptr<WebstoreInstaller::Approval> PendingApprovals::PopApproval(
     Profile* profile, const std::string& id) {
-  for (size_t i = 0; i < approvals.size(); ++i) {
-    WebstoreInstaller::Approval* approval = approvals[i];
+  for (size_t i = 0; i < approvals_.size(); ++i) {
+    WebstoreInstaller::Approval* approval = approvals_[i];
     if (approval->extension_id == id &&
         profile->IsSameProfile(approval->profile)) {
-      approvals.weak_erase(approvals.begin() + i);
+      approvals_.weak_erase(approvals_.begin() + i);
       return scoped_ptr<WebstoreInstaller::Approval>(approval);
     }
   }
   return scoped_ptr<WebstoreInstaller::Approval>(NULL);
 }
 
+// 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);
+ 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);
+}
+
+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;
 
 const char kAppInstallBubbleKey[] = "appInstallBubble";
 const char kEnableLauncherKey[] = "enableLauncher";
@@ -101,7 +156,7 @@ const char kManifestKey[] = "manifest";
 // A preference set by the web store to indicate login information for
 // purchased apps.
 const char kWebstoreLogin[] = "extensions.webstore_login";
-
+const char kAlreadyInstalledError[] = "This item is already installed";
 const char kCannotSpecifyIconDataAndUrlError[] =
     "You cannot specify both icon data and an icon url";
 const char kInvalidIconUrlError[] = "Invalid icon url";
@@ -277,6 +332,15 @@ bool BeginInstallWithManifestFunction::RunImpl() {
     EXTENSION_FUNCTION_VALIDATE(details->GetBoolean(
         kEnableLauncherKey, &enable_launcher_));
 
+  ExtensionService* service =
+    extensions::ExtensionSystem::Get(profile_)->extension_service();
+  if (service->GetInstalledExtension(id_) ||
+      !g_pending_installs.Get().InsertInstall(profile_, id_)) {
+    SetResultCode(ALREADY_INSTALLED);
+    error_ = kAlreadyInstalledError;
+    return false;
+  }
+
   net::URLRequestContextGetter* context_getter = NULL;
   if (!icon_url.is_empty())
     context_getter = profile()->GetRequestContext();
@@ -326,6 +390,9 @@ void BeginInstallWithManifestFunction::SetResultCode(ResultCode code) {
     case SIGNIN_FAILED:
       SetResult(Value::CreateStringValue("signin_failed"));
       break;
+    case ALREADY_INSTALLED:
+      SetResult(Value::CreateStringValue("already_installed"));
+      break;
     default:
       CHECK(false);
   }
@@ -388,6 +455,7 @@ void BeginInstallWithManifestFunction::OnWebstoreParseFailure(
       CHECK(false);
   }
   error_ = error_message;
+  g_pending_installs.Get().EraseInstall(profile_, id);
   SendResponse(false);
 
   // Matches the AddRef in RunImpl().
@@ -402,6 +470,7 @@ void BeginInstallWithManifestFunction::SigninFailed(
 
   SetResultCode(SIGNIN_FAILED);
   error_ = error.ToString();
+  g_pending_installs.Get().EraseInstall(profile_, id_);
   SendResponse(false);
 
   // Matches the AddRef in RunImpl().
@@ -455,6 +524,7 @@ void BeginInstallWithManifestFunction::InstallUIProceed() {
 void BeginInstallWithManifestFunction::InstallUIAbort(bool user_initiated) {
   error_ = kUserCancelledError;
   SetResultCode(USER_CANCELLED);
+  g_pending_installs.Get().EraseInstall(profile_, id_);
   SendResponse(false);
 
   // The web store install histograms are a subset of the install histograms.
@@ -552,6 +622,7 @@ void CompleteInstallFunction::OnExtensionInstallSuccess(
     test_webstore_installer_delegate->OnExtensionInstallSuccess(id);
 
   LOG(INFO) << "Install success, sending response";
+  g_pending_installs.Get().EraseInstall(profile_, id);
   SendResponse(true);
 
   // Matches the AddRef in RunImpl().
@@ -572,6 +643,7 @@ void CompleteInstallFunction::OnExtensionInstallFailure(
 
   error_ = error;
   LOG(INFO) << "Install failed, sending response";
+  g_pending_installs.Get().EraseInstall(profile_, id);
   SendResponse(false);
 
   // Matches the AddRef in RunImpl().

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

@@ -106,7 +106,10 @@ class BeginInstallWithManifestFunction
     INVALID_ICON_URL,
 
     // Signin has failed.
-    SIGNIN_FAILED
+    SIGNIN_FAILED,
+
+    // An extension with the same extension id has already been installed.
+    ALREADY_INSTALLED,
   };
 
   BeginInstallWithManifestFunction();

chrome/common/extensions/api/webstore_private.json

@@ -124,7 +124,7 @@
               {
                 "name": "result",
                 "type": "string",
-                "description": "A string result code, which will be empty upon success. The possible values in the case of errors include 'unknown_error', 'user_cancelled', 'manifest_error', 'icon_error', 'invalid_id', 'permission_denied', and 'invalid_icon_url'."
+                "description": "A string result code, which will be empty upon success. The possible values in the case of errors include 'unknown_error', 'user_cancelled', 'manifest_error', 'icon_error', 'invalid_id', 'permission_denied', 'invalid_icon_url', 'signin_failed', and 'already_installed'."
               }
             ]
           }

chrome/test/data/extensions/api_test/webstore_private/accepted.js

@@ -40,17 +40,35 @@ var tests = [
 
     var manifest = getManifest();
     getIconData(function(icon) {
+        installAndCleanUp(
+            {'id': extensionId, 'iconData': icon, 'manifest': manifest},
+            function() {});
+    });
+  },
 
-      // Begin installing.
-      chrome.webstorePrivate.beginInstallWithManifest3(
-          {'id': extensionId,'iconData': icon, 'manifest': manifest },
-          function(result) {
-        assertNoLastError();
-        assertEq(result, "");
+  function duplicateInstall() {
+    // See things through all the way to a successful install.
+    listenOnce(chrome.management.onInstalled, function(info) {
+      assertEq(info.id, extensionId);
+    });
+
+    var manifest = getManifest();
+    getIconData(function(icon) {
+        installAndCleanUp(
+            {'id': extensionId, 'iconData': icon, 'manifest': manifest},
+            function() {
+              // Kick off a serial second install. This should fail.
+              var expectedError = "This item is already installed";
+              chrome.webstorePrivate.beginInstallWithManifest3(
+                  {'id': extensionId, 'iconData': icon, 'manifest': manifest},
+                  callbackFail(expectedError));
+            });
 
-        // Now complete the installation.
-        chrome.webstorePrivate.completeInstall(extensionId, callbackPass());
-      });
+        // Kick off a simultaneous second install. This should fail.
+        var expectedError = "This item is already installed";
+        chrome.webstorePrivate.beginInstallWithManifest3(
+            {'id': extensionId, 'iconData': icon, 'manifest': manifest},
+            callbackFail(expectedError));
     });
   }
 ];

chrome/test/data/extensions/api_test/webstore_private/common.js

@@ -41,6 +41,7 @@ var img = null;
 function getIconData(callback) {
   if (cachedIcon) {
     callback(cachedIcon);
+    return;
   }
   var canvas = document.createElement("canvas");
   canvas.style.display = "none";
@@ -69,3 +70,27 @@ function getManifest(alternativePath) {
   xhr.send(null);
   return xhr.responseText;
 }
+
+// Installs the extension with the given |installOptions|, calls
+// |whileInstalled| and then uninstalls the extension.
+function installAndCleanUp(installOptions, whileInstalled) {
+  // Begin installing.
+  chrome.webstorePrivate.beginInstallWithManifest3(
+      installOptions,
+      callbackPass(function(result) {
+        assertNoLastError();
+        assertEq("", result);
+
+        // Now complete the installation.
+        chrome.webstorePrivate.completeInstall(
+            extensionId,
+            callbackPass(function(result) {
+              assertNoLastError();
+              assertEq(undefined, result);
+
+              whileInstalled();
+
+              chrome.management.uninstall(extensionId, {}, callbackPass());
+            }));
+      }));
+}

chrome/test/data/extensions/api_test/webstore_private/icon_url.js

@@ -9,6 +9,16 @@ function makeAbsoluteUrl(path) {
   return parts.join("/");
 }
 
+// The |absoluteIconUrl| parameter controls whether to use a relative or
+// absolute url for the test.
+function runSuccessTest(absoluteIconUrl, manifest) {
+  var iconPath = "extension/icon.png";
+  var iconUrl = absoluteIconUrl ? makeAbsoluteUrl(iconPath) : iconPath;
+  installAndCleanUp(
+      {'id': extensionId,'iconUrl': iconUrl, 'manifest': manifest },
+      function() {});
+}
+
 var tests = [
   function IconUrlFailure() {
     var manifest = getManifest();
@@ -22,18 +32,12 @@ var tests = [
 
   function IconUrlSuccess() {
     var manifest = getManifest();
+    runSuccessTest(false, manifest);
+  },
 
-    // The |absoluteIconUrl| parameter controls whether to use a relative or
-    // absolute url for the test.
-    function runSuccessTest(absoluteIconUrl) {
-      var iconPath = "extension/icon.png";
-      var iconUrl = absoluteIconUrl ? makeAbsoluteUrl(iconPath) : iconPath;
-      chrome.webstorePrivate.beginInstallWithManifest3(
-          {'id': extensionId,'iconUrl': iconUrl, 'manifest': manifest },
-          callbackPass());
-    }
-    runSuccessTest(true);
-    runSuccessTest(false);
+  function IconUrlSuccessAbsoluteUrl() {
+    var manifest = getManifest();
+    runSuccessTest(true, manifest);
   }
 ];