Added support for file URI path wildcards in content settings

chrome/common/content_settings_pattern.cc

Index: chrome/common/content_settings_pattern.cc
===================================================================
--- chrome/common/content_settings_pattern.cc	(revision 116653)
+++ chrome/common/content_settings_pattern.cc	(working copy)
@@ -130,9 +130,16 @@
 BuilderInterface* ContentSettingsPattern::Builder::WithPath(
     const std::string& path) {
   parts_.path = path;
+  parts_.is_path_wildcard = false;
   return this;
 }
 
+BuilderInterface* ContentSettingsPattern::Builder::WithPathWildcard() {
+  parts_.path = "";
+  parts_.is_path_wildcard = true;
+  return this;
+}
+
 BuilderInterface* ContentSettingsPattern::Builder::Invalid() {
   is_valid_ = false;
   return this;
@@ -157,10 +164,11 @@
   const std::string scheme(StringToLowerASCII(parts->scheme));
   parts->scheme = scheme;
 
-  if (parts->scheme == std::string(chrome::kFileScheme)) {
-    GURL url(std::string(chrome::kFileScheme) +
-             std::string(chrome::kStandardSchemeSeparator) + parts->path);
-    parts->path = url.path();
+  if (parts->scheme == std::string(chrome::kFileScheme) &&
+      !parts->is_path_wildcard) {
+      GURL url(std::string(chrome::kFileScheme) +
+               std::string(chrome::kStandardSchemeSeparator) + parts->path);
+      parts->path = url.path();
   }
 
   // Canonicalize the host part.
@@ -190,12 +198,16 @@
   }
 
   // file:// URL patterns have an empty host and port.
-  if (parts.scheme == std::string(chrome::kFileScheme))
-    return parts.host.empty() &&
-           parts.port.empty() &&
-           !parts.path.empty() &&
-           parts.path != std::string("/") &&
-           parts.path.find("*") == std::string::npos;
+  if (parts.scheme == std::string(chrome::kFileScheme)) {
+    if (parts.has_domain_wildcard || !parts.host.empty() || !parts.port.empty())
+      return false;
+    else if (parts.is_path_wildcard)

[Bernhard Bauer, 2012/01/09 13:23:41]

The |else| is unnecessary if you have a |return| in the if-clause body.

[Francois, 2012/01/09 14:51:03]

Done.

+      return parts.path.empty();
+    else

[Bernhard Bauer, 2012/01/09 13:23:41]

Same here.

(If you'd keep the else clause, you should put braces around the bodies, as
they're more than one line; see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Condit...)

[Francois, 2012/01/09 14:51:03]

Done.

+      return (!parts.path.empty() &&
+              parts.path != "/" &&
+              parts.path.find("*") == std::string::npos);
+  }
 
   // If the pattern is for an extension URL test if it is valid.
   if (parts.scheme == std::string(chrome::kExtensionScheme) &&
@@ -267,7 +279,8 @@
 ContentSettingsPattern::PatternParts::PatternParts()
         : is_scheme_wildcard(false),
           has_domain_wildcard(false),
-          is_port_wildcard(false) {}
+          is_port_wildcard(false),
+          is_path_wildcard(false) {}
 
 ContentSettingsPattern::PatternParts::~PatternParts() {}
 
@@ -371,7 +384,8 @@
 ContentSettingsPattern ContentSettingsPattern::Wildcard() {
   scoped_ptr<ContentSettingsPattern::BuilderInterface> builder(
       ContentSettingsPattern::CreateBuilder(true));
-  builder->WithSchemeWildcard()->WithDomainWildcard()->WithPortWildcard();
+  builder->WithSchemeWildcard()->WithDomainWildcard()->WithPortWildcard()->
+      WithPathWildcard();

[Bernhard Bauer, 2012/01/09 13:23:41]

Nit: I think it's nicer if you align this call with |WithSchemeWildcard()|
above.

[Francois, 2012/01/09 14:51:03]

Done.

   return builder->Build();
 }
 
@@ -416,7 +430,7 @@
   // one setting for all file URLs. So the path should be ignored.

[Bernhard Bauer, 2012/01/09 13:23:41]

Nit: Can you update this comment?

[Francois, 2012/01/09 14:51:03]

Done, but not sure whether the TODO needs to change as well? Are file paths
going to be supported for the foreseeable future?

[markusheintz_, 2012/01/09 15:01:00]

Please also remove the TODO completely. Thanks

[Francois, 2012/01/09 15:33:01]

Done.

   if (!parts_.is_scheme_wildcard &&
       scheme == std::string(chrome::kFileScheme)) {
-    if (parts_.path == std::string(url.path()))
+    if (parts_.is_path_wildcard || parts_.path == std::string(url.path()))

[markusheintz_, 2012/01/09 15:01:00]

Nit: You could get rid of the if. Just 
" return parts_.is_path_wildcard || parts_.path == std::string(url.path());"

[Francois, 2012/01/09 15:33:01]

Done.

       return true;
     return false;
   }