[DNR] Return RequestAction for block/redirect rules in RulesetMatcher

This is CL 1/2 which moves the construction of RequestAction from
RulesetManager to RulesetMatcher. This is done in order to attribute
RequestActions to the matched rule it was constructed from.

Bug: 1009887
Change-Id: Ife65eec7b9a81b589753eeac7b4a712a300ff262
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1834561
Commit-Queue: Kelvin Jiang <kelvinjiang@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705621}

extensions/browser/api/declarative_net_request/composite_matcher.cc

@@ -10,13 +10,14 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "extensions/browser/api/declarative_net_request/flat/extension_ruleset_generated.h"
+#include "extensions/browser/api/declarative_net_request/request_action.h"
 #include "extensions/browser/api/declarative_net_request/utils.h"
 
 namespace extensions {
 namespace declarative_net_request {
 namespace flat_rule = url_pattern_index::flat;
 using PageAccess = PermissionsData::PageAccess;
-using RedirectAction = CompositeMatcher::RedirectAction;
+using RedirectActionInfo = CompositeMatcher::RedirectActionInfo;
 
 namespace {
 
@@ -50,45 +51,21 @@ bool HasMatchingAllowRule(const RulesetMatcher* matcher,
   return params.allow_rule_cache[matcher];
 }
 
-// Upgrades the url's scheme to HTTPS.
-GURL GetUpgradedUrl(const GURL& url) {
-  DCHECK(url.SchemeIs(url::kHttpScheme) || url.SchemeIs(url::kFtpScheme));
-
-  GURL::Replacements replacements;
-  replacements.SetSchemeStr(url::kHttpsScheme);
-  return url.ReplaceComponents(replacements);
-}
-
-// Compares |redirect_rule| and |upgrade_rule| and determines the redirect URL
-// based on the rule with the higher priority.
-GURL GetUrlByRulePriority(const flat_rule::UrlRule* redirect_rule,
-                          const flat_rule::UrlRule* upgrade_rule,
-                          const GURL& request_url,
-                          GURL redirect_rule_url) {
-  DCHECK(upgrade_rule || redirect_rule);
-
-  if (!upgrade_rule)
-    return redirect_rule_url;
-
-  if (!redirect_rule)
-    return GetUpgradedUrl(request_url);
-
-  return upgrade_rule->priority() > redirect_rule->priority()
-             ? GetUpgradedUrl(request_url)
-             : redirect_rule_url;
-}
-
 }  // namespace
 
-RedirectAction::RedirectAction(base::Optional<GURL> redirect_url,
-                               bool notify_request_withheld)
-    : redirect_url(std::move(redirect_url)),
-      notify_request_withheld(notify_request_withheld) {}
+RedirectActionInfo::RedirectActionInfo(base::Optional<RequestAction> action,
+                                       bool notify_request_withheld)
+    : action(std::move(action)),
+      notify_request_withheld(notify_request_withheld) {
+  if (action)
+    DCHECK_EQ(RequestAction::Type::REDIRECT, action->type);
+}
 
-RedirectAction::~RedirectAction() = default;
+RedirectActionInfo::~RedirectActionInfo() = default;
 
-RedirectAction::RedirectAction(RedirectAction&&) = default;
-RedirectAction& RedirectAction::operator=(RedirectAction&& other) = default;
+RedirectActionInfo::RedirectActionInfo(RedirectActionInfo&&) = default;
+RedirectActionInfo& RedirectActionInfo::operator=(RedirectActionInfo&& other) =
+    default;
 
 CompositeMatcher::CompositeMatcher(MatcherList matchers)
     : matchers_(std::move(matchers)) {
@@ -123,7 +100,8 @@ void CompositeMatcher::AddOrUpdateRuleset(
   has_any_extra_headers_matcher_.reset();
 }
 
-bool CompositeMatcher::ShouldBlockRequest(const RequestParams& params) const {
+base::Optional<RequestAction> CompositeMatcher::GetBlockOrCollapseAction(
+    const RequestParams& params) const {
   // TODO(karandeepb): change this to report time in micro-seconds.
   SCOPED_UMA_HISTOGRAM_TIMER(
       "Extensions.DeclarativeNetRequest.ShouldBlockRequestTime."
@@ -131,15 +109,18 @@ bool CompositeMatcher::ShouldBlockRequest(const RequestParams& params) const {
 
   for (const auto& matcher : matchers_) {
     if (HasMatchingAllowRule(matcher.get(), params))
-      return false;
-    if (matcher->HasMatchingBlockRule(params))
-      return true;
+      return base::nullopt;
+
+    base::Optional<RequestAction> action =
+        matcher->GetBlockOrCollapseAction(params);
+    if (action)
+      return action;
   }
 
-  return false;
+  return base::nullopt;
 }
 
-RedirectAction CompositeMatcher::ShouldRedirectRequest(
+RedirectActionInfo CompositeMatcher::GetRedirectAction(
     const RequestParams& params,
     PageAccess page_access) const {
   // TODO(karandeepb): change this to report time in micro-seconds.
@@ -150,42 +131,35 @@ RedirectAction CompositeMatcher::ShouldRedirectRequest(
   bool notify_request_withheld = false;
   for (const auto& matcher : matchers_) {
     if (HasMatchingAllowRule(matcher.get(), params)) {
-      return RedirectAction(base::nullopt /* redirect_url */,
-                            false /* notify_request_withheld */);
+      return RedirectActionInfo(base::nullopt /* action */,
+                                false /* notify_request_withheld */);
     }
 
     if (page_access == PageAccess::kAllowed) {
-      GURL redirect_rule_url;
-
-      const flat_rule::UrlRule* redirect_rule =
-          matcher->GetRedirectRule(params, &redirect_rule_url);
-      const flat_rule::UrlRule* upgrade_rule = matcher->GetUpgradeRule(params);
-
-      if (!upgrade_rule && !redirect_rule)
+      base::Optional<RequestAction> action =
+          matcher->GetRedirectOrUpgradeActionByPriority(params);
+      if (!action)
         continue;
 
-      GURL redirect_url =
-          GetUrlByRulePriority(redirect_rule, upgrade_rule, *params.url,
-                               std::move(redirect_rule_url));
-      return RedirectAction(std::move(redirect_url),
-                            false /* notify_request_withheld */);
+      return RedirectActionInfo(std::move(action),
+                                false /* notify_request_withheld */);
     }
 
     // If the extension has no host permissions for the request, it can still
     // upgrade the request.
-    if (matcher->GetUpgradeRule(params)) {
-      return RedirectAction(GetUpgradedUrl(*params.url),
-                            false /* notify_request_withheld */);
+    base::Optional<RequestAction> upgrade_action =
+        matcher->GetUpgradeAction(params);
+    if (upgrade_action) {
+      return RedirectActionInfo(std::move(upgrade_action),
+                                false /* notify_request_withheld */);
     }
 
-    GURL redirect_url;
-    notify_request_withheld |=
-        (page_access == PageAccess::kWithheld &&
-         matcher->GetRedirectRule(params, &redirect_url));
+    notify_request_withheld |= (page_access == PageAccess::kWithheld &&
+                                matcher->GetRedirectAction(params));
   }
 
-  return RedirectAction(base::nullopt /* redirect_url */,
-                        notify_request_withheld);
+  return RedirectActionInfo(base::nullopt /* action */,
+                            notify_request_withheld);
 }
 
 uint8_t CompositeMatcher::GetRemoveHeadersMask(const RequestParams& params,

extensions/browser/api/declarative_net_request/composite_matcher.h

@@ -11,6 +11,7 @@
 
 #include "base/macros.h"
 #include "base/optional.h"
+#include "extensions/browser/api/declarative_net_request/request_action.h"
 #include "extensions/browser/api/declarative_net_request/ruleset_matcher.h"
 #include "extensions/common/permissions/permissions_data.h"
 
@@ -21,22 +22,22 @@ namespace declarative_net_request {
 // while respecting their priorities.
 class CompositeMatcher {
  public:
-  struct RedirectAction {
-    RedirectAction(base::Optional<GURL> redirect_url, bool notify);
-    ~RedirectAction();
-    RedirectAction(RedirectAction&& other);
-    RedirectAction& operator=(RedirectAction&& other);
+  struct RedirectActionInfo {
+    RedirectActionInfo(base::Optional<RequestAction> action, bool notify);
+    ~RedirectActionInfo();
+    RedirectActionInfo(RedirectActionInfo&& other);
+    RedirectActionInfo& operator=(RedirectActionInfo&& other);
 
-    // The URL the request will be redirected to. The request should not be
-    // redirected if this is not specified.
-    base::Optional<GURL> redirect_url;
+    // The action to be taken for this request. If specified, the action type
+    // must be |REDIRECT|.
+    base::Optional<RequestAction> action;
 
     // Whether the extension should be notified that the request was unable to
     // be redirected as the extension lacks the appropriate host permission for
     // the request.
     bool notify_request_withheld = false;
 
-    DISALLOW_COPY_AND_ASSIGN(RedirectAction);
+    DISALLOW_COPY_AND_ASSIGN(RedirectActionInfo);
   };
 
   using MatcherList = std::vector<std::unique_ptr<RulesetMatcher>>;
@@ -49,14 +50,15 @@ class CompositeMatcher {
   // corresponding ID is already present, updates the matcher.
   void AddOrUpdateRuleset(std::unique_ptr<RulesetMatcher> new_matcher);
 
-  // Returns whether the network request as specified by |params| should be
-  // blocked.
-  bool ShouldBlockRequest(const RequestParams& params) const;
+  // Returns a RequestAction if the network request specified by |params| should
+  // be blocked.
+  base::Optional<RequestAction> GetBlockOrCollapseAction(
+      const RequestParams& params) const;
 
-  // Returns a RedirectAction struct containing a redirect URL if the request
-  // is to be redirected, and whether the extension should be notified if its
-  // access to the request is withheld.
-  RedirectAction ShouldRedirectRequest(
+  // Returns a RedirectActionInfo struct containing a RequestAction if the
+  // request is to be redirected, and whether the extension should be notified
+  // if its access to the request is withheld.
+  RedirectActionInfo GetRedirectAction(
       const RequestParams& params,
       PermissionsData::PageAccess page_access) const;
 

extensions/browser/api/declarative_net_request/composite_matcher_unittest.cc

@@ -24,7 +24,7 @@ namespace extensions {
 namespace declarative_net_request {
 
 using PageAccess = PermissionsData::PageAccess;
-using RedirectAction = CompositeMatcher::RedirectAction;
+using RedirectActionInfo = CompositeMatcher::RedirectActionInfo;
 
 class CompositeMatcherTest : public ::testing::Test {
  public:
@@ -86,7 +86,8 @@ TEST_F(CompositeMatcherTest, RulesetPriority) {
   google_params.is_third_party = false;
 
   // The second ruleset should get more priority.
-  EXPECT_FALSE(composite_matcher->ShouldBlockRequest(google_params));
+  EXPECT_FALSE(
+      composite_matcher->GetBlockOrCollapseAction(google_params).has_value());
 
   GURL example_url = GURL("http://example.com");
   RequestParams example_params;
@@ -95,10 +96,11 @@ TEST_F(CompositeMatcherTest, RulesetPriority) {
       url_pattern_index::flat::ElementType_SUBDOCUMENT;
   example_params.is_third_party = false;
 
-  RedirectAction action = composite_matcher->ShouldRedirectRequest(
+  RedirectActionInfo action_info = composite_matcher->GetRedirectAction(
       example_params, PageAccess::kAllowed);
-  EXPECT_EQ(GURL("http://ruleset2.com"), action.redirect_url);
-  EXPECT_FALSE(action.notify_request_withheld);
+  ASSERT_TRUE(action_info.action);
+  EXPECT_EQ(GURL("http://ruleset2.com"), action_info.action->redirect_url);
+  EXPECT_FALSE(action_info.notify_request_withheld);
 
   // Now switch the priority of the two rulesets. This requires re-constructing
   // the two ruleset matchers.
@@ -120,12 +122,14 @@ TEST_F(CompositeMatcherTest, RulesetPriority) {
   example_params.allow_rule_cache.clear();
 
   // The first ruleset should get more priority.
-  EXPECT_TRUE(composite_matcher->ShouldBlockRequest(google_params));
-
-  action = composite_matcher->ShouldRedirectRequest(example_params,
-                                                    PageAccess::kAllowed);
-  EXPECT_EQ(GURL("http://ruleset1.com"), action.redirect_url);
-  EXPECT_FALSE(action.notify_request_withheld);
+  EXPECT_TRUE(
+      composite_matcher->GetBlockOrCollapseAction(google_params).has_value());
+
+  action_info = composite_matcher->GetRedirectAction(example_params,
+                                                     PageAccess::kAllowed);
+  ASSERT_TRUE(action_info.action);
+  EXPECT_EQ(GURL("http://ruleset1.com"), action_info.action->redirect_url);
+  EXPECT_FALSE(action_info.notify_request_withheld);
 }
 
 // Ensure allow rules in a higher priority matcher override redirect
@@ -188,10 +192,11 @@ TEST_F(CompositeMatcherTest, AllowRuleOverrides) {
   google_params.is_third_party = false;
 
   // The second ruleset should get more priority.
-  RedirectAction action = composite_matcher->ShouldRedirectRequest(
-      google_params, PageAccess::kAllowed);
-  EXPECT_EQ(GURL("http://ruleset2.com"), action.redirect_url);
-  EXPECT_FALSE(action.notify_request_withheld);
+  RedirectActionInfo action_info =
+      composite_matcher->GetRedirectAction(google_params, PageAccess::kAllowed);
+  ASSERT_TRUE(action_info.action);
+  EXPECT_EQ(GURL("http://ruleset2.com"), action_info.action->redirect_url);
+  EXPECT_FALSE(action_info.notify_request_withheld);
 
   // Send a request to example.com with headers, expect the allow rule to be
   // matched and the headers to remain.
@@ -226,10 +231,10 @@ TEST_F(CompositeMatcherTest, AllowRuleOverrides) {
 
   // The first ruleset should get more priority and so the request to google.com
   // should not be redirected.
-  action = composite_matcher->ShouldRedirectRequest(google_params,
-                                                    PageAccess::kAllowed);
-  EXPECT_FALSE(action.redirect_url);
-  EXPECT_FALSE(action.notify_request_withheld);
+  action_info =
+      composite_matcher->GetRedirectAction(google_params, PageAccess::kAllowed);
+  EXPECT_FALSE(action_info.action.has_value());
+  EXPECT_FALSE(action_info.notify_request_withheld);
 
   // The request to example.com should now have its headers removed.
   example_params.allow_rule_cache.clear();
@@ -310,11 +315,11 @@ TEST_F(CompositeMatcherTest, NotifyWithholdFromPageAccess) {
     params.element_type = url_pattern_index::flat::ElementType_SUBDOCUMENT;
     params.is_third_party = false;
 
-    RedirectAction redirect_action =
-        composite_matcher->ShouldRedirectRequest(params, test_case.access);
+    RedirectActionInfo redirect_action_info =
+        composite_matcher->GetRedirectAction(params, test_case.access);
 
     EXPECT_EQ(test_case.should_notify_withheld,
-              redirect_action.notify_request_withheld);
+              redirect_action_info.notify_request_withheld);
   }
 }
 
@@ -387,16 +392,18 @@ TEST_F(CompositeMatcherTest, GetRedirectUrlFromPriority) {
     params.element_type = url_pattern_index::flat::ElementType_SUBDOCUMENT;
     params.is_third_party = false;
 
-    RedirectAction redirect_action =
-        composite_matcher->ShouldRedirectRequest(params, PageAccess::kAllowed);
+    RedirectActionInfo redirect_action_info =
+        composite_matcher->GetRedirectAction(params, PageAccess::kAllowed);
 
     if (test_case.expected_final_url) {
-      EXPECT_EQ(test_case.expected_final_url->spec(),
-                redirect_action.redirect_url->spec());
-    } else
-      EXPECT_FALSE(redirect_action.redirect_url);
-
-    EXPECT_FALSE(redirect_action.notify_request_withheld);
+      ASSERT_TRUE(redirect_action_info.action);
+      EXPECT_EQ(test_case.expected_final_url,
+                redirect_action_info.action->redirect_url);
+    } else {
+      EXPECT_FALSE(redirect_action_info.action.has_value());
+    }
+
+    EXPECT_FALSE(redirect_action_info.notify_request_withheld);
   }
 }
 

extensions/browser/api/declarative_net_request/ruleset_manager.cc

@@ -162,13 +162,6 @@ bool IsRequestPageAllowed(const WebRequestInfo& request,
          allowed_pages.MatchesURL(*request.frame_data.pending_main_frame_url);
 }
 
-bool ShouldCollapseResourceType(flat_rule::ElementType type) {
-  // TODO(crbug.com/848842): Add support for other element types like
-  // OBJECT.
-  return type == flat_rule::ElementType_IMAGE ||
-         type == flat_rule::ElementType_SUBDOCUMENT;
-}
-
 void NotifyRequestWithheld(const ExtensionId& extension_id,
                            const WebRequestInfo& request) {
   DCHECK(ExtensionsAPIClient::Get());
@@ -404,13 +397,10 @@ base::Optional<RequestAction> RulesetManager::GetBlockOrCollapseAction(
     const std::vector<const ExtensionRulesetData*>& rulesets,
     const RequestParams& params) const {
   for (const ExtensionRulesetData* ruleset : rulesets) {
-    if (ruleset->matcher->ShouldBlockRequest(params)) {
-      RequestAction action = ShouldCollapseResourceType(params.element_type)
-                                 ? RequestAction(RequestAction::Type::COLLAPSE)
-                                 : RequestAction(RequestAction::Type::BLOCK);
-      action.extension_id = ruleset->extension_id;
+    base::Optional<RequestAction> action =
+        ruleset->matcher->GetBlockOrCollapseAction(params);
+    if (action)
       return action;
-    }
   }
   return base::nullopt;
 }
@@ -439,23 +429,20 @@ base::Optional<RequestAction> RulesetManager::GetRedirectOrUpgradeAction(
         WebRequestPermissions::REQUIRE_HOST_PERMISSION_FOR_URL_AND_INITIATOR,
         request.initiator, request.type);
 
-    CompositeMatcher::RedirectAction redirect_action =
-        ruleset->matcher->ShouldRedirectRequest(params, page_access);
+    CompositeMatcher::RedirectActionInfo redirect_action_info =
+        ruleset->matcher->GetRedirectAction(params, page_access);
 
-    DCHECK(!(redirect_action.redirect_url &&
-             redirect_action.notify_request_withheld));
-    if (redirect_action.notify_request_withheld) {
+    DCHECK(!(redirect_action_info.action &&
+             redirect_action_info.notify_request_withheld));
+    if (redirect_action_info.notify_request_withheld) {
       NotifyRequestWithheld(ruleset->extension_id, request);
       continue;
     }
 
-    if (!redirect_action.redirect_url)
+    if (!redirect_action_info.action)
       continue;
 
-    RequestAction action(RequestAction::Type::REDIRECT);
-    action.redirect_url = std::move(redirect_action.redirect_url);
-    action.extension_id = ruleset->extension_id;
-    return action;
+    return std::move(redirect_action_info.action);
   }
 
   return base::nullopt;

extensions/browser/api/declarative_net_request/ruleset_matcher.cc

@@ -297,6 +297,22 @@ GURL GetTransformedURL(const RequestParams& params,
   return params.url->ReplaceComponents(replacements);
 }
 
+bool ShouldCollapseResourceType(flat_rule::ElementType type) {
+  // TODO(crbug.com/848842): Add support for other element types like
+  // OBJECT.
+  return type == flat_rule::ElementType_IMAGE ||
+         type == flat_rule::ElementType_SUBDOCUMENT;
+}
+
+// Upgrades the url's scheme to HTTPS.
+GURL GetUpgradedUrl(const GURL& url) {
+  DCHECK(url.SchemeIs(url::kHttpScheme) || url.SchemeIs(url::kFtpScheme));
+
+  GURL::Replacements replacements;
+  replacements.SetSchemeStr(url::kHttpsScheme);
+  return url.ReplaceComponents(replacements);
+}
+
 }  // namespace
 
 RequestParams::RequestParams(const WebRequestInfo& info)
@@ -343,13 +359,88 @@ RulesetMatcher::LoadRulesetResult RulesetMatcher::CreateVerifiedMatcher(
 
   // Using WrapUnique instead of make_unique since this class has a private
   // constructor.
-  *matcher = base::WrapUnique(new RulesetMatcher(
-      std::move(ruleset_data), source.id(), source.priority()));
+  *matcher = base::WrapUnique(new RulesetMatcher(std::move(ruleset_data),
+                                                 source.id(), source.priority(),
+                                                 source.extension_id()));
   return kLoadSuccess;
 }
 
 RulesetMatcher::~RulesetMatcher() = default;
 
+base::Optional<RequestAction> RulesetMatcher::GetBlockOrCollapseAction(
+    const RequestParams& params) const {
+  const flat_rule::UrlRule* rule =
+      GetMatchingRule(params, flat::ActionIndex_block);
+  if (!rule)
+    return base::nullopt;
+
+  RequestAction action = ShouldCollapseResourceType(params.element_type)
+                             ? RequestAction(RequestAction::Type::COLLAPSE)
+                             : RequestAction(RequestAction::Type::BLOCK);
+
+  action.extension_id = extension_id_;
+  return action;
+}
+
+base::Optional<RequestAction> RulesetMatcher::GetRedirectAction(
+    const RequestParams& params) const {
+  GURL redirect_rule_url;
+  const flat_rule::UrlRule* redirect_rule =
+      GetRedirectRule(params, &redirect_rule_url);
+
+  if (!redirect_rule)
+    return base::nullopt;
+
+  RequestAction redirect_action(RequestAction::Type::REDIRECT);
+  redirect_action.extension_id = extension_id_;
+  redirect_action.redirect_url = std::move(redirect_rule_url);
+
+  return redirect_action;
+}
+
+base::Optional<RequestAction> RulesetMatcher::GetUpgradeAction(
+    const RequestParams& params) const {
+  const flat_rule::UrlRule* upgrade_rule = GetUpgradeRule(params);
+
+  if (!upgrade_rule)
+    return base::nullopt;
+
+  RequestAction upgrade_action(RequestAction::Type::REDIRECT);
+  upgrade_action.extension_id = extension_id_;
+  upgrade_action.redirect_url = GetUpgradedUrl(*params.url);
+
+  return upgrade_action;
+}
+
+base::Optional<RequestAction>
+RulesetMatcher::GetRedirectOrUpgradeActionByPriority(
+    const RequestParams& params) const {
+  GURL redirect_rule_url;
+  const flat_rule::UrlRule* redirect_rule =
+      GetRedirectRule(params, &redirect_rule_url);
+  const flat_rule::UrlRule* upgrade_rule = GetUpgradeRule(params);
+
+  if (!redirect_rule && !upgrade_rule)
+    return base::nullopt;
+
+  GURL highest_priority_url;
+  if (!upgrade_rule) {
+    highest_priority_url = std::move(redirect_rule_url);
+  } else if (!redirect_rule) {
+    highest_priority_url = GetUpgradedUrl(*params.url);
+  } else {
+    highest_priority_url = upgrade_rule->priority() > redirect_rule->priority()
+                               ? GetUpgradedUrl(*params.url)
+                               : std::move(redirect_rule_url);
+  }
+
+  RequestAction action(RequestAction::Type::REDIRECT);
+  action.extension_id = extension_id_;
+  action.redirect_url = std::move(highest_priority_url);
+
+  return action;
+}
+
 uint8_t RulesetMatcher::GetRemoveHeadersMask(const RequestParams& params,
                                              uint8_t ignored_mask) const {
   uint8_t mask = 0;
@@ -391,6 +482,19 @@ uint8_t RulesetMatcher::GetRemoveHeadersMask(const RequestParams& params,
   return mask;
 }
 
+RulesetMatcher::RulesetMatcher(std::string ruleset_data,
+                               size_t id,
+                               size_t priority,
+                               const ExtensionId& extension_id)
+    : ruleset_data_(std::move(ruleset_data)),
+      root_(flat::GetExtensionIndexedRuleset(ruleset_data_.data())),
+      matchers_(GetMatchers(root_)),
+      metadata_list_(root_->extension_metadata()),
+      id_(id),
+      priority_(priority),
+      extension_id_(extension_id),
+      is_extra_headers_matcher_(IsExtraHeadersMatcherInternal(*root_)) {}
+
 const flat_rule::UrlRule* RulesetMatcher::GetRedirectRule(
     const RequestParams& params,
     GURL* redirect_url) const {
@@ -438,17 +542,6 @@ const flat_rule::UrlRule* RulesetMatcher::GetUpgradeRule(
              : nullptr;
 }
 
-RulesetMatcher::RulesetMatcher(std::string ruleset_data,
-                               size_t id,
-                               size_t priority)
-    : ruleset_data_(std::move(ruleset_data)),
-      root_(flat::GetExtensionIndexedRuleset(ruleset_data_.data())),
-      matchers_(GetMatchers(root_)),
-      metadata_list_(root_->extension_metadata()),
-      id_(id),
-      priority_(priority),
-      is_extra_headers_matcher_(IsExtraHeadersMatcherInternal(*root_)) {}
-
 const flat_rule::UrlRule* RulesetMatcher::GetMatchingRule(
     const RequestParams& params,
     flat::ActionIndex index,

extensions/browser/api/declarative_net_request/ruleset_matcher.h

@@ -11,8 +11,10 @@
 #include <vector>
 
 #include "base/containers/flat_map.h"
+#include "base/optional.h"
 #include "components/url_pattern_index/url_pattern_index.h"
 #include "extensions/browser/api/declarative_net_request/flat/extension_ruleset_generated.h"
+#include "extensions/common/extension_id.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
@@ -20,6 +22,7 @@ namespace extensions {
 struct WebRequestInfo;
 
 namespace declarative_net_request {
+struct RequestAction;
 class RulesetSource;
 
 namespace flat {
@@ -93,34 +96,39 @@ class RulesetMatcher {
 
   ~RulesetMatcher();
 
-  // Returns whether the ruleset has a matching blocking rule.
-  bool HasMatchingBlockRule(const RequestParams& params) const {
-    return GetMatchingRule(params, flat::ActionIndex_block);
-  }
+  // Returns the ruleset's matching RequestAction with type |BLOCK| or
+  // |COLLAPSE|, or base::nullopt if the ruleset has no matching blocking rule.
+  base::Optional<RequestAction> GetBlockOrCollapseAction(
+      const RequestParams& params) const;
 
   // Returns whether the ruleset has a matching allow rule.
   bool HasMatchingAllowRule(const RequestParams& params) const {
     return GetMatchingRule(params, flat::ActionIndex_allow);
   }
 
+  // Returns the ruleset's matching redirect RequestAction if there is a
+  // matching redirect rule, otherwise returns base::nullopt.
+  base::Optional<RequestAction> GetRedirectAction(
+      const RequestParams& params) const;
+
+  // Returns the ruleset's matching RequestAction with |params.url| upgraded to
+  // HTTPS as the redirect url, or base::nullopt if no matching rule is found or
+  // if the request's scheme is not upgradeable.
+  base::Optional<RequestAction> GetUpgradeAction(
+      const RequestParams& params) const;
+
+  // Returns a RedirectAction constructed from the matching redirect or upgrade
+  // rule with the highest priority, or base::nullopt if no matching redirect or
+  // upgrade rules are found for this request.
+  base::Optional<RequestAction> GetRedirectOrUpgradeActionByPriority(
+      const RequestParams& params) const;
+
   // Returns the bitmask of headers to remove from the request. The bitmask
   // corresponds to RemoveHeadersMask type. |ignored_mask| denotes the mask of
   // headers to be skipped for evaluation and is excluded in the return value.
   uint8_t GetRemoveHeadersMask(const RequestParams& params,
                                uint8_t ignored_mask) const;
 
-  // Returns the ruleset's matching redirect rule and populates
-  // |redirect_url| if there is a matching redirect rule, otherwise returns
-  // nullptr.
-  const url_pattern_index::flat::UrlRule* GetRedirectRule(
-      const RequestParams& params,
-      GURL* redirect_url) const;
-
-  // Returns the ruleset's matching upgrade scheme rule or nullptr if no
-  // matching rule is found or if the request's scheme is not upgradeable.
-  const url_pattern_index::flat::UrlRule* GetUpgradeRule(
-      const RequestParams& params) const;
-
   // Returns whether this modifies "extraHeaders".
   bool IsExtraHeadersMatcher() const { return is_extra_headers_matcher_; }
 
@@ -137,7 +145,22 @@ class RulesetMatcher {
   using ExtensionMetadataList =
       flatbuffers::Vector<flatbuffers::Offset<flat::UrlRuleMetadata>>;
 
-  explicit RulesetMatcher(std::string ruleset_data, size_t id, size_t priority);
+  explicit RulesetMatcher(std::string ruleset_data,
+                          size_t id,
+                          size_t priority,
+                          const ExtensionId& extension_id);
+
+  // Returns the ruleset's matching redirect rule and populates
+  // |redirect_url| if there is a matching redirect rule, otherwise returns
+  // nullptr.
+  const url_pattern_index::flat::UrlRule* GetRedirectRule(
+      const RequestParams& params,
+      GURL* redirect_url) const;
+
+  // Returns the ruleset's matching upgrade scheme rule or nullptr if no
+  // matching rule is found or if the request's scheme is not upgradeable.
+  const url_pattern_index::flat::UrlRule* GetUpgradeRule(
+      const RequestParams& params) const;
 
   const url_pattern_index::flat::UrlRule* GetMatchingRule(
       const RequestParams& params,
@@ -154,8 +177,11 @@ class RulesetMatcher {
 
   const ExtensionMetadataList* const metadata_list_;
 
-  size_t id_;
-  size_t priority_;
+  const size_t id_;
+  const size_t priority_;
+
+  // The ID of the extension from which this matcher's ruleset originates from.
+  const ExtensionId extension_id_;
 
   const bool is_extra_headers_matcher_;
 

extensions/browser/api/declarative_net_request/ruleset_matcher_unittest.cc

@@ -14,6 +14,7 @@
 #include "components/url_pattern_index/flat/url_pattern_index_generated.h"
 #include "components/version_info/version_info.h"
 #include "extensions/browser/api/declarative_net_request/constants.h"
+#include "extensions/browser/api/declarative_net_request/request_action.h"
 #include "extensions/browser/api/declarative_net_request/ruleset_source.h"
 #include "extensions/browser/api/declarative_net_request/test_utils.h"
 #include "extensions/browser/api/declarative_net_request/utils.h"
@@ -49,7 +50,7 @@ TEST_F(RulesetMatcherTest, BlockingRule) {
 
   auto should_block_request = [&matcher](const RequestParams& params) {
     return !matcher->HasMatchingAllowRule(params) &&
-           matcher->HasMatchingBlockRule(params);
+           matcher->GetBlockOrCollapseAction(params).has_value();
   };
 
   GURL google_url("http://google.com");
@@ -77,11 +78,6 @@ TEST_F(RulesetMatcherTest, RedirectRule) {
   std::unique_ptr<RulesetMatcher> matcher;
   ASSERT_TRUE(CreateVerifiedMatcher({rule}, CreateTemporarySource(), &matcher));
 
-  auto should_redirect_request = [&matcher](const RequestParams& params,
-                                            GURL* redirect_url) {
-    return matcher->GetRedirectRule(params, redirect_url) != nullptr;
-  };
-
   GURL google_url("http://google.com");
   GURL yahoo_url("http://yahoo.com");
 
@@ -90,12 +86,13 @@ TEST_F(RulesetMatcherTest, RedirectRule) {
   params.element_type = url_pattern_index::flat::ElementType_SUBDOCUMENT;
   params.is_third_party = true;
 
-  GURL redirect_url;
-  EXPECT_TRUE(should_redirect_request(params, &redirect_url));
-  EXPECT_EQ(yahoo_url, redirect_url);
+  base::Optional<RequestAction> redirect_action =
+      matcher->GetRedirectAction(params);
+  ASSERT_TRUE(redirect_action);
+  EXPECT_EQ(yahoo_url, redirect_action->redirect_url);
 
   params.url = &yahoo_url;
-  EXPECT_FALSE(should_redirect_request(params, &redirect_url));
+  EXPECT_FALSE(matcher->GetRedirectAction(params));
 }
 
 // Test that a URL cannot redirect to itself, as filed in crbug.com/954646.
@@ -116,8 +113,7 @@ TEST_F(RulesetMatcherTest, PreventSelfRedirect) {
   params.element_type = url_pattern_index::flat::ElementType_SUBDOCUMENT;
   params.is_third_party = true;
 
-  GURL redirect_url;
-  EXPECT_FALSE(matcher->GetRedirectRule(params, &redirect_url));
+  EXPECT_FALSE(matcher->GetRedirectAction(params));
 }
 
 // Tests a simple upgrade scheme rule.
@@ -131,7 +127,7 @@ TEST_F(RulesetMatcherTest, UpgradeRule) {
   ASSERT_TRUE(CreateVerifiedMatcher({rule}, CreateTemporarySource(), &matcher));
 
   auto should_upgrade_request = [&matcher](const RequestParams& params) {
-    return matcher->GetUpgradeRule(params) != nullptr;
+    return matcher->GetUpgradeAction(params).has_value();
   };
 
   GURL google_url("http://google.com");
@@ -232,21 +228,21 @@ TEST_F(RulesetMatcherTest, RedirectToExtensionPath) {
   const size_t kId = 1;
   const size_t kPriority = 1;
   const size_t kRuleCountLimit = 10;
-  const ExtensionId extension_id = "extension_id";
   ASSERT_TRUE(CreateVerifiedMatcher(
-      {rule},
-      CreateTemporarySource(kId, kPriority, kRuleCountLimit, extension_id),
+      {rule}, CreateTemporarySource(kId, kPriority, kRuleCountLimit),
       &matcher));
 
   GURL example_url("http://example.com");
   RequestParams params;
   params.url = &example_url;
 
-  GURL redirect_url;
-  EXPECT_TRUE(matcher->GetRedirectRule(params, &redirect_url));
+  base::Optional<RequestAction> redirect_action =
+      matcher->GetRedirectAction(params);
+
+  ASSERT_TRUE(redirect_action.has_value());
   GURL expected_redirect_url(
-      "chrome-extension://extension_id/path/newfile.js?query#fragment");
-  EXPECT_EQ(expected_redirect_url, redirect_url);
+      "chrome-extension://extensionid/path/newfile.js?query#fragment");
+  EXPECT_EQ(expected_redirect_url, redirect_action->redirect_url);
 }
 
 // Tests a rule to redirect to a static url.
@@ -265,10 +261,12 @@ TEST_F(RulesetMatcherTest, RedirectToStaticUrl) {
   RequestParams params;
   params.url = &example_url;
 
-  GURL redirect_url;
+  base::Optional<RequestAction> redirect_action =
+      matcher->GetRedirectAction(params);
+
+  ASSERT_TRUE(redirect_action.has_value());
   GURL expected_redirect_url("https://google.com");
-  EXPECT_TRUE(matcher->GetRedirectRule(params, &redirect_url));
-  EXPECT_EQ(expected_redirect_url, redirect_url);
+  EXPECT_EQ(expected_redirect_url, redirect_action->redirect_url);
 }
 
 // Tests url transformation rules.
@@ -396,17 +394,20 @@ TEST_F(RulesetMatcherTest, UrlTransform) {
     RequestParams params;
     params.url = &url;
 
-    GURL redirect_url;
+    base::Optional<RequestAction> redirect_action =
+        matcher->GetRedirectAction(params);
+
     if (!test_case.expected_redirect_url) {
-      EXPECT_FALSE(matcher->GetRedirectRule(params, &redirect_url))
-          << redirect_url.spec();
+      EXPECT_FALSE(redirect_action) << redirect_action->redirect_url->spec();
       continue;
     }
 
     ASSERT_TRUE(GURL(*test_case.expected_redirect_url).is_valid())
         << *test_case.expected_redirect_url;
-    EXPECT_TRUE(matcher->GetRedirectRule(params, &redirect_url));
-    EXPECT_EQ(*test_case.expected_redirect_url, redirect_url.spec());
+
+    ASSERT_TRUE(redirect_action.has_value());
+    EXPECT_EQ(GURL(*test_case.expected_redirect_url),
+              redirect_action->redirect_url);
   }
 }