Fixed the browsingData API so that it now actually performs deletion of server-bound certificates.

Modified the implementation so that now it parses the arguments to the remove() general-purpose function correctly, that is, it now looks for an option named "serverBoundCertificates" instead of "serverBoundCerts", which is now in line with the JSON API definition.

As per our discussion with mkwst, I removed implementation for the removeServerBoundCerts() function (which was never defined in the JSON API), and modified the removeCookies() short-cut function to remove server-bound certificates as well (and updated the documentation accordingly). Rationale: it makes a lot more sense to remove server-bound certs along with cookies, and the remove() function still provides a way to remove them separately if need be. 

Also fixed a subtle race condition that prevented multiple concurrent browsing data removal sessions from being prevented.

BUG=152948

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

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

3 files changed, 13 insertions, 25 deletions

diff --git a/chrome/browser/extensions/api/browsing_data/browsing_data_api.cc b/chrome/browser/extensions/api/browsing_data/browsing_data_api.cc
index 1ce20ab..7545871 100644
--- a/chrome/browser/extensions/api/browsing_data/browsing_data_api.cc
+++ b/chrome/browser/extensions/api/browsing_data/browsing_data_api.cc
@@ -35,7 +35,7 @@ const char kFormDataKey[] = "formData";
 const char kHistoryKey[] = "history";
 const char kIndexedDBKey[] = "indexedDB";
 const char kLocalStorageKey[] = "localStorage";
-const char kServerBoundCertsKey[] = "serverBoundCerts";
+const char kServerBoundCertsKey[] = "serverBoundCertificates";
 const char kPasswordsKey[] = "passwords";
 const char kPluginDataKey[] = "pluginData";
 const char kWebSQLKey[] = "webSQL";
@@ -113,11 +113,6 @@ bool BrowsingDataExtensionFunction::RunImpl() {
   // If we don't have a profile, something's pretty wrong.
   DCHECK(profile());
 
-  if (BrowsingDataRemover::is_removing()) {
-    error_ = extension_browsing_data_api_constants::kOneAtATimeError;
-    return false;
-  }
-
   // Grab the initial |options| parameter, and parse out the arguments.
   DictionaryValue* options;
   EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &options));
@@ -169,6 +164,12 @@ void BrowsingDataExtensionFunction::CheckRemovingPluginDataSupported(
 }
 
 void BrowsingDataExtensionFunction::StartRemoving() {
+  if (BrowsingDataRemover::is_removing()) {
+    error_ = extension_browsing_data_api_constants::kOneAtATimeError;
+    SendResponse(false);
+    return;
+  }
+
   // If we're good to go, add a ref (Balanced in OnBrowsingDataRemoverDone)
   AddRef();
 
@@ -240,7 +241,8 @@ int RemoveCacheFunction::GetRemovalMask() const {
 }
 
 int RemoveCookiesFunction::GetRemovalMask() const {
-  return BrowsingDataRemover::REMOVE_COOKIES;
+  return BrowsingDataRemover::REMOVE_COOKIES |
+         BrowsingDataRemover::REMOVE_SERVER_BOUND_CERTS;
 }
 
 int RemoveDownloadsFunction::GetRemovalMask() const {
@@ -267,10 +269,6 @@ int RemoveLocalStorageFunction::GetRemovalMask() const {
   return BrowsingDataRemover::REMOVE_LOCAL_STORAGE;
 }
 
-int RemoveServerBoundCertsFunction::GetRemovalMask() const {
-  return BrowsingDataRemover::REMOVE_SERVER_BOUND_CERTS;
-}
-
 int RemovePluginDataFunction::GetRemovalMask() const {
   return BrowsingDataRemover::REMOVE_PLUGIN_DATA;
 }
diff --git a/chrome/browser/extensions/api/browsing_data/browsing_data_api.h b/chrome/browser/extensions/api/browsing_data/browsing_data_api.h
index 59074fa..458ae98 100644
--- a/chrome/browser/extensions/api/browsing_data/browsing_data_api.h
+++ b/chrome/browser/extensions/api/browsing_data/browsing_data_api.h
@@ -195,17 +195,6 @@ class RemoveLocalStorageFunction : public BrowsingDataExtensionFunction {
   virtual int GetRemovalMask() const OVERRIDE;
 };
 
-class RemoveServerBoundCertsFunction : public BrowsingDataExtensionFunction {
- public:
-  DECLARE_EXTENSION_FUNCTION_NAME("browsingData.removeServerBoundCertificates")
-
- protected:
-  virtual ~RemoveServerBoundCertsFunction() {}
-
-  // BrowsingDataExtensionFunction:
-  virtual int GetRemovalMask() const OVERRIDE;
-};
-
 class RemovePluginDataFunction : public BrowsingDataExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION_NAME("browsingData.removePluginData")
diff --git a/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc b/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc
index e007b31..69aa1d8 100644
--- a/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc
+++ b/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc
@@ -29,8 +29,8 @@ const char kRemoveEverythingArguments[] = "[{\"since\": 1000}, {"
     "\"appcache\": true, \"cache\": true, \"cookies\": true, "
     "\"downloads\": true, \"fileSystems\": true, \"formData\": true, "
     "\"history\": true, \"indexedDB\": true, \"localStorage\": true, "
-    "\"serverBoundCerts\": true, \"passwords\": true, \"pluginData\": true, "
-    "\"webSQL\": true"
+    "\"serverBoundCertificates\": true, \"passwords\": true, "
+    "\"pluginData\": true, \"webSQL\": true"
     "}]";
 
 class ExtensionBrowsingDataTest : public InProcessBrowserTest,
@@ -188,7 +188,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowsingDataTest, BrowsingDataRemovalMask) {
   RunRemoveBrowsingDataFunctionAndCompareRemovalMask(
       "localStorage", BrowsingDataRemover::REMOVE_LOCAL_STORAGE);
   RunRemoveBrowsingDataFunctionAndCompareRemovalMask(
-      "serverBoundCerts", BrowsingDataRemover::REMOVE_SERVER_BOUND_CERTS);
+      "serverBoundCertificates",
+      BrowsingDataRemover::REMOVE_SERVER_BOUND_CERTS);
   RunRemoveBrowsingDataFunctionAndCompareRemovalMask(
       "passwords", BrowsingDataRemover::REMOVE_PASSWORDS);
   // We can't remove plugin data inside a test profile.