Fix issues related to <all_urls> in extensions.

chrome/common/extensions/url_pattern.cc

 // Copyright (c) 2011 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/common/extensions/url_pattern.h"
 
 #include "base/string_number_conversions.h"
 #include "base/string_piece.h"
 #include "base/string_split.h"
 #include "base/string_util.h"
@@ skipping 120 matching lines @@
 bool URLPattern::operator==(const URLPattern& other) const {
   return GetAsString() == other.GetAsString();
 }
 
 URLPattern::ParseResult URLPattern::Parse(const std::string& pattern,
                                           ParseOption strictness) {
   spec_.clear();
 
   // Special case pattern to match every valid URL.
   if (pattern == kAllUrlsPattern) {
-    match_all_urls_ = true;
-    match_subdomains_ = true;
-    scheme_ = "*";
-    host_.clear();
-    SetPath("/*");
+    SetMatchAllURLs(true);
     return PARSE_SUCCESS;
   }
 
   // Parse out the scheme.
   size_t scheme_end_pos = pattern.find(chrome::kStandardSchemeSeparator);
   bool has_standard_scheme_separator = true;
 
   // Some urls also use ':' alone as the scheme separator.
   if (scheme_end_pos == std::string::npos) {
     scheme_end_pos = pattern.find(':');
@@ skipping 94 matching lines @@
 }
 
 void URLPattern::SetHost(const std::string& host) {
   spec_.clear();
   host_ = host;
 }
 
 void URLPattern::SetMatchAllURLs(bool val) {
   spec_.clear();
   match_all_urls_ = val;
+
+  if (val) {
+    match_subdomains_ = true;
+    scheme_ = "*";
+    host_.clear();
+    SetPath("/*");
+  }
 }

[jstritar, 2011/10/10 20:23:24]

This was the real bug- the <all_urls> URLPatterns were different depending on
how you create them. This method feels strange. What should happen to the other
variables if calling it with false? I wonder if URLPattern should be immutable,
or make SetMatchAllURLs private? 

[Matt Perry, 2011/10/10 23:22:39]

Hmm.. I think the real bug is that the caller didn't call Parse. Without calling
Parse(), the URLPattern is in an invalid state. The caller should be changed to
do URLPattern pattern(SCHEMES_ALL, kAllUrlsPattern);

I think your change here is good, though. Toggling match_all_urls_ off should
leave the rest of the state unchanged (since this stuff would be set in a
particular state if match_all_urls_ were true anyway).

[jstritar, 2011/10/11 15:13:38]

Ah, I see... you're supposed to always call Parse. I updated
ExtensionPermissionSet::InitEffectiveHosts to initialize based on
kAllUrlsPattern.

 
 void URLPattern::SetMatchSubdomains(bool val) {
   spec_.clear();
   match_subdomains_ = val;
 }
 
 bool URLPattern::SetScheme(const std::string& scheme) {
   spec_.clear();
   scheme_ = scheme;
   if (scheme_ == "*") {
@@ skipping 219 matching lines @@
   }
 
   return result;
 }
 
 // static
 const char* URLPattern::GetParseResultString(
     URLPattern::ParseResult parse_result) {
   return kParseResultMessages[parse_result];
 }