support exception rules in the custom filters box of brave://adblock

brave · Feb 1, 2021 · bd1a6e9 · bd1a6e9
1 parent 9b5e404
commit bd1a6e9
Show file tree

Hide file tree

Showing 17 changed files with 157 additions and 121 deletions.
diff --git a/DEPS b/DEPS
@@ -1,7 +1,7 @@
 use_relative_paths = True
 
 deps = {
- "vendor/adblock_rust_ffi": "https://github.com/brave/adblock-rust-ffi.git@8c5103fd864e23cb10e0a8e0ec1169bc6f6246bf",
+ "vendor/adblock_rust_ffi": "https://github.com/brave/adblock-rust-ffi.git@dee5183b9b9d8f60d1aa71a69f6c663f737b791e",
  "vendor/extension-whitelist": "https://github.com/brave/extension-whitelist.git@b4d059c73042cacf3a5e9156d4b1698e7bc18678",
  "vendor/hashset-cpp": "https://github.com/brave/hashset-cpp.git@6eab0271d014ff09bd9f38abe1e0c117e13e9aa9",
  "vendor/requests": "https://github.com/kennethreitz/requests@e4d59bedfd3c7f4f254f4f5d036587bcd8152458",

diff --git a/browser/brave_shields/ad_block_service_browsertest.cc b/browser/brave_shields/ad_block_service_browsertest.cc
@@ -315,6 +315,44 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, AdsGetBlockedByCustomBlocker) {
  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL);
 }
 
+// Load a page with an ad image, with a corresponding exception installed in
+// the custom filters, and make sure it is not blocked.
+IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, DefaultBlockCustomException) {
+ EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
+ UpdateAdBlockInstanceWithRules("*ad_banner.png");
+ ASSERT_TRUE(g_brave_browser_process->ad_block_custom_filters_service()
+ ->UpdateCustomFilters("@@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);
+}
+
+// Load a page with an image blocked by custom filters, with a corresponding
+// exception installed in the default filters, and make sure it is not blocked.
+IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CustomBlockDefaultException) {
+ EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
+ UpdateAdBlockInstanceWithRules("@@ad_banner.png");
+ ASSERT_TRUE(g_brave_browser_process->ad_block_custom_filters_service()
+ ->UpdateCustomFilters("*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);
+}
+
 // Load a page with an image which is not an ad, and make sure it is NOT
 // blocked.
 IN_PROC_BROWSER_TEST_F(AdBlockServiceTest,

diff --git a/browser/net/brave_ad_block_tp_network_delegate_helper.cc b/browser/net/brave_ad_block_tp_network_delegate_helper.cc
@@ -56,29 +56,39 @@ content::WebContents* GetWebContents(int render_process_id,
 
 void ShouldBlockAdOnTaskRunner(std::shared_ptr<BraveRequestInfo> ctx,
  base::Optional<std::string> canonical_name) {
+ bool did_match_rule = false;
  bool did_match_exception = false;
+ bool did_match_important = false;
  if (!ctx->initiator_url.is_valid()) {
  return;
  }
  std::string source_host = ctx->initiator_url.host();
- if (!g_brave_browser_process->ad_block_service()->ShouldStartRequest(
- ctx->request_url, ctx->resource_type, source_host,
- &did_match_exception, &ctx->mock_data_url)) {
+
+ g_brave_browser_process->ad_block_service()->ShouldStartRequest(
+ ctx->request_url, ctx->resource_type, source_host,
+ &did_match_rule, &did_match_exception, &did_match_important,
+ &ctx->mock_data_url);
+ if (did_match_important) {
  ctx->blocked_by = kAdBlocked;
- } else if (!did_match_exception && canonical_name.has_value() &&
- ctx->request_url.host() != *canonical_name &&
- *canonical_name != "") {
+ return;
+ }
+
+ if (canonical_name.has_value() && ctx->request_url.host() != *canonical_name
+ && *canonical_name != "") {
  GURL::Replacements replacements = GURL::Replacements();
  replacements.SetHost(
  canonical_name->c_str(),
  url::Component(0, static_cast<int>(canonical_name->length())));
  const GURL canonical_url = ctx->request_url.ReplaceComponents(replacements);
 
- if (!g_brave_browser_process->ad_block_service()->ShouldStartRequest(
- canonical_url, ctx->resource_type, source_host,
- &did_match_exception, &ctx->mock_data_url)) {
- ctx->blocked_by = kAdBlocked;
- }
+ g_brave_browser_process->ad_block_service()->ShouldStartRequest(
+ ctx->request_url, ctx->resource_type, source_host,
+ &did_match_rule, &did_match_exception, &did_match_important,
+ &ctx->mock_data_url);
+ }
+
+ if (did_match_important || (did_match_rule && !did_match_exception)) {
+ ctx->blocked_by = kAdBlocked;
  }
 }
 

diff --git a/build/rust/Cargo.lock b/build/rust/Cargo.lock
diff --git a/components/brave_shields/browser/ad_block_base_service.cc b/components/brave_shields/browser/ad_block_base_service.cc
@@ -21,7 +21,7 @@
 #include "brave/common/pref_names.h"
 #include "brave/components/brave_component_updater/browser/dat_file_util.h"
 #include "brave/components/brave_shields/common/brave_shield_constants.h"
-#include "brave/vendor/adblock_rust_ffi/src/wrapper.hpp"
+#include "brave/vendor/adblock_rust_ffi/src/wrapper.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
@@ -113,11 +113,13 @@ AdBlockBaseService::~AdBlockBaseService() {
  GetTaskRunner()->DeleteSoon(FROM_HERE, ad_block_client_.release());
 }
 
-bool AdBlockBaseService::ShouldStartRequest(
+void AdBlockBaseService::ShouldStartRequest(
  const GURL& url,
  blink::mojom::ResourceType resource_type,
  const std::string& tab_host,
+ bool* did_match_rule,
  bool* did_match_exception,
+ bool* did_match_important,
  std::string* mock_data_url) {
  DCHECK(GetTaskRunner()->RunsTasksInCurrentSequence());
 
@@ -128,27 +130,15 @@ bool AdBlockBaseService::ShouldStartRequest(
  url,
  url::Origin::CreateFromNormalizedTuple("https", tab_host.c_str(), 80),
  INCLUDE_PRIVATE_REGISTRIES);
- bool saved_from_exception;
- if (ad_block_client_->matches(url.spec(), url.host(), tab_host,
- is_third_party,
- ResourceTypeToString(resource_type),
- &saved_from_exception, mock_data_url)) {
- // We'd only possibly match an exception filter if we're returning true.
- if (did_match_exception) {
- *did_match_exception = false;
- }
- // LOG(ERROR) << "AdBlockBaseService::ShouldStartRequest(), host: "
- // << tab_host
- // << ", resource type: " << resource_type
- // << ", url.spec(): " << url.spec();
- return false;
- }
+ ad_block_client_->matches(
+ url.spec(), url.host(), tab_host, is_third_party,
+ ResourceTypeToString(resource_type), did_match_rule,
+ did_match_exception, did_match_important, mock_data_url);
 
- if (did_match_exception) {
- *did_match_exception = saved_from_exception;
- }
-
- return true;
+ // LOG(ERROR) << "AdBlockBaseService::ShouldStartRequest(), host: "
+ // << tab_host
+ // << ", resource type: " << resource_type
+ // << ", url.spec(): " << url.spec();
 }
 
 void AdBlockBaseService::EnableTag(const std::string& tag, bool enabled) {

diff --git a/components/brave_shields/browser/ad_block_base_service.h b/components/brave_shields/browser/ad_block_base_service.h
@@ -40,10 +40,12 @@ class AdBlockBaseService : public BaseBraveShieldsService {
  explicit AdBlockBaseService(BraveComponent::Delegate* delegate);
  ~AdBlockBaseService() override;
 
- bool ShouldStartRequest(const GURL& url,
+ void ShouldStartRequest(const GURL& url,
  blink::mojom::ResourceType resource_type,
  const std::string& tab_host,
+ bool* did_match_rule,
  bool* did_match_exception,
+ bool* did_match_important,
  std::string* mock_data_url) override;
  void AddResources(const std::string& resources);
  void EnableTag(const std::string& tag, bool enabled);

diff --git a/components/brave_shields/browser/ad_block_custom_filters_service.cc b/components/brave_shields/browser/ad_block_custom_filters_service.cc
@@ -9,7 +9,7 @@
 #include "brave/browser/brave_browser_process_impl.h"
 #include "brave/common/pref_names.h"
 #include "brave/components/brave_shields/browser/ad_block_service.h"
-#include "brave/vendor/adblock_rust_ffi/src/wrapper.hpp"
+#include "brave/vendor/adblock_rust_ffi/src/wrapper.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_thread.h"