Address hidden renderer feedback

browser/brave_search/backup_results_service_impl.cc

@@ -63,15 +63,17 @@ class BackupResultsWebContentsObserver
  public:
   void DidFinishNavigation(content::NavigationHandle* controller) override {
     const auto* response_headers = controller->GetResponseHeaders();
-    if (response_headers) {
-      backup_results_service_->HandleWebContentsDidFinishNavigation(
-          web_contents(), response_headers->response_code());
+    if (!response_headers || !backup_results_service_) {
+      return;
     }
+    backup_results_service_->HandleWebContentsDidFinishNavigation(
+        web_contents(), response_headers->response_code());
   }
 
   void DidFinishLoad(content::RenderFrameHost* render_frame_host,
                      const GURL& validated_url) override {
-    if (!validated_url.SchemeIs(url::kHttpsScheme)) {
+    if (!validated_url.SchemeIs(url::kHttpsScheme) ||
+        !backup_results_service_) {
       return;
     }
     backup_results_service_->HandleWebContentsDidFinishLoad(web_contents());
@@ -107,6 +109,7 @@ void BackupResultsServiceImpl::FetchBackupResults(
     std::optional<net::HttpRequestHeaders> headers,
     BackupResultsCallback callback) {
   if (!profile_) {
+    std::move(callback).Run(std::nullopt);
     return;
   }
   auto otr_profile_id =
@@ -318,15 +321,23 @@ void BackupResultsServiceImpl::CleanupAndDispatchResult(
 }
 
 void BackupResultsServiceImpl::OnProfileWillBeDestroyed(Profile* profile) {
-  profile_->RemoveObserver(this);
-  for (auto& request : pending_requests_) {
-    request.web_contents = nullptr;
-    auto* otr_profile = request.otr_profile.get();
-    request.otr_profile = nullptr;
-    profile_->DestroyOffTheRecordProfile(otr_profile);
+  Shutdown();
+}
+
+void BackupResultsServiceImpl::Shutdown() {
+  if (profile_) {
+    profile_->RemoveObserver(this);
+    for (auto& request : pending_requests_) {
+      request.web_contents = nullptr;
+      auto* otr_profile = request.otr_profile.get();
+      request.otr_profile = nullptr;
+      profile_->DestroyOffTheRecordProfile(otr_profile);
+    }
+    pending_requests_.clear();
+    profile_ = nullptr;
   }
-  pending_requests_.clear();
-  profile_ = nullptr;
+
+  weak_ptr_factory_.InvalidateWeakPtrs();
 }
 
 }  // namespace brave_search

browser/brave_search/backup_results_service_impl.h

@@ -56,6 +56,9 @@ class BackupResultsServiceImpl : public BackupResultsService,
   // ProfileObserver:
   void OnProfileWillBeDestroyed(Profile* profile) override;
 
+  // KeyedService:
+  void Shutdown() override;
+
  private:
   struct PendingRequest {
     PendingRequest(std::unique_ptr<content::WebContents> web_contents,

components/brave_search/browser/BUILD.gn

@@ -33,3 +33,15 @@ static_library("browser") {
     "//url",
   ]
 }
+
+source_set("unit_tests") {
+  testonly = true
+  sources = [ "backup_results_allowed_urls_unittest.cc" ]
+
+  deps = [
+    ":browser",
+    "//base/test:test_support",
+    "//testing/gtest:gtest",
+    "//url",
+  ]
+}

components/brave_search/browser/backup_results_allowed_urls.cc

@@ -5,11 +5,14 @@
 
 #include "brave/components/brave_search/browser/backup_results_allowed_urls.h"
 
+#include <string>
 #include <string_view>
+#include <vector>
 
 #include "base/containers/fixed_flat_set.h"
 #include "base/logging.h"
 #include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "url/url_constants.h"
 
 namespace brave_search {
@@ -59,6 +62,7 @@ bool IsBackupResultURLAllowed(const GURL& url) {
   if (!url.SchemeIs(url::kHttpsScheme)) {
     return false;
   }
+
   // Extract the hostname from the URL
   auto hostname = url.host_piece();
 
@@ -70,10 +74,16 @@ bool IsBackupResultURLAllowed(const GURL& url) {
     return false;
   }
 
-  std::string_view tld = host_parts.back();
-  std::string_view sld = host_parts[host_parts.size() - 2];
+  // Look for "google" in the parts
+  auto google_it = std::find(host_parts.begin(), host_parts.end(), kGoogleSLD);
+  if (google_it == host_parts.end()) {
+    return false;
+  }
+
+  std::string potential_tld = base::JoinString(
+      std::vector<std::string>(google_it + 1, host_parts.end()), ".");
 
-  return sld == kGoogleSLD && kAllowedGoogleTLDs.contains(tld);
+  return !potential_tld.empty() && kAllowedGoogleTLDs.contains(potential_tld);
 }
 
 }  // namespace brave_search

components/brave_search/browser/backup_results_allowed_urls_unittest.cc

@@ -0,0 +1,37 @@
+/* Copyright (c) 2025 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+#include "brave/components/brave_search/browser/backup_results_allowed_urls.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace brave_search {
+
+TEST(BackupResultsAllowedURLsTest, ValidGoogleDomains) {
+  EXPECT_TRUE(IsBackupResultURLAllowed(GURL("https://google.com")));
+  EXPECT_TRUE(IsBackupResultURLAllowed(GURL("https://google.co.uk")));
+  EXPECT_TRUE(IsBackupResultURLAllowed(GURL("https://google.com.au")));
+  EXPECT_TRUE(IsBackupResultURLAllowed(GURL("https://google.fr")));
+  EXPECT_TRUE(IsBackupResultURLAllowed(GURL("https://google.de")));
+  EXPECT_TRUE(IsBackupResultURLAllowed(GURL("https://www.google.de")));
+  EXPECT_TRUE(
+      IsBackupResultURLAllowed(GURL("https://www.google.co.uk/search")));
+}
+
+TEST(BackupResultsAllowedURLsTest, InvalidDomains) {
+  EXPECT_FALSE(
+      IsBackupResultURLAllowed(GURL("http://google.com")));  // Not HTTPS
+  EXPECT_FALSE(IsBackupResultURLAllowed(GURL("https://fake-google.com")));
+  EXPECT_FALSE(IsBackupResultURLAllowed(GURL("https://google.invalid")));
+  EXPECT_FALSE(
+      IsBackupResultURLAllowed(GURL("https://google")));  // Missing TLD
+  EXPECT_FALSE(IsBackupResultURLAllowed(GURL("https://googles.com")));
+  EXPECT_FALSE(IsBackupResultURLAllowed(GURL("https://googles.com/search")));
+  EXPECT_FALSE(IsBackupResultURLAllowed(GURL("about:blank")));
+  EXPECT_FALSE(IsBackupResultURLAllowed(GURL("https://brave.com")));
+}
+
+}  // namespace brave_search

components/brave_search/browser/backup_results_service.cc

@@ -5,11 +5,13 @@
 
 #include "brave/components/brave_search/browser/backup_results_service.h"
 
+#include <utility>
+
 namespace brave_search {
 
 BackupResultsService::BackupResults::BackupResults(int final_status_code,
                                                    std::string html)
-    : final_status_code(final_status_code), html(html) {}
+    : final_status_code(final_status_code), html(std::move(html)) {}
 
 BackupResultsService::BackupResults::~BackupResults() = default;
 

components/brave_search/browser/backup_results_service.h

@@ -22,14 +22,15 @@ namespace brave_search {
 
 // Fetches search results from a backup search provider,
 // for use in Brave Search fallback mixing (GMix) and Web Discovery Project.
+// This class does not interact with Brave Search.
 //
 // Each request will use an OTR profile for temporarily storing cookies, etc.
 //
 // There are three modes of operation for this service:
-// i)   If `features::kBackupResultsFullRender` is disabled, the initial search
+// i)   If `features::kBackupResultsFullRender` is disabled, the bootstrap
 //      page will be rendered, and the actual search engine results page will be
 //      fetched.
-// ii)  If `features::kBackupResultsFullRender` is enabled, the initial search
+// ii)  If `features::kBackupResultsFullRender` is enabled, the bootstrap
 //      page and the actual search engine results page will be rendered.
 // iii) If a cookie header value is provided in `FetchBackupResults`, the actual
 //      search engine result page will be directly fetched, with no rendering.

components/brave_search/browser/brave_search_fallback_host.cc

@@ -76,6 +76,11 @@ void BraveSearchFallbackHost::FetchBackupResults(
     headers.SetHeader(net::HttpRequestHeaders::kCookie, *cookie_header_value);
   }
 
+  if (!backup_results_service_) {
+    OnResultsAvailable(std::move(callback), std::nullopt);
+    return;
+  }
+
   backup_results_service_->FetchBackupResults(
       url, headers,
       base::BindOnce(&BraveSearchFallbackHost::OnResultsAvailable,

components/brave_search/browser/brave_search_fallback_host.h

@@ -48,7 +48,7 @@ class BraveSearchFallbackHost final
   void OnResultsAvailable(
       BraveSearchFallbackHost::FetchBackupResultsCallback callback,
       const std::optional<BackupResultsService::BackupResults> backup_results);
-  base::WeakPtr<brave_search::BackupResultsService> backup_results_service_;
+  base::WeakPtr<BackupResultsService> backup_results_service_;
   base::WeakPtrFactory<BraveSearchFallbackHost> weak_factory_;
 };
 

test/BUILD.gn

@@ -203,6 +203,7 @@ test("brave_unit_tests") {
     "//brave/components/brave_rewards/core/test",
     "//brave/components/brave_rewards/test:brave_rewards_unit_tests",
     "//brave/components/brave_search/browser",
+    "//brave/components/brave_search/browser:unit_tests",
     "//brave/components/brave_search/common",
     "//brave/components/brave_search_conversion",
     "//brave/components/brave_search_conversion:unit_tests",