Add transparent support for filesystem URLs in URLPatterns.

Given that we don't want to add in filesystem-specific patterns, the simplest approach seems to be just to allow all patterns to match either outer or inner URLs.

BUG=none
TEST=unit tests


Review URL: http://codereview.chromium.org/10224011

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

4 files changed, 21 insertions, 40 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index c26f225..0c54249 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -2459,7 +2459,6 @@ FileBrowserHandler* Extension::LoadFileBrowserHandler(
       // wildcards in URLPattern, so transform to what will match correctly.
       filter.replace(0, 11, "chrome-extension://*/");
       URLPattern pattern(URLPattern::SCHEME_EXTENSION);
-      pattern.set_partial_filesystem_support_hack(true);
       if (pattern.Parse(filter) != URLPattern::PARSE_SUCCESS) {
         *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
             errors::kInvalidURLPatternError, filter);
diff --git a/chrome/common/extensions/url_pattern.cc b/chrome/common/extensions/url_pattern.cc
index 94bb877..c66e780 100644
--- a/chrome/common/extensions/url_pattern.cc
+++ b/chrome/common/extensions/url_pattern.cc
@@ -97,14 +97,12 @@ bool IsValidPortForScheme(const std::string scheme, const std::string& port) {
 URLPattern::URLPattern()
     : valid_schemes_(SCHEME_NONE),
       match_all_urls_(false),
-      partial_filesystem_support_hack_(false),
       match_subdomains_(false),
       port_("*") {}
 
 URLPattern::URLPattern(int valid_schemes)
     : valid_schemes_(valid_schemes),
       match_all_urls_(false),
-      partial_filesystem_support_hack_(false),
       match_subdomains_(false),
       port_("*") {}
 
@@ -113,7 +111,6 @@ URLPattern::URLPattern(int valid_schemes, const std::string& pattern)
     // appropriate when we know |pattern| is valid.
     : valid_schemes_(valid_schemes),
       match_all_urls_(false),
-      partial_filesystem_support_hack_(false),
       match_subdomains_(false),
       port_("*") {
   if (PARSE_SUCCESS != Parse(pattern))
@@ -301,11 +298,11 @@ bool URLPattern::MatchesURL(const GURL& test) const {
   const GURL* test_url = &test;
   bool has_inner_url = test.inner_url() != NULL;
 
-  if (partial_filesystem_support_hack_ != has_inner_url)
-    return false;
-
-  if (has_inner_url)
+  if (has_inner_url) {
+    if (!test.SchemeIsFileSystem())
+      return false;  // The only nested URLs we handle are filesystem URLs.
     test_url = test.inner_url();
+  }
 
   if (!MatchesScheme(test_url->scheme()))
     return false;
@@ -322,13 +319,22 @@ bool URLPattern::MatchesURL(const GURL& test) const {
 }
 
 bool URLPattern::MatchesSecurityOrigin(const GURL& test) const {
-  if (!MatchesScheme(test.scheme()))
+  const GURL* test_url = &test;
+  bool has_inner_url = test.inner_url() != NULL;
+
+  if (has_inner_url) {
+    if (!test.SchemeIsFileSystem())
+      return false;  // The only nested URLs we handle are filesystem URLs.
+    test_url = test.inner_url();
+  }
+
+  if (!MatchesScheme(test_url->scheme()))
     return false;
 
   if (match_all_urls_)
     return true;
 
-  return MatchesSecurityOriginHelper(test);
+  return MatchesSecurityOriginHelper(*test_url);
 }
 
 bool URLPattern::MatchesScheme(const std::string& test) const {
@@ -449,10 +455,6 @@ bool URLPattern::OverlapsWith(const URLPattern& other) const {
   DCHECK(path_.find('*') == path_.size() - 1);
   DCHECK(other.path().find('*') == other.path().size() - 1);
 
-  if (partial_filesystem_support_hack_ !=
-      other.partial_filesystem_support_hack())
-    return false;
-
   if (!MatchesPath(other.path().substr(0, other.path().size() - 1)) &&
       !other.MatchesPath(path_.substr(0, path_.size() - 1)))
     return false;
diff --git a/chrome/common/extensions/url_pattern.h b/chrome/common/extensions/url_pattern.h
index 3cde957..ace2dc8 100644
--- a/chrome/common/extensions/url_pattern.h
+++ b/chrome/common/extensions/url_pattern.h
@@ -119,15 +119,6 @@ class URLPattern {
   bool match_all_urls() const { return match_all_urls_; }
   void SetMatchAllURLs(bool val);
 
-  // Returns true if this pattern matches inner URLs of filesystem: URLs only.
-  // Returns false if this pattern matches only non-filesystem URLs.
-  bool partial_filesystem_support_hack() const {
-    return partial_filesystem_support_hack_;
-  }
-  void set_partial_filesystem_support_hack(bool val) {
-    partial_filesystem_support_hack_ = val;
-  }
-
   // Sets the scheme for pattern matches. This can be a single '*' if the
   // pattern matches all valid schemes (as defined by the valid_schemes_
   // property). Returns false on failure (if the scheme is not valid).
@@ -147,6 +138,9 @@ class URLPattern {
   bool MatchesSecurityOrigin(const GURL& test) const;
 
   // Returns true if |test| matches our scheme.
+  // Note that if test is "filesystem", this may fail whereas MatchesURL
+  // may succeed.  MatchesURL is smart enough to look at the inner_url instead
+  // of the outer "filesystem:" part.
   bool MatchesScheme(const std::string& test) const;
 
   // Returns true if |test| matches our host.
@@ -213,11 +207,6 @@ class URLPattern {
   // True if this is a special-case "<all_urls>" pattern.
   bool match_all_urls_;
 
-  // True if we're trying to match against the inner URL of a filesystem URL;
-  // this is a temporary hack so as not to break ChromeOS as we work on full
-  // support.
-  bool partial_filesystem_support_hack_;
-
   // The scheme for the pattern.
   std::string scheme_;
 
diff --git a/chrome/common/extensions/url_pattern_unittest.cc b/chrome/common/extensions/url_pattern_unittest.cc
index 10dff3d..d19d72d 100644
--- a/chrome/common/extensions/url_pattern_unittest.cc
+++ b/chrome/common/extensions/url_pattern_unittest.cc
@@ -110,11 +110,8 @@ TEST(ExtensionURLPatternTest, Match2) {
   EXPECT_TRUE(pattern.MatchesURL(GURL("https://www.google.com/foobar")));
   EXPECT_FALSE(pattern.MatchesURL(GURL("http://www.google.com/foo")));
   EXPECT_FALSE(pattern.MatchesURL(GURL("https://www.google.com/")));
-  EXPECT_FALSE(pattern.MatchesURL(
-      GURL("filesystem:https://www.google.com/foobar/")));
-  pattern.set_partial_filesystem_support_hack(true);
   EXPECT_TRUE(pattern.MatchesURL(
-      GURL("filesystem:https://www.google.com/foobar/bas")));
+      GURL("filesystem:https://www.google.com/foobar/")));
 }
 
 // subdomains
@@ -132,9 +129,8 @@ TEST(URLPatternTest, Match3) {
   EXPECT_TRUE(pattern.MatchesURL(
       GURL("http://monkey.images.google.com/foooobar")));
   EXPECT_FALSE(pattern.MatchesURL(GURL("http://yahoo.com/foobar")));
-  EXPECT_FALSE(pattern.MatchesURL(
-      GURL("filesystem:http://google.com/foobar/")));
-  pattern.set_partial_filesystem_support_hack(true);
+  EXPECT_TRUE(pattern.MatchesURL(
+      GURL("filesystem:http://google.com/foo/bar")));
   EXPECT_FALSE(pattern.MatchesURL(
       GURL("filesystem:http://google.com/temporary/foobar")));
 }
@@ -383,8 +379,6 @@ TEST(ExtensionURLPatternTest, Match17) {
   EXPECT_FALSE(pattern.MatchesURL(
       GURL("filesystem:http://www.example.com:8080/foo/")));
   EXPECT_FALSE(pattern.MatchesURL(GURL("filesystem:http://www.example.com/f/foo")));
-  pattern.set_partial_filesystem_support_hack(true);
-  EXPECT_FALSE(pattern.MatchesURL(GURL("filesystem:http://www.example.com/f/foo")));
 }
 
 // Explicit port wildcard
@@ -421,9 +415,6 @@ TEST(ExtensionURLPatternTest, Match19) {
   EXPECT_TRUE(pattern.MatchesURL(
       GURL("chrome-extension://ftw/https://google.com")));
   EXPECT_FALSE(pattern.MatchesURL(GURL("chrome-extension://foobar")));
-  EXPECT_FALSE(pattern.MatchesURL(
-      GURL("filesystem:chrome-extension://ftw/t/file.txt")));
-  pattern.set_partial_filesystem_support_hack(true);
   EXPECT_TRUE(pattern.MatchesURL(
       GURL("filesystem:chrome-extension://ftw/t/file.txt")));
 };