[DNR] Attribute actions to extensions (using a list of actions)

This CL changes DNR actions such that a list of actions is returned when
DNR rules are matched, and each action has a unique extension ID. This
is necessary for remove header rules and the action feedback API to
determine exactly which extensions removed which headers from a request.

Bug: 973211, 991420
Change-Id: Ie77a9a51a3c6761da414f8bc3a0f8747068f3717
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1758806
Commit-Queue: Kelvin Jiang <kelvinjiang@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697450}

chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc

@@ -3150,6 +3150,116 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
   }
 }
 
+// Test that the badge text for extensions will update correctly for
+// removeHeader rules.
+IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
+                       RemoveHeadersBadgeText) {
+  auto referer_url = embedded_test_server()->GetURL(
+      "example.com", "/set-header?referer: none");
+  auto set_cookie_url =
+      embedded_test_server()->GetURL("example.com", "/set-cookie?a=b");
+
+  // Navigates frame with name |frame_name| to |url|.
+  auto navigate_frame = [this](const std::string& frame_name, const GURL& url,
+                               bool use_frame_referrer) {
+    content::TestNavigationObserver navigation_observer(
+        web_contents(), 1 /*number_of_navigations*/);
+
+    const char* referrer_policy = use_frame_referrer ? "origin" : "no-referrer";
+
+    ASSERT_TRUE(content::ExecuteScript(
+        GetMainFrame(),
+        base::StringPrintf(R"(
+          document.getElementsByName('%s')[0].referrerPolicy = '%s';
+          document.getElementsByName('%s')[0].src = '%s';)",
+                           frame_name.c_str(), referrer_policy,
+                           frame_name.c_str(), url.spec().c_str())));
+    navigation_observer.Wait();
+  };
+
+  const std::string kFrameName1 = "frame1";
+  const GURL page_url = embedded_test_server()->GetURL(
+      "nomatch.com", "/page_with_two_frames.html");
+
+  // Create an extension with a rule to remove the Set-Cookie header, and get
+  // the ExtensionAction for it.
+  TestRule rule1 = CreateGenericRule();
+  rule1.id = kMinValidID;
+  rule1.condition->url_filter = "example.com";
+  rule1.condition->resource_types = std::vector<std::string>({"sub_frame"});
+  rule1.action->type = "removeHeaders";
+  rule1.action->remove_headers_list = std::vector<std::string>({"setCookie"});
+
+  ASSERT_NO_FATAL_FAILURE(LoadExtensionWithRules({rule1}, "extension_1", {}));
+
+  const ExtensionId remove_set_cookie_ext_id = last_loaded_extension_id();
+  ExtensionPrefs::Get(profile())->SetDNRUseActionCountAsBadgeText(
+      remove_set_cookie_ext_id, true);
+
+  ExtensionAction* remove_set_cookie_action =
+      ExtensionActionManager::Get(web_contents()->GetBrowserContext())
+          ->GetExtensionAction(*extension_registry()->GetExtensionById(
+              remove_set_cookie_ext_id,
+              extensions::ExtensionRegistry::ENABLED));
+
+  // Create an extension with a rule to remove the referer header, and get the
+  // ExtensionAction for it.
+  TestRule rule2 = CreateGenericRule();
+  rule2.id = kMinValidID;
+  rule2.condition->url_filter = "example.com";
+  rule2.condition->resource_types = std::vector<std::string>({"sub_frame"});
+  rule2.action->type = "removeHeaders";
+  rule2.action->remove_headers_list = std::vector<std::string>({"referer"});
+
+  ASSERT_NO_FATAL_FAILURE(LoadExtensionWithRules({rule2}, "extension_2", {}));
+
+  const ExtensionId remove_referer_ext_id = last_loaded_extension_id();
+  ExtensionPrefs::Get(profile())->SetDNRUseActionCountAsBadgeText(
+      remove_referer_ext_id, true);
+
+  ExtensionAction* remove_referer_action =
+      ExtensionActionManager::Get(web_contents()->GetBrowserContext())
+          ->GetExtensionAction(*extension_registry()->GetExtensionById(
+              remove_referer_ext_id, extensions::ExtensionRegistry::ENABLED));
+
+  struct {
+    GURL url;
+    bool use_referrer;
+    std::string expected_remove_referer_badge_text;
+    std::string expected_remove_set_cookie_badge_text;
+  } test_cases[] = {
+      // This request only has a Set-Cookie header. Only the badge text for the
+      // extension with a remove Set-Cookie header rule should be incremented.
+      {set_cookie_url, false, "0", "1"},
+      // This request only has a Referer header. Only the badge text for the
+      // extension with a remove Referer header rule should be incremented.
+      {referer_url, true, "1", "1"},
+      // This request has both a Referer and a Set-Cookie header. The badge text
+      // for both extensions should be incremented.
+      {set_cookie_url, true, "2", "2"},
+  };
+
+  ui_test_utils::NavigateToURL(browser(), page_url);
+  ASSERT_TRUE(WasFrameWithScriptLoaded(GetMainFrame()));
+
+  int first_tab_id = ExtensionTabUtil::GetTabId(web_contents());
+  EXPECT_EQ("0", remove_set_cookie_action->GetDisplayBadgeText(first_tab_id));
+  EXPECT_EQ("0", remove_referer_action->GetDisplayBadgeText(first_tab_id));
+
+  for (const auto& test_case : test_cases) {
+    SCOPED_TRACE(base::StringPrintf("Testing URL: %s, using referrer: %s",
+                                    test_case.url.spec().c_str(),
+                                    test_case.use_referrer ? "true" : "false"));
+
+    navigate_frame(kFrameName1, test_case.url, test_case.use_referrer);
+    EXPECT_EQ(test_case.expected_remove_set_cookie_badge_text,
+              remove_set_cookie_action->GetDisplayBadgeText(first_tab_id));
+
+    EXPECT_EQ(test_case.expected_remove_referer_badge_text,
+              remove_referer_action->GetDisplayBadgeText(first_tab_id));
+  }
+}
+
 // Test fixture to verify that host permissions for the request url and the
 // request initiator are properly checked when redirecting requests. Loads an
 // example.com url with four sub-frames named frame_[1..4] from hosts

chrome/browser/extensions/api/web_request/web_request_api_unittest.cc

@@ -771,7 +771,7 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) {
   headers0.MergeFrom(base_headers);
   WebRequestInfoInitParams info_params;
   WebRequestInfo info(std::move(info_params));
-  info.dnr_action.emplace(Action::Type::NONE);
+  info.dnr_actions.emplace_back(Action::Type::NONE);
   MergeOnBeforeSendHeadersResponses(info, deltas, &headers0, &ignored_actions,
                                     &ignore1, &ignore2,
                                     &request_headers_modified0);
@@ -879,8 +879,10 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) {
   bool request_headers_modified4 = false;
   net::HttpRequestHeaders headers4;
   headers4.MergeFrom(base_headers);
-  info.dnr_action.emplace(Action::Type::REMOVE_HEADERS);
-  info.dnr_action->request_headers_to_remove = {"key5"};
+
+  Action remove_headers_action(Action::Type::REMOVE_HEADERS);
+  remove_headers_action.request_headers_to_remove = {"key5"};
+  info.dnr_actions.push_back(std::move(remove_headers_action));
   MergeOnBeforeSendHeadersResponses(info, deltas, &headers4, &ignored_actions,
                                     &ignore1, &ignore2,
                                     &request_headers_modified4);
@@ -927,7 +929,7 @@ TEST(ExtensionWebRequestHelpersTest,
 
   WebRequestInfoInitParams info_params;
   WebRequestInfo info(std::move(info_params));
-  info.dnr_action.emplace(Action::Type::NONE);
+  info.dnr_actions.emplace_back(Action::Type::NONE);
   helpers::IgnoredActions ignored_actions;
   std::set<std::string> removed_headers, set_headers;
   bool request_headers_modified = false;
@@ -1270,7 +1272,7 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnHeadersReceivedResponses) {
   WebRequestInfoInitParams info_params;
   info_params.url = GURL(kExampleUrl);
   WebRequestInfo info(std::move(info_params));
-  info.dnr_action.emplace(Action::Type::NONE);
+  info.dnr_actions.emplace_back(Action::Type::NONE);
 
   MergeOnHeadersReceivedResponses(info, deltas, base_headers.get(),
                                   &new_headers0, &allowed_unsafe_redirect_url0,
@@ -1350,8 +1352,10 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnHeadersReceivedResponses) {
 
   // Ensure headers removed by Declarative Net Request API can't be added by web
   // request extensions and result in a conflict.
-  info.dnr_action.emplace(Action::Type::REMOVE_HEADERS);
-  info.dnr_action->response_headers_to_remove = {"key3"};
+  Action remove_headers_action(Action::Type::REMOVE_HEADERS);
+  remove_headers_action.response_headers_to_remove = {"key3"};
+  info.dnr_actions.push_back(std::move(remove_headers_action));
+
   ignored_actions.clear();
   bool response_headers_modified3 = false;
   scoped_refptr<net::HttpResponseHeaders> new_headers3;
@@ -1406,7 +1410,7 @@ TEST(ExtensionWebRequestHelpersTest,
   WebRequestInfoInitParams info_params;
   info_params.url = GURL(kExampleUrl);
   WebRequestInfo info(std::move(info_params));
-  info.dnr_action.emplace(Action::Type::NONE);
+  info.dnr_actions.emplace_back(Action::Type::NONE);
 
   MergeOnHeadersReceivedResponses(info, deltas, base_headers.get(),
                                   &new_headers1, &allowed_unsafe_redirect_url1,

extensions/browser/api/declarative_net_request/action_tracker.cc

@@ -22,21 +22,18 @@ ActionTracker::~ActionTracker() {
   DCHECK(actions_matched_.empty());
 }
 
-void ActionTracker::OnRuleMatched(const std::vector<ExtensionId>& extension_ids,
-                                  int tab_id) {
+void ActionTracker::OnRuleMatched(const ExtensionId& extension_id, int tab_id) {
   if (tab_id == extension_misc::kUnknownTabId)
     return;
 
-  for (const auto& extension_id : extension_ids) {
-    auto key = std::make_pair(extension_id, tab_id);
-    int action_count = ++actions_matched_[key];
+  auto key = std::make_pair(extension_id, tab_id);
+  int action_count = ++actions_matched_[key];
 
-    if (extension_prefs_->GetDNRUseActionCountAsBadgeText(extension_id)) {
-      DCHECK(ExtensionsAPIClient::Get());
-      ExtensionsAPIClient::Get()->UpdateActionCount(
-          browser_context_, extension_id, tab_id, action_count,
-          false /* clear_badge_text */);
-    }
+  if (extension_prefs_->GetDNRUseActionCountAsBadgeText(extension_id)) {
+    DCHECK(ExtensionsAPIClient::Get());
+    ExtensionsAPIClient::Get()->UpdateActionCount(
+        browser_context_, extension_id, tab_id, action_count,
+        false /* clear_badge_text */);
   }
 }
 

extensions/browser/api/declarative_net_request/action_tracker.h

@@ -27,7 +27,7 @@ class ActionTracker {
   ~ActionTracker();
 
   // Called whenever a request matches with a rule.
-  void OnRuleMatched(const std::vector<ExtensionId>& extension_ids, int tab_id);
+  void OnRuleMatched(const ExtensionId& extension_id, int tab_id);
 
   // Updates the action count for all tabs for the specified |extension_id|'s
   // extension action. Called when chrome.setActionCountAsBadgeText(true) is

extensions/browser/api/declarative_net_request/composite_matcher.cc

@@ -189,14 +189,16 @@ RedirectAction CompositeMatcher::ShouldRedirectRequest(
 }
 
 uint8_t CompositeMatcher::GetRemoveHeadersMask(const RequestParams& params,
-                                               uint8_t current_mask) const {
-  uint8_t mask = current_mask;
+                                               uint8_t ignored_mask) const {
+  uint8_t mask = 0;
   for (const auto& matcher : matchers_) {
     // The allow rule will override lower priority remove header rules.
     if (HasMatchingAllowRule(matcher.get(), params))
       return mask;
-    mask |= matcher->GetRemoveHeadersMask(params, mask);
+    mask |= matcher->GetRemoveHeadersMask(params, mask | ignored_mask);
   }
+
+  DCHECK(!(mask & ignored_mask));
   return mask;
 }
 

extensions/browser/api/declarative_net_request/composite_matcher.h

@@ -60,11 +60,12 @@ class CompositeMatcher {
       const RequestParams& params,
       PermissionsData::PageAccess page_access) const;
 
-  // Returns the bitmask of headers to remove from the request. The bitmask
-  // corresponds to RemoveHeadersMask type. |current_mask| denotes the current
-  // mask of headers to be removed and is included in the return value.
+  // Returns the bitmask of headers to remove from the request corresponding to
+  // rules matched from this extension. The bitmask corresponds to
+  // RemoveHeadersMask type. |ignored_mask| denotes the current mask of headers
+  // to be skipped for evaluation and is excluded in the return value.
   uint8_t GetRemoveHeadersMask(const RequestParams& params,
-                               uint8_t current_mask) const;
+                               uint8_t ignored_mask) const;
 
   // Returns whether this modifies "extraHeaders".
   bool HasAnyExtraHeadersMatcher() const;

extensions/browser/api/declarative_net_request/ruleset_manager.cc

@@ -326,7 +326,7 @@ void RulesetManager::UpdateAllowedPages(const ExtensionId& extension_id,
   ClearRendererCacheOnNavigation();
 }
 
-const RulesetManager::Action& RulesetManager::EvaluateRequest(
+const std::vector<RulesetManager::Action>& RulesetManager::EvaluateRequest(
     const WebRequestInfo& request,
     bool is_incognito_context) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -337,10 +337,12 @@ const RulesetManager::Action& RulesetManager::EvaluateRequest(
   // |is_incognito_context| will stay the same for a given |request|. This also
   // assumes that the core state of the WebRequestInfo isn't changed between the
   // different EvaluateRequest invocations.
-  if (!request.dnr_action)
-    request.dnr_action = EvaluateRequestInternal(request, is_incognito_context);
+  if (request.dnr_actions.empty()) {
+    request.dnr_actions =
+        EvaluateRequestInternal(request, is_incognito_context);
+  }
 
-  return *request.dnr_action;
+  return request.dnr_actions;
 }
 
 bool RulesetManager::HasAnyExtraHeadersMatcher() const {
@@ -357,7 +359,8 @@ bool RulesetManager::HasAnyExtraHeadersMatcher() const {
 bool RulesetManager::HasExtraHeadersMatcherForRequest(
     const WebRequestInfo& request,
     bool is_incognito_context) const {
-  const Action& action = EvaluateRequest(request, is_incognito_context);
+  const std::vector<Action>& actions =
+      EvaluateRequest(request, is_incognito_context);
 
   // We only support removing a subset of extra headers currently. If that
   // changes, the implementation here should change as well.
@@ -365,7 +368,12 @@ bool RulesetManager::HasExtraHeadersMatcherForRequest(
                 "Modify this method to ensure HasExtraHeadersMatcherForRequest "
                 "is updated as new actions are added.");
 
-  return action.type == Action::Type::REMOVE_HEADERS;
+  for (const auto& action : actions) {
+    if (action.type == Action::Type::REMOVE_HEADERS)
+      return true;
+  }
+
+  return false;
 }
 
 void RulesetManager::SetObserverForTest(TestObserver* observer) {
@@ -404,7 +412,7 @@ base::Optional<RulesetManager::Action> RulesetManager::GetBlockOrCollapseAction(
       Action action = ShouldCollapseResourceType(params.element_type)
                           ? Action(Action::Type::COLLAPSE)
                           : Action(Action::Type::BLOCK);
-      action.extension_ids.push_back(ruleset->extension_id);
+      action.extension_id = ruleset->extension_id;
       return action;
     }
   }
@@ -451,48 +459,59 @@ RulesetManager::GetRedirectOrUpgradeAction(
 
     Action action(Action::Type::REDIRECT);
     action.redirect_url = std::move(redirect_action.redirect_url);
-    action.extension_ids.push_back(ruleset->extension_id);
+    action.extension_id = ruleset->extension_id;
     return action;
   }
 
   return base::nullopt;
 }
 
-base::Optional<RulesetManager::Action> RulesetManager::GetRemoveHeadersAction(
+std::vector<RulesetManager::Action> RulesetManager::GetRemoveHeadersActions(
     const std::vector<const ExtensionRulesetData*>& rulesets,
     const RequestParams& params) const {
-  Action action(Action::Type::REMOVE_HEADERS);
-  uint8_t mask = 0;
+  std::vector<Action> remove_headers_actions;
+
+  // Keep a combined mask of all headers to be removed to be passed into
+  // GetRemoveHeadersMask. This is done to ensure the ruleset matchers will skip
+  // matching rules for headers already slated to be removed.
+  uint8_t combined_mask = 0;
   for (const ExtensionRulesetData* ruleset : rulesets) {
-    uint8_t ruleset_mask = ruleset->matcher->GetRemoveHeadersMask(params, mask);
-    if (ruleset_mask)
-      action.extension_ids.push_back(ruleset->extension_id);
+    uint8_t ruleset_mask = ruleset->matcher->GetRemoveHeadersMask(
+        params, combined_mask /* ignored_mask */);
+    if (!ruleset_mask)
+      continue;
 
-    mask |= ruleset_mask;
+    Action action(Action::Type::REMOVE_HEADERS);
+    PopulateHeadersFromMask(ruleset_mask, &action.request_headers_to_remove,
+                            &action.response_headers_to_remove);
+    action.extension_id = ruleset->extension_id;
+    remove_headers_actions.push_back(std::move(action));
+    combined_mask |= ruleset_mask;
   }
 
-  if (!mask)
-    return base::nullopt;
-
-  PopulateHeadersFromMask(mask, &action.request_headers_to_remove,
-                          &action.response_headers_to_remove);
-  return action;
+  return remove_headers_actions;
 }
 
-RulesetManager::Action RulesetManager::EvaluateRequestInternal(
+std::vector<RulesetManager::Action> RulesetManager::EvaluateRequestInternal(
     const WebRequestInfo& request,
     bool is_incognito_context) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(!request.dnr_action);
+  DCHECK(request.dnr_actions.empty());
+
+  std::vector<Action> actions;
 
-  if (!ShouldEvaluateRequest(request))
-    return Action(Action::Type::NONE);
+  if (!ShouldEvaluateRequest(request)) {
+    actions.emplace_back(Action::Type::NONE);
+    return actions;
+  }
 
   if (test_observer_)
     test_observer_->OnEvaluateRequest(request, is_incognito_context);
 
-  if (rulesets_.empty())
-    return Action(Action::Type::NONE);
+  if (rulesets_.empty()) {
+    actions.emplace_back(Action::Type::NONE);
+    return actions;
+  }
 
   SCOPED_UMA_HISTOGRAM_TIMER(
       "Extensions.DeclarativeNetRequest.EvaluateRequestTime.AllExtensions2");
@@ -532,24 +551,31 @@ RulesetManager::Action RulesetManager::EvaluateRequestInternal(
   // If the request is blocked, no further modifications can happen.
   base::Optional<Action> action =
       GetBlockOrCollapseAction(rulesets_to_evaluate, params);
-  if (action)
-    return std::move(*action);
+  if (action) {
+    actions.push_back(std::move(std::move(*action)));
+    return actions;
+  }
 
   // If the request is redirected, no further modifications can happen. A new
   // request will be created and subsequently evaluated.
   action = GetRedirectOrUpgradeAction(rulesets_to_evaluate, request, tab_id,
                                       crosses_incognito, params);
-  if (action)
-    return std::move(*action);
+  if (action) {
+    actions.push_back(std::move(std::move(*action)));
+    return actions;
+  }
 
   // Removing headers doesn't require host permissions.
   // Note: If we add other "non-destructive" actions (i.e., actions that don't
   // end the request), we should combine them with the remove-headers action.
-  action = GetRemoveHeadersAction(rulesets_to_evaluate, params);
-  if (action)
-    return std::move(*action);
+  std::vector<Action> remove_headers_actions =
+      GetRemoveHeadersActions(rulesets_to_evaluate, params);
+
+  if (!remove_headers_actions.empty())
+    return remove_headers_actions;
 
-  return Action(Action::Type::NONE);
+  actions.emplace_back(Action::Type::NONE);
+  return actions;
 }
 
 bool RulesetManager::ShouldEvaluateRequest(

extensions/browser/api/declarative_net_request/ruleset_manager.h

@@ -62,10 +62,9 @@ class RulesetManager {
     // Valid iff |type| is |REDIRECT|.
     base::Optional<GURL> redirect_url;
 
-    // The ids of the extensions the action is attributed to.
-    // TODO(crbug.com/991420): This is not exactly correct for attributing
-    // an Action to the extension(s) for |REMOVE_HEADERS| rules.
-    std::vector<ExtensionId> extension_ids;
+    // The id of the extension the action is attributed to. Valid iff |type| is
+    // not |NONE|.
+    base::Optional<ExtensionId> extension_id;
 
     // Valid iff |type| is |REMOVE_HEADERS|. The vectors point to strings of
     // static storage duration.
@@ -113,8 +112,8 @@ class RulesetManager {
   // Precedence order: Allow > Blocking > Redirect rules.
   // For redirect rules, most recently installed extensions are given
   // preference.
-  const Action& EvaluateRequest(const WebRequestInfo& request,
-                                bool is_incognito_context) const;
+  const std::vector<Action>& EvaluateRequest(const WebRequestInfo& request,
+                                             bool is_incognito_context) const;
 
   // Returns true if there is an active matcher which modifies "extraHeaders".
   bool HasAnyExtraHeadersMatcher() const;
@@ -162,13 +161,13 @@ class RulesetManager {
       const int tab_id,
       const bool crosses_incognito,
       const RequestParams& params) const;
-  base::Optional<Action> GetRemoveHeadersAction(
+  std::vector<Action> GetRemoveHeadersActions(
       const std::vector<const ExtensionRulesetData*>& rulesets,
       const RequestParams& params) const;
 
   // Helper for EvaluateRequest.
-  Action EvaluateRequestInternal(const WebRequestInfo& request,
-                                 bool is_incognito_context) const;
+  std::vector<Action> EvaluateRequestInternal(const WebRequestInfo& request,
+                                              bool is_incognito_context) const;
 
   // Returns true if the given |request| should be evaluated for
   // blocking/redirection.

extensions/browser/api/declarative_net_request/ruleset_matcher.cc

@@ -351,8 +351,8 @@ RulesetMatcher::LoadRulesetResult RulesetMatcher::CreateVerifiedMatcher(
 RulesetMatcher::~RulesetMatcher() = default;
 
 uint8_t RulesetMatcher::GetRemoveHeadersMask(const RequestParams& params,
-                                             uint8_t current_mask) const {
-  uint8_t mask = current_mask;
+                                             uint8_t ignored_mask) const {
+  uint8_t mask = 0;
 
   static_assert(kRemoveHeadersMask_Max <= std::numeric_limits<uint8_t>::max(),
                 "RemoveHeadersMask can't fit in a uint8_t");
@@ -365,21 +365,21 @@ uint8_t RulesetMatcher::GetRemoveHeadersMask(const RequestParams& params,
         break;
       case dnr_api::REMOVE_HEADER_TYPE_COOKIE:
         bit = kRemoveHeadersMask_Cookie;
-        if (mask & bit)
+        if (ignored_mask & bit)
           break;
         if (GetMatchingRule(params, flat::ActionIndex_remove_cookie_header))
           mask |= bit;
         break;
       case dnr_api::REMOVE_HEADER_TYPE_REFERER:
         bit = kRemoveHeadersMask_Referer;
-        if (mask & bit)
+        if (ignored_mask & bit)
           break;
         if (GetMatchingRule(params, flat::ActionIndex_remove_referer_header))
           mask |= bit;
         break;
       case dnr_api::REMOVE_HEADER_TYPE_SETCOOKIE:
         bit = kRemoveHeadersMask_SetCookie;
-        if (mask & bit)
+        if (ignored_mask & bit)
           break;
         if (GetMatchingRule(params, flat::ActionIndex_remove_set_cookie_header))
           mask |= bit;
@@ -387,6 +387,7 @@ uint8_t RulesetMatcher::GetRemoveHeadersMask(const RequestParams& params,
     }
   }
 
+  DCHECK(!(mask & ignored_mask));
   return mask;
 }
 

extensions/browser/api/declarative_net_request/ruleset_matcher.h

@@ -53,8 +53,7 @@ struct RequestParams {
 // RulesetMatcher encapsulates the Declarative Net Request API ruleset
 // corresponding to a single RulesetSource. This uses the url_pattern_index
 // component to achieve fast matching of network requests against declarative
-// rules. Since this class is immutable, it is thread-safe. In practice it is
-// accessed on the IO thread but created on a sequence where file IO is allowed.
+// rules. Since this class is immutable, it is thread-safe.
 class RulesetMatcher {
  public:
   // Describes the result of creating a RulesetMatcher instance.
@@ -105,10 +104,10 @@ class RulesetMatcher {
   }
 
   // Returns the bitmask of headers to remove from the request. The bitmask
-  // corresponds to RemoveHeadersMask type. |current_mask| denotes the current
-  // mask of headers to be removed and is included in the return value.
+  // 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 current_mask) const;
+                               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

extensions/browser/api/declarative_net_request/ruleset_matcher_unittest.cc

@@ -193,25 +193,30 @@ TEST_F(RulesetMatcherTest, RemoveHeaders) {
   params.url = &example_url;
   params.element_type = url_pattern_index::flat::ElementType_SUBDOCUMENT;
   params.is_third_party = true;
-  EXPECT_EQ(0u, matcher->GetRemoveHeadersMask(params, 0u /* current_mask */));
+  EXPECT_EQ(0u, matcher->GetRemoveHeadersMask(params, 0u /* ignored_mask */));
 
   rule.action->type = std::string("removeHeaders");
   rule.action->remove_headers_list =
-      std::vector<std::string>({"referer", "setCookie"});
+      std::vector<std::string>({"referer", "cookie", "setCookie"});
   ASSERT_TRUE(CreateVerifiedMatcher({rule}, CreateTemporarySource(), &matcher));
   EXPECT_TRUE(matcher->IsExtraHeadersMatcher());
-  EXPECT_EQ(kRemoveHeadersMask_Referer | kRemoveHeadersMask_SetCookie,
-            matcher->GetRemoveHeadersMask(params, 0u /* current_mask */));
+  EXPECT_EQ(kRemoveHeadersMask_Referer | kRemoveHeadersMask_Cookie |
+                kRemoveHeadersMask_SetCookie,
+            matcher->GetRemoveHeadersMask(params, 0u /* ignored_mask */));
 
   GURL google_url("http://google.com");
   params.url = &google_url;
-  EXPECT_EQ(0u, matcher->GetRemoveHeadersMask(params, 0u /* current_mask */));
+  EXPECT_EQ(0u, matcher->GetRemoveHeadersMask(params, 0u /* ignored_mask */));
 
-  // The current mask is ignored while matching and returned as part of the
-  // result.
-  uint8_t current_mask =
+  uint8_t ignored_mask =
       kRemoveHeadersMask_Referer | kRemoveHeadersMask_SetCookie;
-  EXPECT_EQ(current_mask, matcher->GetRemoveHeadersMask(params, current_mask));
+  EXPECT_EQ(0u, matcher->GetRemoveHeadersMask(params, ignored_mask));
+
+  // The current mask is ignored while matching and is not returned as part of
+  // the result.
+  params.url = &example_url;
+  EXPECT_EQ(kRemoveHeadersMask_Cookie,
+            matcher->GetRemoveHeadersMask(params, ignored_mask));
 }
 
 // Tests a rule to redirect to an extension path.

extensions/browser/api/web_request/web_request_api_helpers.cc

@@ -325,6 +325,40 @@ static_assert(static_cast<size_t>(ResponseHeaderType::kMaxValue) - 1 ==
 static_assert(ValidateHeaderEntries(kResponseHeaderEntries),
               "Invalid response header entries");
 
+bool HasMatchingRemovedDNRRequestHeader(
+    const extensions::WebRequestInfo& request,
+    const std::string& header) {
+  for (const auto& action : request.dnr_actions) {
+    if (std::find_if(action.request_headers_to_remove.begin(),
+                     action.request_headers_to_remove.end(),
+                     [&header](const char* header_to_remove) {
+                       return base::EqualsCaseInsensitiveASCII(header_to_remove,
+                                                               header);
+                     }) != action.request_headers_to_remove.end()) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+bool HasMatchingRemovedDNRResponseHeader(
+    const extensions::WebRequestInfo& request,
+    const std::string& header) {
+  for (const auto& action : request.dnr_actions) {
+    if (std::find_if(action.response_headers_to_remove.begin(),
+                     action.response_headers_to_remove.end(),
+                     [&header](const char* header_to_remove) {
+                       return base::EqualsCaseInsensitiveASCII(
+                           header, header_to_remove);
+                     }) != action.response_headers_to_remove.end()) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 }  // namespace
 
 IgnoredAction::IgnoredAction(extensions::ExtensionId extension_id,
@@ -934,14 +968,8 @@ void MergeOnBeforeSendHeadersResponses(
 
         // Prevent extensions from adding any header removed by the Declarative
         // Net Request API.
-        DCHECK(request.dnr_action.has_value());
-        if (std::find_if(request.dnr_action->request_headers_to_remove.begin(),
-                         request.dnr_action->request_headers_to_remove.end(),
-                         [&key](const char* header_to_remove) {
-                           return base::EqualsCaseInsensitiveASCII(
-                               header_to_remove, key);
-                         }) !=
-            request.dnr_action->request_headers_to_remove.end()) {
+        DCHECK(!request.dnr_actions.empty());
+        if (HasMatchingRemovedDNRRequestHeader(request, key)) {
           extension_conflicts = true;
           break;
         }
@@ -1312,17 +1340,11 @@ void MergeOnHeadersReceivedResponses(
 
     // Prevent extensions from adding any response header which was removed by
     // the Declarative Net Request API.
-    DCHECK(request.dnr_action.has_value());
-    if (!extension_conflicts &&
-        !request.dnr_action->response_headers_to_remove.empty()) {
+    DCHECK(!request.dnr_actions.empty());
+
+    if (!extension_conflicts) {
       for (const ResponseHeader& header : delta.added_response_headers) {
-        if (std::find_if(request.dnr_action->response_headers_to_remove.begin(),
-                         request.dnr_action->response_headers_to_remove.end(),
-                         [&header](const char* header_to_remove) {
-                           return base::EqualsCaseInsensitiveASCII(
-                               header.first, header_to_remove);
-                         }) !=
-            request.dnr_action->response_headers_to_remove.end()) {
+        if (HasMatchingRemovedDNRResponseHeader(request, header.first)) {
           extension_conflicts = true;
           break;
         }

extensions/browser/api/web_request/web_request_info.h

@@ -168,8 +168,10 @@ struct WebRequestInfo {
 
   // The Declarative Net Request action associated with this request. Mutable
   // since this is lazily computed. Cached to avoid redundant computations.
-  mutable base::Optional<declarative_net_request::RulesetManager::Action>
-      dnr_action;
+  // Valid when non-empty. In case no action is taken, populated with
+  // Action::Type::NONE.
+  mutable std::vector<declarative_net_request::RulesetManager::Action>
+      dnr_actions;
 
   const bool is_service_worker_script;