Explicitly cancel blocked requests (attempt 2)

browser/net/brave_ad_block_tp_network_delegate_helper.cc

@@ -60,7 +60,7 @@ void ShouldBlockAdOnTaskRunner(std::shared_ptr<BraveRequestInfo> ctx,
   std::string tab_host = ctx->tab_origin.host();
   if (!g_brave_browser_process->ad_block_service()->ShouldStartRequest(
           ctx->request_url, ctx->resource_type, tab_host, &did_match_exception,
-          &ctx->cancel_request_explicitly, &ctx->mock_data_url)) {
+          &ctx->mock_data_url)) {
     ctx->blocked_by = kAdBlocked;
   } else if (!did_match_exception && canonical_name.has_value() &&
              ctx->request_url.host() != *canonical_name &&
@@ -73,7 +73,7 @@ void ShouldBlockAdOnTaskRunner(std::shared_ptr<BraveRequestInfo> ctx,
 
     if (!g_brave_browser_process->ad_block_service()->ShouldStartRequest(
             canonical_url, ctx->resource_type, tab_host, &did_match_exception,
-            &ctx->cancel_request_explicitly, &ctx->mock_data_url)) {
+            &ctx->mock_data_url)) {
       ctx->blocked_by = kAdBlocked;
     }
   }

browser/net/brave_proxying_url_loader_factory.cc

@@ -348,10 +348,12 @@ void BraveProxyingURLLoaderFactory::InProgressRequest::
 
   // TODO(iefremov): Shorten
   if (ctx_->blocked_by != brave::kNotBlocked) {
-    if (ctx_->cancel_request_explicitly) {
-      OnRequestError(network::URLLoaderCompletionStatus(net::ERR_ABORTED));
+    if (!ctx_->ShouldMockRequest()) {
+      OnRequestError(
+          network::URLLoaderCompletionStatus(net::ERR_BLOCKED_BY_CLIENT));
       return;
     }
+
     auto response = network::mojom::URLResponseHead::New();
     std::string response_data;
     brave_shields::MakeStubResponse(ctx_->mock_data_url, request_, &response,

browser/net/brave_request_handler.cc

@@ -297,9 +297,9 @@ void BraveRequestHandler::RunNextCallback(
       *ctx->new_url = GURL(ctx->new_url_spec);
     }
     if (ctx->blocked_by == brave::kAdBlocked) {
-      if (ctx->cancel_request_explicitly) {
+      if (!ctx->ShouldMockRequest()) {
         RunCallbackForRequestIdentifier(ctx->request_identifier,
-                                        net::ERR_ABORTED);
+                                        net::ERR_BLOCKED_BY_CLIENT);
         return;
       }
     }

browser/net/url_context.h

@@ -96,10 +96,13 @@ struct BraveRequestInfo {
   BraveNetworkDelegateEventType event_type = kUnknownEventType;
   const base::ListValue* referral_headers_list = nullptr;
   BlockedBy blocked_by = kNotBlocked;
-  bool cancel_request_explicitly = false;
   std::string mock_data_url;
   bool ipfs_local = true;
 
+  bool ShouldMockRequest() const {
+    return !mock_data_url.empty();
+  }
+
   net::NetworkIsolationKey network_isolation_key = net::NetworkIsolationKey();
 
   // Default to invalid type for resource_type, so delegate helpers

components/brave_perf_predictor/browser/perf_predictor_tab_helper_browsertest.cc

@@ -79,10 +79,10 @@ IN_PROC_BROWSER_TEST_F(PerfPredictorTabHelperTest, NoBlockNoSavings) {
   content::WebContents* contents =
       browser()->tab_strip_model()->GetActiveWebContents();
 
-  EXPECT_EQ(true, EvalJsWithManualReply(contents,
-                                    "addImage('logo.png');"
-                                    "setExpectations(0, 0, 0, 0, 1, 0);"
-                                    "xhr('analytics.js')"));
+  EXPECT_EQ(true, EvalJs(contents,
+                         "addImage('logo.png');"
+                         "setExpectations(0, 0, 0, 0, 1, 0);"
+                         "xhr('analytics.js')"));
   // Prediction triggered when web contents are closed
   contents->Close();
   EXPECT_EQ(getProfileBandwidthSaved(browser()), 0ULL);
@@ -98,10 +98,10 @@ IN_PROC_BROWSER_TEST_F(PerfPredictorTabHelperTest, ScriptBlockHasSavings) {
   content::WebContents* contents =
       browser()->tab_strip_model()->GetActiveWebContents();
 
-  EXPECT_EQ(true, EvalJsWithManualReply(contents,
-                                    "addImage('logo.png');"
-                                    "setExpectations(0, 0, 0, 0, 1, 0);"
-                                    "xhr('analytics.js')"));
+  EXPECT_EQ(true, EvalJs(contents,
+                         "addImage('logo.png');"
+                         "setExpectations(0, 0, 0, 0, 0, 1);"
+                         "xhr('analytics.js')"));
 
   EXPECT_EQ(getProfileAdsBlocked(browser()), 1ULL);
   // Prediction triggered when web contents are closed
@@ -119,19 +119,19 @@ IN_PROC_BROWSER_TEST_F(PerfPredictorTabHelperTest, NewNavigationStoresSavings) {
   content::WebContents* contents =
       browser()->tab_strip_model()->GetActiveWebContents();
 
-  EXPECT_EQ(true, EvalJsWithManualReply(contents,
-                                    "addImage('logo.png');"
-                                    "setExpectations(0, 0, 0, 0, 1, 0);"
-                                    "xhr('analytics.js')"));
+  EXPECT_EQ(true, EvalJs(contents,
+                         "addImage('logo.png');"
+                         "setExpectations(0, 0, 0, 0, 0, 1);"
+                         "xhr('analytics.js')"));
   EXPECT_EQ(getProfileAdsBlocked(browser()), 1ULL);
   // Prediction triggered when web contents are closed
   GURL second_url =
       embedded_test_server()->GetURL("example.com", "/blocking.html");
   ui_test_utils::NavigateToURL(browser(), second_url);
-  EXPECT_EQ(true, EvalJsWithManualReply(contents,
-                                    "addImage('logo.png');"
-                                    "setExpectations(0, 0, 0, 0, 1, 0);"
-                                    "xhr('analytics.js')"));
+  EXPECT_EQ(true, EvalJs(contents,
+                         "addImage('logo.png');"
+                         "setExpectations(0, 0, 0, 0, 0, 1);"
+                         "xhr('analytics.js')"));
 
   auto previous_nav_savings = getProfileBandwidthSaved(browser());
   EXPECT_NE(previous_nav_savings, 0ULL);

components/brave_shields/browser/ad_block_base_service.cc

@@ -118,7 +118,6 @@ bool AdBlockBaseService::ShouldStartRequest(
     blink::mojom::ResourceType resource_type,
     const std::string& tab_host,
     bool* did_match_exception,
-    bool* cancel_request_explicitly,
     std::string* mock_data_url) {
   DCHECK(GetTaskRunner()->RunsTasksInCurrentSequence());
 
@@ -129,15 +128,13 @@ bool AdBlockBaseService::ShouldStartRequest(
       url,
       url::Origin::CreateFromNormalizedTuple("https", tab_host.c_str(), 80),
       INCLUDE_PRIVATE_REGISTRIES);
+  // TODO(spinda): Remove explicit_cancel here when removed from adblock-rust.
   bool explicit_cancel;
   bool saved_from_exception;
   if (ad_block_client_->matches(
           url.spec(), url.host(), tab_host, is_third_party,
           ResourceTypeToString(resource_type), &explicit_cancel,
           &saved_from_exception, mock_data_url)) {
-    if (cancel_request_explicitly) {
-      *cancel_request_explicitly = explicit_cancel;
-    }
     // We'd only possibly match an exception filter if we're returning true.
     if (did_match_exception) {
       *did_match_exception = false;

components/brave_shields/browser/ad_block_base_service.h

@@ -44,7 +44,6 @@ class AdBlockBaseService : public BaseBraveShieldsService {
                           blink::mojom::ResourceType resource_type,
                           const std::string& tab_host,
                           bool* did_match_exception,
-                          bool* cancel_request_explicitly,
                           std::string* mock_data_url) override;
   void AddResources(const std::string& resources);
   void EnableTag(const std::string& tag, bool enabled);

components/brave_shields/browser/ad_block_regional_service_manager.cc

@@ -128,13 +128,12 @@ bool AdBlockRegionalServiceManager::ShouldStartRequest(
     blink::mojom::ResourceType resource_type,
     const std::string& tab_host,
     bool* matching_exception_filter,
-    bool* cancel_request_explicitly,
     std::string* mock_data_url) {
   base::AutoLock lock(regional_services_lock_);
   for (const auto& regional_service : regional_services_) {
     if (!regional_service.second->ShouldStartRequest(
             url, resource_type, tab_host, matching_exception_filter,
-            cancel_request_explicitly, mock_data_url)) {
+            mock_data_url)) {
       return false;
     }
     if (matching_exception_filter && *matching_exception_filter) {

components/brave_shields/browser/ad_block_regional_service_manager.h

@@ -51,7 +51,6 @@ class AdBlockRegionalServiceManager {
                           blink::mojom::ResourceType resource_type,
                           const std::string& tab_host,
                           bool* matching_exception_filter,
-                          bool* cancel_request_explicitly,
                           std::string* mock_data_url);
   void EnableTag(const std::string& tag, bool enabled);
   void AddResources(const std::string& resources);

components/brave_shields/browser/ad_block_service.cc

@@ -77,30 +77,26 @@ bool AdBlockService::ShouldStartRequest(
     blink::mojom::ResourceType resource_type,
     const std::string& tab_host,
     bool* did_match_exception,
-    bool* cancel_request_explicitly,
     std::string* mock_data_url) {
 
   if (!AdBlockBaseService::ShouldStartRequest(
-          url, resource_type, tab_host, did_match_exception,
-          cancel_request_explicitly, mock_data_url)) {
+          url, resource_type, tab_host, did_match_exception, mock_data_url)) {
     return false;
   }
   if (did_match_exception && *did_match_exception) {
     return true;
   }
 
   if (!regional_service_manager()->ShouldStartRequest(
-          url, resource_type, tab_host, did_match_exception,
-          cancel_request_explicitly, mock_data_url)) {
+          url, resource_type, tab_host, did_match_exception, mock_data_url)) {
     return false;
   }
   if (did_match_exception && *did_match_exception) {
     return true;
   }
 
   if (!custom_filters_service()->ShouldStartRequest(
-        url, resource_type, tab_host, did_match_exception,
-        cancel_request_explicitly, mock_data_url)) {
+        url, resource_type, tab_host, did_match_exception, mock_data_url)) {
     return false;
   }
   if (did_match_exception && *did_match_exception) {

components/brave_shields/browser/ad_block_service.h

@@ -50,7 +50,6 @@ class AdBlockService : public AdBlockBaseService {
                           blink::mojom::ResourceType resource_type,
                           const std::string& tab_host,
                           bool* did_match_exception,
-                          bool* cancel_request_explicitly,
                           std::string* mock_data_url) override;
 
   AdBlockRegionalServiceManager* regional_service_manager();

components/brave_shields/browser/base_brave_shields_service.cc

@@ -54,12 +54,10 @@ bool BaseBraveShieldsService::ShouldStartRequest(
     blink::mojom::ResourceType resource_type,
     const std::string& tab_host,
     bool* did_match_exception,
-    bool* cancel_request_explicitly,
     std::string* mock_data_url) {
   if (did_match_exception) {
     *did_match_exception = false;
   }
-  // Intentionally don't set cancel_request_explicitly
   return true;
 }
 

components/brave_shields/browser/base_brave_shields_service.h

@@ -35,7 +35,6 @@ class BaseBraveShieldsService : public BraveComponent {
                                   blink::mojom::ResourceType resource_type,
                                   const std::string& tab_host,
                                   bool* did_match_exception,
-                                  bool* cancel_request_explicitly,
                                   std::string* mock_data_url);
 
  protected:

components/brave_shields/browser/tracking_protection_service.cc

@@ -197,8 +197,7 @@ bool TrackingProtectionService::ShouldStartRequest(
     const GURL& url,
     blink::mojom::ResourceType resource_type,
     const std::string& tab_host,
-    bool* matching_exception_filter,
-    bool* cancel_request_explicitly) {
+    bool* matching_exception_filter) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   // There are no exceptions in the TP service, but exceptions are
   // combined with brave/ad-block.

components/brave_shields/browser/tracking_protection_service.h

@@ -43,8 +43,7 @@ class TrackingProtectionService : public LocalDataFilesObserver {
   bool ShouldStartRequest(const GURL& spec,
                           blink::mojom::ResourceType resource_type,
                           const std::string& tab_host,
-                          bool* matching_exception_filter,
-                          bool* cancel_request_explicitly);
+                          bool* matching_exception_filter);
 
   // implementation of LocalDataFilesObserver
   void OnComponentReady(const std::string& component_id,

test/data/blocking.html

@@ -16,66 +16,68 @@
 
 // Performs an XHR for the specified src
 function xhr(src) {
-  const xhr = new XMLHttpRequest();
-  xhr.open('GET', src, true);
-  xhr.onload = function(e) {
-    if (xhr.response.length !== 0) {
-      numXHRLoaded++
-    } else {
-      numXHRLoadedBlank++
+  return new Promise(resolve => {
+    const xhr = new XMLHttpRequest();
+    xhr.open('GET', src, true);
+    xhr.onload = function(e) {
+      if (xhr.response.length !== 0) {
+        numXHRLoaded++
+      } else {
+        numXHRLoadedBlank++
+      }
+      resolve(onLoad())
     }
-    onLoad()
-  }
-  xhr.onerror = function(e) {
-    numXHRBlocked++
-    onLoad()
-  }
-  xhr.send();
+    xhr.onerror = function(e) {
+      numXHRBlocked++
+      resolve(onLoad())
+    }
+    xhr.send();
+  });
 }
 
 // Performs an XHR for the specified src and reports successful,
 // only if the content matches expected_content.
 function xhr_expect_content(src, expected_content) {
-  const xhr = new XMLHttpRequest();
-  xhr.open('GET', src, true);
-  xhr.onload = function(e) {
-    if (xhr.response === expected_content) {
-      numXHRLoaded++
+  return new Promise(resolve => {
+    const xhr = new XMLHttpRequest();
+    xhr.open('GET', src, true);
+    xhr.onload = function(e) {
+      if (xhr.response === expected_content) {
+        numXHRLoaded++
+      }
+      resolve(onLoad())
     }
-    onLoad()
-  }
-  xhr.onerror = function(e) {
-    console.log(e)
-    numXHRBlocked++
-    onLoad()
-  }
-  xhr.send();
+    xhr.onerror = function(e) {
+      console.log(e)
+      numXHRBlocked++
+      resolve(onLoad())
+    }
+    xhr.send();
+  });
 }
 
 // Adds an image to the DOM with the specified src
 function addImage(src) {
-  const img = document.createElement('img')
-  img.src = src
-  img.className = 'adImage'
-  img.addEventListener('load', onLoad)
-  img.addEventListener('error', () => {
-    onLoad()
-  })
+  return new Promise(resolve => {
+    const img = document.createElement('img')
+    img.src = src
+    img.className = 'adImage'
 
-  img.addEventListener('load', () => {
-    if (img.complete &&
-        (img.naturalHeight !== 1 || img.naturalWidth !== 1)) {
-      numImgLoaded++
-    } else {
-      numImgLoadedBlank++
-    }
-    onLoad()
-  })
-  img.addEventListener('error', () => {
-    numImgBlocked++
-    onLoad()
-  })
-  document.body.appendChild(img)
+    img.addEventListener('load', () => {
+      if (img.complete &&
+          (img.naturalHeight !== 1 || img.naturalWidth !== 1)) {
+        numImgLoaded++
+      } else {
+        numImgLoadedBlank++
+      }
+      resolve(onLoad())
+    })
+    img.addEventListener('error', () => {
+      numImgBlocked++
+      resolve(onLoad())
+    })
+    document.body.appendChild(img)
+  });
 }
 
 // Sets the expectation for the number of images and XHR loaded and blocked
@@ -102,7 +104,7 @@
   // For debugging:
   // console.log('sending: ' + JSON.stringify({result, expectedImgLoaded, expectedImgLoadedBlank, expectedImgBlocked, numImgLoadedBlank, numImgBlocked,
   //   numXHRLoaded, expectedXHRLoaded, numXHRLoadedBlank, numXHRBlocked, expectedXHRLoadedBlank, expectedXHRBlocked}))
-  window.domAutomationController.send(result)
+  return result;
 }
 </script>
 </head>