Refactoring how conditions without URL attributes are handled.

chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.h

 // 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.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_API_DECLARATIVE_WEBREQUEST_WEBREQUEST_CONDITION_H_
 #define CHROME_BROWSER_EXTENSIONS_API_DECLARATIVE_WEBREQUEST_WEBREQUEST_CONDITION_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 31 matching lines @@
 
   // Factory method that instantiates a WebRequestCondition according to
   // the description |condition| passed by the extension API.
   static scoped_ptr<WebRequestCondition> Create(
       URLMatcherConditionFactory* url_matcher_condition_factory,
       const base::Value& condition,
       std::string* error);
 
   // Returns whether the request is a match, given that the URLMatcher found
   // a match for |url_matcher_conditions_|.
   bool IsFulfilled(const WebRequestRule::RequestData& request_data) const;

[battre, 2012/12/18 08:20:44]

nit: can you add a new line here, please?

[vabr (Chromium), 2012/12/18 18:26:42]

Done.

+  // Returns true iff the request is a match and no condition attribute of this
+  // condition is a UrlFilter.
+  bool IsFulfilledIndependentlyOfURL(
+      const WebRequestRule::RequestData& request_data) const;
 
   // Returns a URLMatcherConditionSet::ID which is the canonical representation
   // for all URL patterns that need to be matched by this WebRequestCondition.
   // This ID is registered in a URLMatcher that can inform us in case of a
   // match.
   URLMatcherConditionSet::ID url_matcher_condition_set_id() const {
+    CHECK(url_matcher_conditions_.get());

[vabr (Chromium), 2012/12/13 12:18:06]

This check is needed because now it is possible to have
|url_matcher_conditions_| NULL -- that happens exactly when there are no
UrlFilter attributes in this condition. I went for CHECK instead of a graceful
exit, because if the code asks for ID in a code-path where there may be no
UrlFilter condition attributes, then something is wrong there.

[Jeffrey Yasskin, 2012/12/18 02:24:32]

I believe Chrome convention is to DCHECK in this case, because the result would
be a NULL-pointer dereference, which isn't a security bug.

[vabr (Chromium), 2012/12/18 18:26:42]

TIL, thanks! Done.

     return url_matcher_conditions_->id();
   }
 
   // Returns the set of conditions that are checked on the URL. This is the
   // primary trigger for WebRequestCondition and therefore never empty.
   // (If it was empty, the URLMatcher would never notify us about network
   // requests which might fulfill the entire WebRequestCondition).
   scoped_refptr<URLMatcherConditionSet> url_matcher_condition_set() const {
     return url_matcher_conditions_;
   }
@@ skipping 44 matching lines @@
 
   // Returns whether any condition in the condition set is fulfilled
   // based on a match |url_match| and the value of |request_data.request|.
   // This function should be called for each URLMatcherConditionSet::ID
   // that was found by the URLMatcher to ensure that the each trigger in
   // |match_triggers_| is found.
   bool IsFulfilled(
       URLMatcherConditionSet::ID url_match,
       const WebRequestRule::RequestData& request_data) const;
 
+  // Returns whether any condition in the condition set is fulfilled even when
+  // no URL match has been triggered. Same as |IsFulfilled|, but only
+  // considering conditions which don't have UrlFilter attributes.
+  bool IsFulfilledWithoutURLMatcher(
+      const WebRequestRule::RequestData& request_data) const;
+
   // Appends the URLMatcherConditionSet from all conditions to |condition_sets|.
   void GetURLMatcherConditionSets(
       URLMatcherConditionSet::Vector* condition_sets) const;
 
+  // Returns whether there are some conditions without UrlFilter attributes.
+  bool has_untriggered_conditions() const {

[Jeffrey Yasskin, 2012/12/18 02:24:32]

"untriggered" isn't a great word for this. Perhaps
"has_conditions_without_urls()"?

[vabr (Chromium), 2012/12/18 18:26:42]

Done.

+    return has_untriggered_conditions_;
+  }
+
  private:
-  Conditions conditions_;
-
   typedef std::map<URLMatcherConditionSet::ID, WebRequestCondition*>

[vabr (Chromium), 2012/12/13 12:18:06]

Typedefs should be before variables, so I reshuffled this.

       MatchTriggers;
+
+  // Returns whether there is a condition among |conditions| which has no
+  // URLMatcherConditionSet assigned.
+  static bool IsSomeConditionWithoutUrlMatcher(

[battre, 2012/12/18 08:20:44]

nit: ContainsConditionWithoutUrlMatcher?

[vabr (Chromium), 2012/12/18 18:26:42]

Already changed due to Jeffrey's comment (HasConditionWithoutUrlMatcher,
anonymous-namespace non-member function in the *.cc file).

+    const WebRequestConditionSet::Conditions& conditions);
+
   MatchTriggers match_triggers_;
 
+  const Conditions conditions_;
+  const bool has_untriggered_conditions_;
+
   DISALLOW_COPY_AND_ASSIGN(WebRequestConditionSet);
 };
 
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_DECLARATIVE_WEBREQUEST_WEBREQUEST_CONDITION_H_