address review comments

browser/brave_shields/ad_block_service_browsertest.cc

@@ -72,6 +72,7 @@ const char kRegionalAdBlockComponentTest64PublicKey[] =
 using brave_shields::features::kBraveAdblockCnameUncloaking;
 using brave_shields::features::kBraveAdblockCollapseBlockedElements;
 using brave_shields::features::kBraveAdblockCosmeticFiltering;
+using brave_shields::features::kBraveAdblockDefault1pBlocking;
 using content::BrowserThread;
 
 void AdBlockServiceTest::SetUpOnMainThread() {
@@ -1138,6 +1139,90 @@ IN_PROC_BROWSER_TEST_F(CollapseBlockedElementsFlagDisabledTest,
                          "i[0].clientHeight !== 0"));
 }
 
+class Default1pBlockingFlagDisabledTest : public AdBlockServiceTest {
+ public:
+  Default1pBlockingFlagDisabledTest() {
+    feature_list_.InitAndDisableFeature(kBraveAdblockDefault1pBlocking);
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+// Load a page with an image from a first party and a third party, which both
+// match the same filter in the default engine. Ensure the third-party one is
+// blocked while the first-party one is allowed.
+IN_PROC_BROWSER_TEST_F(Default1pBlockingFlagDisabledTest, Default1pBlocking) {
+  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
+  UpdateAdBlockInstanceWithRules("^ad_banner.png");
+
+  GURL url = embedded_test_server()->GetURL(kAdBlockTestPage);
+  ui_test_utils::NavigateToURL(browser(), url);
+  content::WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  ASSERT_EQ(true, EvalJs(contents,
+                         "setExpectations(1, 0, 0, 0);"
+                         "addImage('ad_banner.png')"));
+  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
+
+  ASSERT_EQ(true, EvalJs(contents,
+                         "setExpectations(1, 1, 0, 0);"
+                         "addImage('https://thirdparty.com/ad_banner.png')"));
+  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL);
+}
+
+// Load a page with an image from a first party and a third party, which both
+// match the same filter in the default engine. Enable aggressive mode, and
+// ensure that both are blocked.
+IN_PROC_BROWSER_TEST_F(Default1pBlockingFlagDisabledTest,
+                       Aggressive1pBlocking) {
+  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
+  brave_shields::SetCosmeticFilteringControlType(
+      content_settings(), brave_shields::ControlType::BLOCK, GURL());
+  UpdateAdBlockInstanceWithRules("^ad_banner.png");
+
+  GURL url = embedded_test_server()->GetURL(kAdBlockTestPage);
+  ui_test_utils::NavigateToURL(browser(), url);
+  content::WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  ASSERT_EQ(true, EvalJs(contents,
+                         "setExpectations(0, 1, 0, 0);"
+                         "addImage('ad_banner.png')"));
+  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL);
+
+  ASSERT_EQ(true, EvalJs(contents,
+                         "setExpectations(0, 2, 0, 0);"
+                         "addImage('https://thirdparty.com/ad_banner.png')"));
+  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 2ULL);
+}
+
+// Load a page with an image from a first party and a third party, which both
+// match the same filter in the custom filters engine. Ensure that both are
+// blocked.
+IN_PROC_BROWSER_TEST_F(Default1pBlockingFlagDisabledTest, Custom1pBlocking) {
+  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
+  ASSERT_TRUE(g_brave_browser_process->ad_block_custom_filters_service()
+                  ->UpdateCustomFilters("^ad_banner.png"));
+  WaitForAdBlockServiceThreads();
+
+  GURL url = embedded_test_server()->GetURL(kAdBlockTestPage);
+  ui_test_utils::NavigateToURL(browser(), url);
+  content::WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  ASSERT_EQ(true, EvalJs(contents,
+                         "setExpectations(0, 1, 0, 0);"
+                         "addImage('ad_banner.png')"));
+  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL);
+
+  ASSERT_EQ(true, EvalJs(contents,
+                         "setExpectations(0, 2, 0, 0);"
+                         "addImage('https://thirdparty.com/ad_banner.png')"));
+  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 2ULL);
+}
+
 // Load a page with a script which uses a redirect data URL.
 IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, RedirectRulesAreRespected) {
   UpdateAdBlockInstanceWithRules("js_mock_me.js$redirect=noopjs",

browser/net/url_context.cc

@@ -138,6 +138,8 @@ std::shared_ptr<brave::BraveRequestInfo> BraveRequestInfo::MakeCTX(
       brave_shields::GetBraveShieldsEnabled(map, ctx->tab_origin);
   ctx->allow_ads = brave_shields::GetAdControlType(map, ctx->tab_origin) ==
                    brave_shields::ControlType::ALLOW;
+  // Currently, "aggressive" mode is registered as a cosmetic filtering control
+  // type, even though it can also affect network blocking.
   ctx->aggressive_blocking =
       brave_shields::GetCosmeticFilteringControlType(map, ctx->tab_origin) ==
       brave_shields::ControlType::BLOCK;

browser/net/url_context.h

@@ -76,6 +76,8 @@ struct BraveRequestInfo {
   // TODO(iefremov): rename to shields_up.
   bool allow_brave_shields = true;
   bool allow_ads = false;
+  // Whether or not Shields "aggressive" mode was enabled where the request was
+  // initiated.
   bool aggressive_blocking = false;
   bool allow_http_upgradable_resource = false;
   bool allow_referrers = false;

components/brave_shields/browser/ad_block_service.cc

@@ -75,8 +75,7 @@ void AdBlockService::ShouldStartRequest(
       base::FeatureList::IsEnabled(
           brave_shields::features::kBraveAdblockDefault1pBlocking) ||
       !SameDomainOrHost(
-          url,
-          url::Origin::CreateFromNormalizedTuple("https", tab_host.c_str(), 80),
+          url, url::Origin::CreateFromNormalizedTuple("https", tab_host, 80),
           net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
     AdBlockBaseService::ShouldStartRequest(
         url, resource_type, tab_host, aggressive_blocking, did_match_rule,

components/brave_shields/browser/base_brave_shields_service.cc

@@ -57,7 +57,6 @@ void BaseBraveShieldsService::ShouldStartRequest(
     bool* did_match_rule,
     bool* did_match_exception,
     bool* did_match_important,
-    std::string* mock_data_url) {
-}
+    std::string* mock_data_url) {}
 
 }  // namespace brave_shields

components/brave_shields/common/features.cc

@@ -10,8 +10,10 @@
 namespace brave_shields {
 namespace features {
 
-// When disabled, Brave will allow first-party requests in "standard" blocking
-// mode regardless of whether or not they appear in a filter list.
+// When enabled, Brave will block first-party requests that appear in a filter
+// list when Shields is in "standard" blocking mode. When disabled, Brave will
+// allow first-party requests in "standard" blocking mode regardless of whether
+// or not they appear in a filter list.
 const base::Feature kBraveAdblockDefault1pBlocking{
     "BraveAdblockDefault1pBlocking", base::FEATURE_ENABLED_BY_DEFAULT};
 // When enabled, Brave will issue DNS queries for requests that the adblock