Adding a BrowsingDataHelper class to hold some useful methods.

There's simply no good reason to hardcode checks against
chrome::kExtensionScheme in various bits of BrowsingData*. BrowsingDataHelper
provides a static method that we can start using whenever we need to know about
browsing-data-relevant schemes.

BUG=121636
TEST=unit_tests:BrowsingDataRemover*

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

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

7 files changed, 179 insertions, 11 deletions

diff --git a/chrome/browser/browsing_data_helper.cc b/chrome/browser/browsing_data_helper.cc
new file mode 100644
index 0000000..88d0b37
--- /dev/null
+++ b/chrome/browser/browsing_data_helper.cc
@@ -0,0 +1,30 @@
+// 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/browsing_data_helper.h"
+
+#include "base/utf_string_conversions.h"
+#include "chrome/common/url_constants.h"
+#include "content/public/browser/child_process_security_policy.h"
+#include "googleurl/src/gurl.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebString.h"
+
+// Static
+bool BrowsingDataHelper::IsValidScheme(const std::string& scheme) {
+  content::ChildProcessSecurityPolicy* policy =
+      content::ChildProcessSecurityPolicy::GetInstance();
+  return (policy->IsWebSafeScheme(scheme) &&
+          scheme != chrome::kChromeDevToolsScheme &&
+          scheme != chrome::kExtensionScheme);
+}
+
+// Static
+bool BrowsingDataHelper::IsValidScheme(const WebKit::WebString& scheme) {
+  return BrowsingDataHelper::IsValidScheme(UTF16ToUTF8(scheme));
+}
+
+// Static
+bool BrowsingDataHelper::HasValidScheme(const GURL& origin) {
+  return BrowsingDataHelper::IsValidScheme(origin.scheme());
+}
diff --git a/chrome/browser/browsing_data_helper.h b/chrome/browser/browsing_data_helper.h
new file mode 100644
index 0000000..68a3e8e
--- /dev/null
+++ b/chrome/browser/browsing_data_helper.h
@@ -0,0 +1,35 @@
+// 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.
+
+// Defines methods relevant to all code that wants to work with browsing data.
+
+#ifndef CHROME_BROWSER_BROWSING_DATA_HELPER_H_
+#define CHROME_BROWSER_BROWSING_DATA_HELPER_H_
+#pragma once
+
+#include <string>
+
+#include "base/basictypes.h"
+
+namespace WebKit {
+class WebString;
+}
+
+class GURL;
+
+class BrowsingDataHelper {
+ public:
+  // Returns true iff the provided scheme is (really) web safe, and suitable
+  // for treatment as "browsing data". This relies on the definition of web safe
+  // in ChildProcessSecurityPolicy, but excluding schemes like
+  // `chrome-extension`.
+  static bool IsValidScheme(const std::string& scheme);
+  static bool IsValidScheme(const WebKit::WebString& scheme);
+  static bool HasValidScheme(const GURL& origin);
+
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(BrowsingDataHelper);
+};
+
+#endif  // CHROME_BROWSER_BROWSING_DATA_HELPER_H_
diff --git a/chrome/browser/browsing_data_helper_unittest.cc b/chrome/browser/browsing_data_helper_unittest.cc
new file mode 100644
index 0000000..a074d2b
--- /dev/null
+++ b/chrome/browser/browsing_data_helper_unittest.cc
@@ -0,0 +1,58 @@
+// 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/browsing_data_helper.h"
+
+#include "base/stringprintf.h"
+#include "chrome/common/url_constants.h"
+#include "content/public/common/url_constants.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "googleurl/src/gurl.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebString.h"
+
+namespace {
+
+class BrowsingDataHelperTest : public testing::Test {
+ public:
+  BrowsingDataHelperTest() {}
+  virtual ~BrowsingDataHelperTest() {}
+
+  bool IsValidScheme(const std::string& scheme) {
+    GURL test(scheme + "://example.com");
+    return (BrowsingDataHelper::HasValidScheme(test) &&
+            BrowsingDataHelper::IsValidScheme(scheme) &&
+            BrowsingDataHelper::IsValidScheme(
+                WebKit::WebString::fromUTF8(scheme)));
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowsingDataHelperTest);
+};
+
+TEST_F(BrowsingDataHelperTest, WebSafeSchemesAreValid) {
+  EXPECT_TRUE(IsValidScheme(chrome::kHttpScheme));
+  EXPECT_TRUE(IsValidScheme(chrome::kHttpsScheme));
+  EXPECT_TRUE(IsValidScheme(chrome::kFtpScheme));
+  EXPECT_TRUE(IsValidScheme(chrome::kDataScheme));
+  EXPECT_TRUE(IsValidScheme("feed"));
+  EXPECT_TRUE(IsValidScheme(chrome::kBlobScheme));
+  EXPECT_TRUE(IsValidScheme(chrome::kFileSystemScheme));
+
+  EXPECT_FALSE(IsValidScheme("invalid-scheme-i-just-made-up"));
+}
+
+TEST_F(BrowsingDataHelperTest, ChromeSchemesAreInvalid) {
+  EXPECT_FALSE(IsValidScheme(chrome::kExtensionScheme));
+  EXPECT_FALSE(IsValidScheme(chrome::kAboutScheme));
+  EXPECT_FALSE(IsValidScheme(chrome::kChromeDevToolsScheme));
+  EXPECT_FALSE(IsValidScheme(chrome::kChromeInternalScheme));
+  EXPECT_FALSE(IsValidScheme(chrome::kChromeUIScheme));
+  EXPECT_FALSE(IsValidScheme(chrome::kJavaScriptScheme));
+  EXPECT_FALSE(IsValidScheme(chrome::kMailToScheme));
+  EXPECT_FALSE(IsValidScheme(chrome::kMetadataScheme));
+  EXPECT_FALSE(IsValidScheme(chrome::kSwappedOutScheme));
+  EXPECT_FALSE(IsValidScheme(chrome::kViewSourceScheme));
+}
+
+}  // namespace
diff --git a/chrome/browser/browsing_data_remover.cc b/chrome/browser/browsing_data_remover.cc
index 1fc54db..70622cf 100644
--- a/chrome/browser/browsing_data_remover.cc
+++ b/chrome/browser/browsing_data_remover.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/autofill/personal_data_manager.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/browsing_data_helper.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -614,6 +615,8 @@ void BrowsingDataRemover::OnGotQuotaManagedOrigins(
   // isn't protected.
   std::set<GURL>::const_iterator origin;
   for (origin = origins.begin(); origin != origins.end(); ++origin) {
+    if (!BrowsingDataHelper::IsValidScheme(origin->scheme()))
+      continue;
     if (special_storage_policy_->IsStorageProtected(origin->GetOrigin()))
       continue;
     if (!remove_origin_.is_empty() && remove_origin_ != origin->GetOrigin())
diff --git a/chrome/browser/browsing_data_remover_unittest.cc b/chrome/browser/browsing_data_remover_unittest.cc
index 833fd24..6df3a3c 100644
--- a/chrome/browser/browsing_data_remover_unittest.cc
+++ b/chrome/browser/browsing_data_remover_unittest.cc
@@ -40,13 +40,17 @@ using content::BrowserThread;
 
 namespace {
 
-const char kTestkOrigin1[] = "http://host1:1/";
-const char kTestkOrigin2[] = "http://host2:1/";
-const char kTestkOrigin3[] = "http://host3:1/";
-
-const GURL kOrigin1(kTestkOrigin1);
-const GURL kOrigin2(kTestkOrigin2);
-const GURL kOrigin3(kTestkOrigin3);
+const char kTestOrigin1[] = "http://host1:1/";
+const char kTestOrigin2[] = "http://host2:1/";
+const char kTestOrigin3[] = "http://host3:1/";
+const char kTestOriginExt[] = "chrome-extension://abcdefghijklmnopqrstuvwxyz/";
+const char kTestOriginDevTools[] = "chrome-devtools://abcdefghijklmnopqrstuvw/";
+
+const GURL kOrigin1(kTestOrigin1);
+const GURL kOrigin2(kTestOrigin2);
+const GURL kOrigin3(kTestOrigin3);
+const GURL kOriginExt(kTestOriginExt);
+const GURL kOriginDevTools(kTestOriginDevTools);
 
 const quota::StorageType kTemporary = quota::kStorageTypeTemporary;
 const quota::StorageType kPersistent = quota::kStorageTypePersistent;
@@ -302,6 +306,14 @@ class RemoveQuotaManagedDataTester : public BrowsingDataRemoverTester {
     PopulateTestQuotaManagedTemporaryData(manager);
   }
 
+  void PopulateTestQuotaManagedNonBrowsingData(
+      quota::MockQuotaManager* manager) {
+    manager->AddOrigin(kOriginDevTools, kTemporary, kClientFile, base::Time());
+    manager->AddOrigin(kOriginDevTools, kPersistent, kClientFile, base::Time());
+    manager->AddOrigin(kOriginExt, kTemporary, kClientFile, base::Time());
+    manager->AddOrigin(kOriginExt, kPersistent, kClientFile, base::Time());
+  }
+
   void PopulateTestQuotaManagedPersistentData(
       quota::MockQuotaManager* manager) {
     manager->AddOrigin(kOrigin2, kPersistent, kClientFile, base::Time());
@@ -515,7 +527,7 @@ TEST_F(BrowsingDataRemoverTest, RemoveServerBoundCertForever) {
   scoped_ptr<RemoveServerBoundCertTester> tester(
       new RemoveServerBoundCertTester(GetProfile()));
 
-  tester->AddServerBoundCert(kTestkOrigin1);
+  tester->AddServerBoundCert(kTestOrigin1);
   EXPECT_EQ(1, tester->ServerBoundCertCount());
 
   BlockUntilBrowsingDataRemoved(BrowsingDataRemover::EVERYTHING,
@@ -530,8 +542,8 @@ TEST_F(BrowsingDataRemoverTest, RemoveServerBoundCertLastHour) {
       new RemoveServerBoundCertTester(GetProfile()));
 
   base::Time now = base::Time::Now();
-  tester->AddServerBoundCert(kTestkOrigin1);
-  tester->AddServerBoundCertWithTimes(kTestkOrigin2,
+  tester->AddServerBoundCert(kTestOrigin1);
+  tester->AddServerBoundCertWithTimes(kTestOrigin2,
                                       now - base::TimeDelta::FromHours(2),
                                       now);
   EXPECT_EQ(2, tester->ServerBoundCertCount());
@@ -543,7 +555,7 @@ TEST_F(BrowsingDataRemoverTest, RemoveServerBoundCertLastHour) {
   EXPECT_EQ(1, tester->ServerBoundCertCount());
   std::vector<net::ServerBoundCertStore::ServerBoundCert> certs;
   tester->GetCertStore()->GetAllServerBoundCerts(&certs);
-  EXPECT_EQ(kTestkOrigin2, certs[0].server_identifier());
+  EXPECT_EQ(kTestOrigin2, certs[0].server_identifier());
 }
 
 TEST_F(BrowsingDataRemoverTest, RemoveHistoryForever) {
@@ -850,6 +862,33 @@ TEST_F(BrowsingDataRemoverTest, RemoveQuotaManagedProtectedSpecificOrigin) {
       kClientFile));
 }
 
+TEST_F(BrowsingDataRemoverTest, RemoveQuotaManagedIgnoreExtensionsAndDevTools) {
+  scoped_ptr<RemoveQuotaManagedDataTester> tester(
+      new RemoveQuotaManagedDataTester());
+  tester->PopulateTestQuotaManagedNonBrowsingData(GetMockManager());
+
+  BlockUntilBrowsingDataRemoved(BrowsingDataRemover::EVERYTHING,
+      BrowsingDataRemover::REMOVE_APPCACHE |
+      BrowsingDataRemover::REMOVE_FILE_SYSTEMS |
+      BrowsingDataRemover::REMOVE_INDEXEDDB |
+      BrowsingDataRemover::REMOVE_WEBSQL, tester.get());
+
+  EXPECT_EQ(BrowsingDataRemover::REMOVE_APPCACHE |
+      BrowsingDataRemover::REMOVE_FILE_SYSTEMS |
+      BrowsingDataRemover::REMOVE_INDEXEDDB |
+      BrowsingDataRemover::REMOVE_WEBSQL, GetRemovalMask());
+
+  // Check that extension and devtools data isn't removed.
+  EXPECT_TRUE(GetMockManager()->OriginHasData(kOriginExt, kTemporary,
+      kClientFile));
+  EXPECT_TRUE(GetMockManager()->OriginHasData(kOriginExt, kPersistent,
+      kClientFile));
+  EXPECT_TRUE(GetMockManager()->OriginHasData(kOriginDevTools, kTemporary,
+      kClientFile));
+  EXPECT_TRUE(GetMockManager()->OriginHasData(kOriginDevTools, kPersistent,
+      kClientFile));
+}
+
 TEST_F(BrowsingDataRemoverTest, OriginBasedHistoryRemoval) {
   scoped_ptr<RemoveHistoryTester> tester(
       new RemoveHistoryTester(GetProfile()));
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 9ddd5ac..fb56aee 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -335,6 +335,8 @@
         'browser/browsing_data_database_helper.h',
         'browser/browsing_data_file_system_helper.cc',
         'browser/browsing_data_file_system_helper.h',
+        'browser/browsing_data_helper.cc',
+        'browser/browsing_data_helper.h',
         'browser/browsing_data_indexed_db_helper.cc',
         'browser/browsing_data_indexed_db_helper.h',
         'browser/browsing_data_local_storage_helper.cc',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index af8fada..9535475 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1300,6 +1300,7 @@
         'browser/browsing_data_cookie_helper_unittest.cc',
         'browser/browsing_data_database_helper_unittest.cc',
         'browser/browsing_data_file_system_helper_unittest.cc',
+        'browser/browsing_data_helper_unittest.cc',
         'browser/browsing_data_indexed_db_helper_unittest.cc',
         'browser/browsing_data_local_storage_helper_unittest.cc',
         'browser/browsing_data_quota_helper_unittest.cc',