Address Web Discovery feedback

browser/brave_tab_helpers.cc

@@ -71,12 +71,6 @@
 #include "brave/components/ai_chat/content/browser/ai_chat_tab_helper.h"
 #endif  // BUILDFLAG(ENABLE_AI_CHAT)
 
-#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
-#include "brave/browser/web_discovery/web_discovery_service_factory.h"
-#include "brave/components/web_discovery/browser/web_discovery_tab_helper.h"
-#include "brave/components/web_discovery/common/features.h"
-#endif
-
 #if BUILDFLAG(ENABLE_WIDEVINE)
 #include "brave/browser/brave_drm_tab_helper.h"
 #endif
@@ -171,7 +165,7 @@ void AttachTabHelpers(content::WebContents* web_contents) {
   psst::PsstTabHelper::MaybeCreateForWebContents(
       web_contents, ISOLATED_WORLD_ID_BRAVE_INTERNAL);
 #if BUILDFLAG(ENABLE_EXTENSIONS) || BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
-  WebDiscoveryTabHelper::MaybeCreateForWebContents(web_contents);
+  web_discovery::WebDiscoveryTabHelper::MaybeCreateForWebContents(web_contents);
 #endif
 
 #if BUILDFLAG(ENABLE_SPEEDREADER)
@@ -219,19 +213,6 @@ void AttachTabHelpers(content::WebContents* web_contents) {
     }
   }
 #endif  // BUILDFLAG(ENABLE_PLAYLIST)
-
-#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
-  if (base::FeatureList::IsEnabled(
-          web_discovery::features::kBraveWebDiscoveryNative)) {
-    auto* web_discovery_service =
-        web_discovery::WebDiscoveryServiceFactory::GetForBrowserContext(
-            web_contents->GetBrowserContext());
-    if (web_discovery_service) {
-      web_discovery::WebDiscoveryTabHelper::CreateForWebContents(
-          web_contents, web_discovery_service);
-    }
-  }
-#endif
 }
 
 }  // namespace brave

browser/sources.gni

@@ -364,7 +364,6 @@ if (enable_web_discovery_native) {
   brave_chrome_browser_deps += [
     "//brave/browser/web_discovery",
     "//brave/components/web_discovery/browser",
-    "//brave/components/web_discovery/browser:tab_helper",
     "//brave/components/web_discovery/common",
   ]
 }

browser/web_discovery/BUILD.gn

@@ -53,9 +53,17 @@ source_set("browser_tests") {
     testonly = true
     defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]
 
-    sources = [ "web_discovery_browsertest.cc" ]
+    sources = [
+      "content_scraper_browsertest.cc",
+      "web_discovery_browsertest.cc",
+    ]
     deps = [
+      "//base/test:test_support",
+      "//brave/components/constants",
       "//brave/components/search_engines",
+      "//brave/components/web_discovery/browser",
+      "//brave/components/web_discovery/common",
+      "//brave/components/web_discovery/common:mojom",
       "//chrome/browser",
       "//chrome/browser/profiles:profile",
       "//chrome/browser/ui",
@@ -64,6 +72,8 @@ source_set("browser_tests") {
       "//components/prefs",
       "//content/public/browser",
       "//content/test:test_support",
+      "//net:test_support",
+      "//services/service_manager/public/cpp",
       "//testing/gmock",
       "//testing/gtest",
     ]

components/web_discovery/browser/content_scraper_browsertest.cc → browser/web_discovery/content_scraper_browsertest.cc

@@ -168,14 +168,12 @@ class WebDiscoveryContentScraperTest : public PlatformBrowserTest {
 
     server_config_loader_->SetLastPatternsForTesting(std::move(patterns_group));
 
-    scraper_ = std::make_unique<ContentScraper>(server_config_loader_.get(),
-                                                &regex_util_);
+    scraper_ = std::make_unique<ContentScraper>(server_config_loader_.get());
   }
 
   content::ContentMockCertVerifier mock_cert_verifier_;
   net::EmbeddedTestServer test_server_{net::EmbeddedTestServer::TYPE_HTTPS};
   base::test::ScopedFeatureList scoped_features_;
-  RegexUtil regex_util_;
   std::unique_ptr<ServerConfigLoader> server_config_loader_;
 };
 

browser/web_discovery/web_discovery_browsertest.cc

@@ -14,6 +14,8 @@
 #include "content/public/test/browser_test.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
+namespace web_discovery {
+
 using WebDiscoveryTest = InProcessBrowserTest;
 using ::testing::_;
 
@@ -41,3 +43,5 @@ IN_PROC_BROWSER_TEST_F(WebDiscoveryTest, InfobarAddedTest) {
   tab_helper->ShowInfoBar(browser()->profile()->GetPrefs());
   infobar_manager->RemoveObserver(&observer);
 }
+
+}  // namespace web_discovery

browser/web_discovery/web_discovery_tab_helper.cc

@@ -6,6 +6,7 @@
 #include "brave/browser/web_discovery/web_discovery_tab_helper.h"
 
 #include <memory>
+#include <utility>
 
 #include "base/time/time.h"
 #include "brave/browser/web_discovery/web_discovery_cta_util.h"
@@ -24,6 +25,14 @@ std::unique_ptr<infobars::InfoBar> CreateWebDiscoveryInfoBar(
     std::unique_ptr<WebDiscoveryInfoBarDelegate> delegate);
 #endif
 
+#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
+#include "brave/browser/web_discovery/web_discovery_service_factory.h"
+#include "brave/components/web_discovery/common/features.h"
+#include "services/service_manager/public/cpp/interface_provider.h"
+#endif
+
+namespace web_discovery {
+
 // static
 void WebDiscoveryTabHelper::MaybeCreateForWebContents(
     content::WebContents* contents) {
@@ -41,13 +50,24 @@ void WebDiscoveryTabHelper::MaybeCreateForWebContents(
 
 WebDiscoveryTabHelper::WebDiscoveryTabHelper(content::WebContents* contents)
     : content::WebContentsObserver(contents),
-      content::WebContentsUserData<WebDiscoveryTabHelper>(*contents) {}
+      content::WebContentsUserData<WebDiscoveryTabHelper>(*contents) {
+#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
+  if (base::FeatureList::IsEnabled(features::kBraveWebDiscoveryNative)) {
+    web_discovery_service_ = WebDiscoveryServiceFactory::GetForBrowserContext(
+        contents->GetBrowserContext());
+  }
+#endif
+}
 
 WebDiscoveryTabHelper::~WebDiscoveryTabHelper() = default;
 
 void WebDiscoveryTabHelper::DidFinishLoad(
     content::RenderFrameHost* render_frame_host,
     const GURL& validated_url) {
+#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
+  MaybeExtractFromPage(render_frame_host, validated_url);
+#endif
+
   if (validated_url.host() != kBraveSearchHost) {
     return;
   }
@@ -89,4 +109,26 @@ void WebDiscoveryTabHelper::ShowInfoBar(PrefService* prefs) {
 #endif
 }
 
+#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
+void WebDiscoveryTabHelper::MaybeExtractFromPage(
+    content::RenderFrameHost* render_frame_host,
+    const GURL& url) {
+  if (!web_discovery_service_) {
+    return;
+  }
+  if (!render_frame_host->IsInPrimaryMainFrame()) {
+    return;
+  }
+  if (!web_discovery_service_->ShouldExtractFromPage(url, render_frame_host)) {
+    return;
+  }
+  mojo::Remote<mojom::DocumentExtractor> remote;
+  render_frame_host->GetRemoteInterfaces()->GetInterface(
+      remote.BindNewPipeAndPassReceiver());
+  web_discovery_service_->StartExtractingFromPage(url, std::move(remote));
+}
+#endif
+
 WEB_CONTENTS_USER_DATA_KEY_IMPL(WebDiscoveryTabHelper);
+
+}  // namespace web_discovery

browser/web_discovery/web_discovery_tab_helper.h

@@ -7,11 +7,19 @@
 #define BRAVE_BROWSER_WEB_DISCOVERY_WEB_DISCOVERY_TAB_HELPER_H_
 
 #include "base/gtest_prod_util.h"
+#include "brave/components/web_discovery/buildflags/buildflags.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 
+#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
+#include "base/memory/raw_ptr.h"
+#include "brave/components/web_discovery/browser/web_discovery_service.h"
+#endif
+
 class PrefService;
 
+namespace web_discovery {
+
 class WebDiscoveryTabHelper
     : public content::WebContentsObserver,
       public content::WebContentsUserData<WebDiscoveryTabHelper> {
@@ -36,7 +44,16 @@ class WebDiscoveryTabHelper
 
   void ShowInfoBar(PrefService* prefs);
 
+#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
+  void MaybeExtractFromPage(content::RenderFrameHost* render_frame_host,
+                            const GURL& url);
+
+  raw_ptr<WebDiscoveryService> web_discovery_service_ = nullptr;
+#endif
+
   WEB_CONTENTS_USER_DATA_KEY_DECL();
 };
 
+}  // namespace web_discovery
+
 #endif  // BRAVE_BROWSER_WEB_DISCOVERY_WEB_DISCOVERY_TAB_HELPER_H_

chromium_src/chrome/renderer/DEPS

@@ -8,6 +8,7 @@ include_rules = [
   "+brave/components/content_settings/renderer",
   "+brave/components/tor/buildflags",
   "+brave/components/tor/renderer",
+  "+brave/components/web_discovery/buildflags",
   "+brave/components/web_discovery/common",
   "+brave/components/web_discovery/renderer",
   "+brave/renderer",

chromium_src/chrome/renderer/chrome_content_renderer_client.cc

@@ -7,7 +7,7 @@
 #include "brave/components/ai_chat/core/common/buildflags/buildflags.h"
 #include "brave/components/ai_rewriter/common/buildflags/buildflags.h"
 #include "brave/components/content_settings/renderer/brave_content_settings_agent_impl.h"
-#include "brave/components/web_discovery/common/buildflags/buildflags.h"
+#include "brave/components/web_discovery/buildflags/buildflags.h"
 #include "chrome/common/chrome_isolated_world_ids.h"
 #include "chrome/renderer/process_state.h"
 #include "components/dom_distiller/content/renderer/distillability_agent.h"

components/web_discovery/browser/BUILD.gn

@@ -19,6 +19,7 @@ component("browser") {
     "credential_signer.h",
     "hash_detection.cc",
     "hash_detection.h",
+    "hash_detection_matrix.h",
     "patterns.cc",
     "patterns.h",
     "payload_generator.cc",
@@ -58,20 +59,6 @@ component("browser") {
   ]
 }
 
-component("tab_helper") {
-  output_name = "web_discovery_tab_helper"
-  sources = [
-    "web_discovery_tab_helper.cc",
-    "web_discovery_tab_helper.h",
-  ]
-  deps = [
-    "//base",
-    "//brave/components/web_discovery/browser",
-    "//content/public/browser",
-    "//services/service_manager/public/cpp",
-  ]
-}
-
 source_set("unit_tests") {
   testonly = true
   sources = [
@@ -93,21 +80,3 @@ source_set("unit_tests") {
     "//testing/gtest",
   ]
 }
-
-source_set("browser_tests") {
-  testonly = true
-  sources = [ "content_scraper_browsertest.cc" ]
-  deps = [
-    ":browser",
-    "//base/test:test_support",
-    "//brave/components/constants",
-    "//brave/components/web_discovery/common",
-    "//brave/components/web_discovery/common:mojom",
-    "//chrome/test:test_support",
-    "//content/test:test_support",
-    "//net:test_support",
-    "//services/service_manager/public/cpp",
-    "//testing/gtest",
-  ]
-  defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]
-}

components/web_discovery/browser/content_scraper.cc

@@ -92,11 +92,9 @@ PageScrapeResult::PageScrapeResult(GURL url, std::string id)
     : url(url), id(id) {}
 PageScrapeResult::~PageScrapeResult() = default;
 
-ContentScraper::ContentScraper(const ServerConfigLoader* server_config_loader,
-                               RegexUtil* regex_util)
+ContentScraper::ContentScraper(const ServerConfigLoader* server_config_loader)
     : sequenced_task_runner_(base::ThreadPool::CreateSequencedTaskRunner({})),
-      server_config_loader_(server_config_loader),
-      regex_util_(regex_util) {}
+      server_config_loader_(server_config_loader) {}
 
 ContentScraper::~ContentScraper() = default;
 
@@ -330,7 +328,7 @@ std::optional<std::string> ContentScraper::ExecuteRefineFunctions(
                              function_args[2].GetInt());
       }
     } else if (func_name == kRefineMaskURLFuncId) {
-      result = MaskURL(*regex_util_, GURL(value));
+      result = MaskURL(GURL(value));
     } else if (func_name == kRefineParseURLFuncId) {
       if (function_args.size() >= 3 && function_args[1].is_string() &&
           function_args[2].is_string()) {

components/web_discovery/browser/content_scraper.h

@@ -15,7 +15,6 @@
 #include "base/values.h"
 #include "brave/components/web_discovery/browser/document_extractor/rust/src/lib.rs.h"
 #include "brave/components/web_discovery/browser/patterns.h"
-#include "brave/components/web_discovery/browser/regex_util.h"
 #include "brave/components/web_discovery/browser/server_config_loader.h"
 #include "brave/components/web_discovery/common/web_discovery.mojom.h"
 #include "url/gurl.h"
@@ -61,8 +60,7 @@ class ContentScraper {
   using PageScrapeResultCallback =
       base::OnceCallback<void(std::unique_ptr<PageScrapeResult>)>;
 
-  ContentScraper(const ServerConfigLoader* server_config_loader,
-                 RegexUtil* regex_util);
+  explicit ContentScraper(const ServerConfigLoader* server_config_loader);
   ~ContentScraper();
 
   ContentScraper(const ContentScraper&) = delete;
@@ -109,7 +107,6 @@ class ContentScraper {
   scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner_;
 
   raw_ptr<const ServerConfigLoader> server_config_loader_;
-  raw_ptr<RegexUtil> regex_util_;
 
   base::WeakPtrFactory<ContentScraper> weak_ptr_factory_{this};
 };