Discharge7443/brave-core
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[CodeHealth] SERP metrics (#34518)

Browse Source 
As part of general code health improvements, apply IWYU fixes, replace
SerpClassifier with free functions, improve TestHttpsServerBuilder,
update tests whose behavior did not match our expectations, and
deduplicate the search engine allowlist.
This commit is contained in:
Terry Mancey
2026-03-09 16:21:51 +00:00
committed by GitHub
parent 1a28501278
commit a86c7f70bd
11 changed files with 264 additions and 201 deletions
Download Patch File Download Diff File Expand all files Collapse all files
browser/serp_metrics/serp_metrics_all_profiles_aggregator_unittest.cc
-1
View File
@@ -15,7 +15,6 @@
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/testing_pref_service.h"
#include "content/public/test/browser_task_environment.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace serp_metrics {
browser/serp_metrics/serp_metrics_tab_helper.cc
+8 -12
View File
@@ -18,12 +18,10 @@
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "components/prefs/pref_service.h"
#include "components/search_engines/search_engine_type.h"
#include "components/search_engines/search_engine_utils.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents.h"
#include "ui/base/page_transition_types.h"
#include "url/gurl.h"
namespace serp_metrics {
@@ -67,21 +65,19 @@ SerpMetricsTabHelper::~SerpMetricsTabHelper() = default;
///////////////////////////////////////////////////////////////////////////////
bool SerpMetricsTabHelper::IsSameSearchQuery(const GURL& url) const {
bool SerpMetricsTabHelper::IsSameSerpAsLastRecorded(const GURL& url) const {
return last_recorded_serp_url_ &&
serp_classifier_.IsSameSearchQuery(url, *last_recorded_serp_url_);
IsSameSearchQuery(url, *last_recorded_serp_url_);
}
void SerpMetricsTabHelper::MaybeClassifyAndRecordSearchEngineForUrl(
const GURL& url) {
if (IsSameSearchQuery(url)) {
// The navigation repeats the same search query as the last recorded SERP,
// so do not double-count it.
if (IsSameSerpAsLastRecorded(url)) {
return;
}
std::optional<SearchEngineType> search_engine_type =
serp_classifier_.MaybeClassify(url);
MaybeClassifySearchEngine(url);
if (!search_engine_type) {
return;
}
@@ -134,10 +130,10 @@ void SerpMetricsTabHelper::DidFinishNavigation(
const GURL& url = navigation_handle->GetURL();
if (!is_new_navigation || !IsSameSearchQuery(url)) {
// Any navigation that doesn't match the previous search engine results page
// should reset it along with any user initiated navigation (omnibox,
// bookmarks, etc...) whether it matches or not.
if (!is_new_navigation || !IsSameSerpAsLastRecorded(url)) {
// If this isn't a new navigation or it doesn't go to the same SERP as the
// last recorded one, clear the last recorded SERP URL so the next visit to
// that SERP can be recorded again.
last_recorded_serp_url_.reset();
}
browser/serp_metrics/serp_metrics_tab_helper.h
+1 -4
View File
@@ -9,7 +9,6 @@
#include <optional>
#include "base/memory/raw_ptr.h"
#include "brave/components/serp_metrics/serp_classifier.h"
#include "components/search_engines/search_engine_type.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h"
@@ -40,7 +39,7 @@ class SerpMetricsTabHelper final
private:
friend class content::WebContentsUserData<SerpMetricsTabHelper>;
bool IsSameSearchQuery(const GURL& url) const;
bool IsSameSerpAsLastRecorded(const GURL& url) const;
void MaybeClassifyAndRecordSearchEngineForUrl(const GURL& url);
void RecordSearchEngine(SearchEngineType search_engine_type);
@@ -49,8 +48,6 @@ class SerpMetricsTabHelper final
void DidFinishNavigation(
content::NavigationHandle* navigation_handle) override;
SerpClassifier serp_classifier_;
raw_ptr<SerpMetrics> serp_metrics_ = nullptr; // Not owned.
std::optional<GURL> last_recorded_serp_url_;
browser/serp_metrics/serp_metrics_tab_helper_browsertest.cc
+100 -72
View File
@@ -11,6 +11,8 @@
#include <vector>
#include "absl/strings/str_format.h"
#include "base/check.h"
#include "base/containers/flat_map.h"
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_future.h"
@@ -42,7 +44,6 @@
#include "net/http/http_status_code.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
#if !BUILDFLAG(IS_ANDROID)
#include "chrome/browser/profiles/keep_alive/profile_keep_alive_types.h"
@@ -58,6 +59,14 @@ namespace serp_metrics {
namespace {
constexpr char kHtmlWithAnchorLinkContent[] = R"HTML(
<html>
<body>
<a id='anchor_link' href='%s'>Link</a>
</body>
</html>
)HTML";
// Builder class to help construct an HTTPS `EmbeddedTestServer` with various
// configurations for testing.
class TestHttpsServerBuilder {
@@ -75,8 +84,15 @@ class TestHttpsServerBuilder {
return *this;
}
TestHttpsServerBuilder& WithAnchorLink(GURL anchor_link) {
anchor_link_ = std::move(anchor_link);
TestHttpsServerBuilder& WithContentOverrideForPath(
const std::string& path,
const std::string& content) {
CHECK(!path.empty());
CHECK(!content.empty());
CHECK(override_http_response_content_.find(path) ==
override_http_response_content_.cend())
<< "Duplicate content override for path: " << path;
override_http_response_content_[path] = content;
return *this;
}
@@ -96,7 +112,8 @@ class TestHttpsServerBuilder {
}
https_server->RegisterRequestHandler(base::BindLambdaForTesting(
[http_status_code = http_status_code_, anchor_link = anchor_link_,
[http_status_code = http_status_code_,
override_http_response_content = override_http_response_content_,
source_path = source_path_, destination_path = destination_path_](
const net::test_server::HttpRequest& http_request)
-> std::unique_ptr<net::test_server::HttpResponse> {
@@ -112,20 +129,20 @@ class TestHttpsServerBuilder {
return http_response;
}
// Serve a basic HTML page with the specified status code and optional
// anchor link.
// Serve a HTML page with the specified status code.
http_response->set_code(http_status_code);
http_response->set_content_type("text/html");
std::string_view http_reason_phrase =
net::GetHttpReasonPhrase(http_status_code);
if (anchor_link) {
http_response->set_content(
absl::StrFormat("<html><body>%s<a id='anchor_link' "
"href='%s'>Link</a></body></html>",
http_reason_phrase, anchor_link->spec()));
const auto iter =
override_http_response_content.find(http_request.GetURL().path());
if (iter != override_http_response_content.cend()) {
// Override the default response content.
http_response->set_content(iter->second);
} else {
http_response->set_content(absl::StrFormat(
"<html><body>%s</body></html>", http_reason_phrase));
// Default response content is a simple HTML page showing the status
// code reason phrase.
http_response->set_content(
absl::StrFormat("<html><body>%s</body></html>",
net::GetHttpReasonPhrase(http_status_code)));
}
return http_response;
@@ -141,9 +158,22 @@ class TestHttpsServerBuilder {
std::optional<std::string> destination_path_;
net::HttpStatusCode http_status_code_ = net::HTTP_OK;
std::optional<GURL> anchor_link_;
base::flat_map</*path*/ std::string, /*http_response_content*/ std::string>
override_http_response_content_;
};
#if !BUILDFLAG(IS_ANDROID)
Browser* CreateProfileAndOpenBrowser() {
base::FilePath profile_path =
g_browser_process->profile_manager()->GenerateNextProfileDirectoryPath();
base::test::TestFuture<Browser*> browser_test_future;
profiles::SwitchToProfile(profile_path, /*always_create=*/false,
browser_test_future.GetCallback());
EXPECT_TRUE(browser_test_future.Wait());
return browser_test_future.Take();
}
#endif // !BUILDFLAG(IS_ANDROID)
} // namespace
// TODO(https://github.com/brave/brave-browser/issues/52599): Migrate SERP
@@ -171,7 +201,7 @@ class SerpMetricsTabHelperTest : public PlatformBrowserTest {
ASSERT_TRUE(https_server_);
ASSERT_TRUE(https_server_->Start());
// Wait for TemplateURLService to finish loading so that SerpClassifier can
// Wait for TemplateURLService to finish loading so that SERP classifier can
// classify search URLs. Without this, navigations that complete before the
// service is loaded will not be classified.
search_test_utils::WaitForTemplateURLServiceToLoad(
@@ -185,17 +215,19 @@ class SerpMetricsTabHelperTest : public PlatformBrowserTest {
}
void SimulateClickingAnchorLink() const {
content::TestNavigationObserver observer(GetWebContents());
content::TestNavigationObserver navigation_observer(GetWebContents());
ASSERT_TRUE(content::ExecJs(
GetWebContents(), "document.getElementById('anchor_link').click();"));
observer.Wait();
navigation_observer.Wait();
}
void Reload() const {
content::TestNavigationObserver observer(GetWebContents());
ASSERT_TRUE(GetWebContents()->GetController().GetLastCommittedEntry());
content::TestNavigationManager navigation_manager(
GetWebContents(), GetWebContents()->GetLastCommittedURL());
GetWebContents()->GetController().Reload(content::ReloadType::NORMAL,
/*check_for_repost=*/false);
observer.Wait();
ASSERT_TRUE(navigation_manager.WaitForNavigationFinished());
ASSERT_TRUE(content::WaitForLoadStop(GetWebContents()));
}
@@ -207,22 +239,6 @@ class SerpMetricsTabHelperTest : public PlatformBrowserTest {
return profile_misc_metrics_service->GetSerpMetrics();
}
ProfileManager* profile_manager() {
return g_browser_process->profile_manager();
}
#if !BUILDFLAG(IS_ANDROID)
Browser* CreateProfileAndOpenBrowser() {
base::FilePath profile_path =
profile_manager()->GenerateNextProfileDirectoryPath();
base::test::TestFuture<Browser*> browser_future;
profiles::SwitchToProfile(profile_path, /*always_create=*/false,
browser_future.GetCallback());
EXPECT_TRUE(browser_future.Wait());
return browser_future.Take();
}
#endif // !BUILDFLAG(IS_ANDROID)
base::test::ScopedFeatureList scoped_feature_list_;
content::ContentMockCertVerifier mock_cert_verifier_;
@@ -278,12 +294,16 @@ IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest,
}
IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest,
RecordSearchWhenNavigatingViaLinkClick) {
auto https_server =
RecordWhenNavigatingViaLinkClick) {
const auto https_server =
TestHttpsServerBuilder()
.WithCertHostnames({"search.brave.com", "plugh.xyzzy.com"})
.WithAnchorLink(
https_server_->GetURL("search.brave.com", "/search?q=test"))
.WithContentOverrideForPath(
"/thud",
absl::StrFormat(
kHtmlWithAnchorLinkContent,
https_server_->GetURL("search.brave.com", "/search?q=test")
.spec()))
.Build();
ASSERT_TRUE(https_server);
ASSERT_TRUE(https_server->Start());
@@ -301,10 +321,10 @@ IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest,
}
IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest, RecordForHttp4xxResponse) {
auto https_server = TestHttpsServerBuilder()
.WithStatusCode(net::HTTP_NOT_FOUND)
.WithCertHostnames({"search.brave.com"})
.Build();
const auto https_server = TestHttpsServerBuilder()
.WithStatusCode(net::HTTP_NOT_FOUND)
.WithCertHostnames({"search.brave.com"})
.Build();
ASSERT_TRUE(https_server);
ASSERT_TRUE(https_server->Start());
@@ -317,10 +337,10 @@ IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest, RecordForHttp4xxResponse) {
}
IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest, RecordForHttp5xxResponse) {
auto https_server = TestHttpsServerBuilder()
.WithStatusCode(net::HTTP_INTERNAL_SERVER_ERROR)
.WithCertHostnames({"www.google.com"})
.Build();
const auto https_server = TestHttpsServerBuilder()
.WithStatusCode(net::HTTP_INTERNAL_SERVER_ERROR)
.WithCertHostnames({"www.google.com"})
.Build();
ASSERT_TRUE(https_server);
ASSERT_TRUE(https_server->Start());
@@ -360,7 +380,7 @@ IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest,
IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest,
DoNotRecordRedirectNavigationForSameSearch) {
auto https_server =
const auto https_server =
TestHttpsServerBuilder()
.WithCertHostnames({"search.brave.com"})
.WithRedirect(/*source_path=*/"/search?q=test",
@@ -380,12 +400,13 @@ IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest,
}
IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest,
RecordSearchAfterRedirectNavigation) {
auto https_server = TestHttpsServerBuilder()
.WithCertHostnames({"search.brave.com"})
.WithRedirect(/*source_path=*/"/a/redirect",
/*destination_path=*/"/search?q=test")
.Build();
RecordAfterRedirectNavigation) {
const auto https_server =
TestHttpsServerBuilder()
.WithCertHostnames({"search.brave.com"})
.WithRedirect(/*source_path=*/"/a/redirect",
/*destination_path=*/"/search?q=test")
.Build();
ASSERT_TRUE(https_server);
ASSERT_TRUE(https_server->Start());
@@ -502,11 +523,15 @@ IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest,
IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest,
RecordSameSearchAfterReloadNavigationAndLinkClick) {
auto https_server =
const auto https_server =
TestHttpsServerBuilder()
.WithCertHostnames({"search.brave.com", "plugh.xyzzy.com"})
.WithAnchorLink(
https_server_->GetURL("search.brave.com", "/search?q=test"))
.WithContentOverrideForPath(
"/thud",
absl::StrFormat(
kHtmlWithAnchorLinkContent,
https_server_->GetURL("search.brave.com", "/search?q=test")
.spec()))
.Build();
ASSERT_TRUE(https_server);
ASSERT_TRUE(https_server->Start());
@@ -524,9 +549,10 @@ IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest,
Reload();
ASSERT_TRUE(content::HistoryGoBack(GetWebContents()));
SimulateClickingAnchorLink();
EXPECT_EQ(2U,
EXPECT_EQ(3U,
GetSerpMetrics()->GetSearchCountForTesting(SerpMetricType::kBrave));
}
@@ -643,14 +669,17 @@ IN_PROC_BROWSER_TEST_F(
2U, GetSerpMetrics()->GetSearchCountForTesting(SerpMetricType::kGoogle));
}
IN_PROC_BROWSER_TEST_F(
SerpMetricsTabHelperTest,
DoNotRecordSameSearchAfterBackForwardNavigationAndLinkClick) {
auto https_server =
IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest,
RecordSameSearchAfterBackForwardNavigationAndLinkClick) {
const auto https_server =
TestHttpsServerBuilder()
.WithCertHostnames({"search.brave.com", "plugh.xyzzy.com"})
.WithAnchorLink(
https_server_->GetURL("search.brave.com", "/search?q=test"))
.WithContentOverrideForPath(
"/thud",
absl::StrFormat(
kHtmlWithAnchorLinkContent,
https_server_->GetURL("search.brave.com", "/search?q=test")
.spec()))
.Build();
ASSERT_TRUE(https_server);
ASSERT_TRUE(https_server->Start());
@@ -669,9 +698,10 @@ IN_PROC_BROWSER_TEST_F(
ASSERT_TRUE(content::HistoryGoBack(GetWebContents()));
ASSERT_TRUE(content::HistoryGoForward(GetWebContents()));
ASSERT_TRUE(content::HistoryGoBack(GetWebContents()));
SimulateClickingAnchorLink();
EXPECT_EQ(2U,
EXPECT_EQ(3U,
GetSerpMetrics()->GetSearchCountForTesting(SerpMetricType::kBrave));
}
@@ -702,11 +732,11 @@ IN_PROC_BROWSER_TEST_F(
}
IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest, RecordWithoutUserGesture) {
content::TestNavigationObserver observer(GetWebContents());
content::TestNavigationObserver navigation_observer(GetWebContents());
ASSERT_TRUE(NavigateToURLFromRendererWithoutUserGesture(
GetWebContents(),
https_server_->GetURL("www.google.com", "/search?q=test")));
observer.Wait();
navigation_observer.Wait();
EXPECT_EQ(
1U, GetSerpMetrics()->GetSearchCountForTesting(SerpMetricType::kGoogle));
@@ -762,7 +792,6 @@ IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest, RecordIfTabWasRestored) {
session_restore_test_helper.Wait();
}
SetBrowser(browser_created_observer.Wait());
ASSERT_TRUE(content::WaitForLoadStop(GetWebContents()));
EXPECT_EQ(
@@ -792,7 +821,7 @@ IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest, RecordForMultipleProfiles) {
SerpMetricsAllProfilesAggregator all_profiles_aggregator(
g_browser_process->local_state(),
profile_manager()->GetProfileAttributesStorage());
g_browser_process->profile_manager()->GetProfileAttributesStorage());
EXPECT_EQ(2U, all_profiles_aggregator.GetSearchCountForTesting(
SerpMetricType::kBrave));
EXPECT_EQ(1U, all_profiles_aggregator.GetSearchCountForTesting(
@@ -800,7 +829,6 @@ IN_PROC_BROWSER_TEST_F(SerpMetricsTabHelperTest, RecordForMultipleProfiles) {
EXPECT_EQ(1U, all_profiles_aggregator.GetSearchCountForTesting(
SerpMetricType::kOther));
}
#endif // !BUILDFLAG(IS_ANDROID)
} // namespace serp_metrics
components/serp_metrics/BUILD.gn
+3
View File
@@ -6,12 +6,14 @@
static_library("serp_metrics") {
public = [
"serp_classifier.h",
"serp_classifier_utils.h",
"serp_metric_type.h",
"serp_metrics.h",
]
sources = [
"serp_classifier.cc",
"serp_classifier_utils.cc",
"serp_metrics.cc",
]
@@ -42,6 +44,7 @@ source_set("unit_tests") {
sources = [
"serp_classifier_unittest.cc",
"serp_classifier_utils_unittest.cc",
"serp_metrics_unittest.cc",
]
components/serp_metrics/serp_classifier.cc
+43 -44
View File
@@ -5,9 +5,11 @@
#include "brave/components/serp_metrics/serp_classifier.h"
#include "base/containers/fixed_flat_set.h"
#include <memory>
#include <string_view>
#include "brave/components/search_engines/brave_prepopulated_engines.h"
#include "components/search_engines/search_engine_type.h"
#include "brave/components/serp_metrics/serp_classifier_utils.h"
#include "components/search_engines/search_terms_data.h"
#include "components/search_engines/template_url.h"
#include "components/search_engines/template_url_data_util.h"
@@ -18,19 +20,15 @@ namespace serp_metrics {
namespace {
constexpr auto kAllowedPrepopulatedEngines =
base::MakeFixedFlatSet<SearchEngineType>(
base::sorted_unique,
{SEARCH_ENGINE_BING, SEARCH_ENGINE_GOOGLE, SEARCH_ENGINE_YAHOO,
SEARCH_ENGINE_DUCKDUCKGO, SEARCH_ENGINE_QWANT, SEARCH_ENGINE_ECOSIA,
SEARCH_ENGINE_BRAVE, SEARCH_ENGINE_STARTPAGE});
constexpr std::string_view kStartpageUrlHost = "www.startpage.com";
constexpr std::string_view kStartpageUrlPath = "/sp/search";
// Returns a `TemplateURL` if `url` matches the search engine results page for
// `prepopulated_engine`.
std::unique_ptr<TemplateURL> MaybeGetTemplateURLForPrepopulatedEngine(
const TemplateURLPrepopulateData::PrepopulatedEngine& prepopulated_engine,
const GURL& url) {
if (!kAllowedPrepopulatedEngines.contains(prepopulated_engine.type)) {
if (!IsAllowedSearchEngine(prepopulated_engine.type)) {
return nullptr;
}
@@ -40,7 +38,7 @@ std::unique_ptr<TemplateURL> MaybeGetTemplateURLForPrepopulatedEngine(
if (!template_url->IsSearchURL(url, SearchTermsData())) {
if (prepopulated_engine.type == SEARCH_ENGINE_STARTPAGE &&
url.host() == "www.startpage.com" && url.path() == "/sp/search") {
url.host() == kStartpageUrlHost && url.path() == kStartpageUrlPath) {
// Startpage uses a path-based SERP URL. Chromium still checks the legacy
// query-based format and does not support the new one. Even if we update
// the search URL, `TemplateURL::IsSearchURL` still fails because it
@@ -54,31 +52,32 @@ std::unique_ptr<TemplateURL> MaybeGetTemplateURLForPrepopulatedEngine(
return template_url;
}
} // namespace
bool SerpClassifier::IsSameSearchQuery(const GURL& lhs, const GURL& rhs) const {
if (lhs.host() == "www.startpage.com") {
// For Startpage, we cannot determine whether two URLs represent the same
// search results page, so these pages are always classified.
return false;
// Returns a `TemplateURL` if `url` matches the search engine results page for
// any prepopulated engine in the allow list.
std::unique_ptr<TemplateURL> MaybeGetTemplateUrl(const GURL& url) {
for (const auto* prepopulated_engine :
TemplateURLPrepopulateData::GetAllPrepopulatedEngines()) {
if (auto search_engine = MaybeGetTemplateURLForPrepopulatedEngine(
*prepopulated_engine, url)) {
return search_engine;
}
}
return NormalizeUrl(lhs) == NormalizeUrl(rhs);
}
std::optional<SearchEngineType> SerpClassifier::MaybeClassify(const GURL& url) {
const GURL normalized_url = NormalizeUrl(url);
if (const auto template_url = MaybeGetTemplateUrl(normalized_url)) {
return template_url->GetEngineType(SearchTermsData());
for (const auto& [_, prepopulated_engine] :
TemplateURLPrepopulateData::kBraveEngines) {
if (auto search_engine = MaybeGetTemplateURLForPrepopulatedEngine(
*prepopulated_engine, url)) {
return search_engine;
}
}
return std::nullopt;
return nullptr;
}
///////////////////////////////////////////////////////////////////////////////
GURL SerpClassifier::NormalizeUrl(const GURL& url) const {
// Normalizes a SERP URL so equivalent search queries produce the same URL for
// comparison. Strips ports, removes non-search parameters, and canonicalizes
// the search terms.
GURL NormalizeUrl(const GURL& url) {
if (!url.is_valid()) {
return url;
}
@@ -102,25 +101,25 @@ GURL SerpClassifier::NormalizeUrl(const GURL& url) const {
return normalized_url;
}
std::unique_ptr<TemplateURL> SerpClassifier::MaybeGetTemplateUrl(
const GURL& url) const {
for (const auto* prepopulated_engine :
TemplateURLPrepopulateData::GetAllPrepopulatedEngines()) {
if (auto search_engine = MaybeGetTemplateURLForPrepopulatedEngine(
*prepopulated_engine, url)) {
return search_engine;
}
} // namespace
bool IsSameSearchQuery(const GURL& lhs, const GURL& rhs) {
if (lhs.host() == kStartpageUrlHost) {
// For Startpage, we cannot determine whether two URLs represent the same
// search results page, so these pages are always classified.
return false;
}
for (const auto& [_, prepopulated_engine] :
TemplateURLPrepopulateData::kBraveEngines) {
if (auto search_engine = MaybeGetTemplateURLForPrepopulatedEngine(
*prepopulated_engine, url)) {
return search_engine;
}
return NormalizeUrl(lhs) == NormalizeUrl(rhs);
}
std::optional<SearchEngineType> MaybeClassifySearchEngine(const GURL& url) {
const GURL normalized_url = NormalizeUrl(url);
if (const auto template_url = MaybeGetTemplateUrl(normalized_url)) {
return template_url->GetEngineType(SearchTermsData());
}
return nullptr;
return std::nullopt;
}
} // namespace serp_metrics
components/serp_metrics/serp_classifier.h
+8 -27
View File
@@ -6,42 +6,23 @@
#ifndef BRAVE_COMPONENTS_SERP_METRICS_SERP_CLASSIFIER_H_
#define BRAVE_COMPONENTS_SERP_METRICS_SERP_CLASSIFIER_H_
#include <memory>
#include <optional>
#include "components/search_engines/search_engine_type.h"
class GURL;
class TemplateURL;
// SERP classifier determines whether a URL is a search engine results page and,
// if so, identifies the corresponding search engine.
namespace serp_metrics {
// SerpClassifier determines whether a URL is a search engine results page and,
// if so, identifies the corresponding search engine.
// Returns `true` if `lhs` and `rhs` represent the same search results page.
bool IsSameSearchQuery(const GURL& lhs, const GURL& rhs);
class SerpClassifier final {
public:
SerpClassifier() = default;
~SerpClassifier() = default;
SerpClassifier(const SerpClassifier&) = delete;
SerpClassifier& operator=(const SerpClassifier&) = delete;
// Returns `true` if `lhs` and `rhs` represent the same search results page.
bool IsSameSearchQuery(const GURL& lhs, const GURL& rhs) const;
// Returns the corresponding search engine type if `url` is a SERP. Returns
// `std::nullopt` if `url` is not a SERP.
std::optional<SearchEngineType> MaybeClassify(const GURL& url);
private:
// Normalizes a URL so equivalent search results pages compare equal.
GURL NormalizeUrl(const GURL& url) const;
// Returns a `TemplateURL` if `url` matches the search engine results page for
// any prepopulated engine in the allow list.
std::unique_ptr<TemplateURL> MaybeGetTemplateUrl(const GURL& url) const;
};
// Returns the corresponding search engine type if `url` is a SERP. Returns
// `std::nullopt` if `url` is not a SERP.
std::optional<SearchEngineType> MaybeClassifySearchEngine(const GURL& url);
} // namespace serp_metrics
components/serp_metrics/serp_classifier_unittest.cc
+27 -41
View File
@@ -7,8 +7,8 @@
#include <optional>
#include "base/containers/fixed_flat_set.h"
#include "brave/components/search_engines/brave_prepopulated_engines.h"
#include "brave/components/serp_metrics/serp_classifier_utils.h"
#include "components/search_engines/search_engine_type.h"
#include "components/search_engines/search_terms_data.h"
#include "components/search_engines/template_url.h"
@@ -21,56 +21,44 @@ namespace serp_metrics {
namespace {
constexpr auto kAllowedPrepopulatedEngines =
base::MakeFixedFlatSet<SearchEngineType>(
base::sorted_unique,
{SEARCH_ENGINE_BING, SEARCH_ENGINE_GOOGLE, SEARCH_ENGINE_YAHOO,
SEARCH_ENGINE_DUCKDUCKGO, SEARCH_ENGINE_QWANT, SEARCH_ENGINE_ECOSIA,
SEARCH_ENGINE_BRAVE, SEARCH_ENGINE_STARTPAGE});
void VerifySerpClassifierExpectation(
const TemplateURLPrepopulateData::PrepopulatedEngine& prepopulated_engine) {
const auto template_url_data =
TemplateURLDataFromPrepopulatedEngine(prepopulated_engine);
TemplateURL template_url(*template_url_data);
const TemplateURL template_url(*template_url_data);
GURL url = template_url.GenerateSearchURL(SearchTermsData(), u"test");
const GURL url = template_url.GenerateSearchURL(SearchTermsData(), u"test");
ASSERT_TRUE(url.is_valid());
SerpClassifier classifier;
if (std::optional<SearchEngineType> search_engine_type =
classifier.MaybeClassify(url)) {
EXPECT_TRUE(kAllowedPrepopulatedEngines.contains(*search_engine_type));
MaybeClassifySearchEngine(url)) {
EXPECT_TRUE(IsAllowedSearchEngine(*search_engine_type));
}
}
} // namespace
TEST(SerpClassifierTest, IsSameSearchQuery) {
SerpClassifier classifier;
EXPECT_TRUE(classifier.IsSameSearchQuery(
GURL(R"(https://www.qwant.com/?q=foobar)"),
GURL(R"(https://www.qwant.com/?q=foobar&t=web)")));
EXPECT_TRUE(IsSameSearchQuery(
GURL(R"(https://search.brave.com/search?q=foobar)"),
GURL(R"(https://search.brave.com/search?q=foobar&t=web)")));
}
TEST(SerpClassifierTest, IsSameSearchQueryWithDifferentParamOrder) {
SerpClassifier classifier;
EXPECT_TRUE(classifier.IsSameSearchQuery(
GURL(R"(https://www.qwant.com/?q=foobar)"),
GURL(R"(https://www.qwant.com/?t=web&q=foobar)")));
EXPECT_TRUE(IsSameSearchQuery(
GURL(R"(https://search.brave.com/search?q=foobar)"),
GURL(R"(https://search.brave.com/search?t=web&q=foobar)")));
}
TEST(SerpClassifierTest, IsNotSameSearchQuery) {
SerpClassifier classifier;
EXPECT_FALSE(classifier.IsSameSearchQuery(
GURL(R"(https://www.qwant.com/?q=foo&t=web)"),
GURL(R"(https://www.qwant.com/?q=bar&t=web")")));
EXPECT_FALSE(IsSameSearchQuery(
GURL(R"(https://search.brave.com/search?q=foo&t=web)"),
GURL(R"(https://search.brave.com/search?q=bar&t=web")")));
}
TEST(SerpClassifierTest, IsNotSameSearchQueryWithInvalidUrl) {
SerpClassifier classifier;
EXPECT_FALSE(classifier.IsSameSearchQuery(
GURL(R"(https://www.qwant.com/?q=foobar)"), GURL("foobar")));
EXPECT_FALSE(IsSameSearchQuery(
GURL(R"(https://search.brave.com/search?q=foobar)"), GURL("invalid")));
}
TEST(SerpClassifierTest, OnlyClassifyAllowedSearchEngines) {
@@ -87,26 +75,24 @@ TEST(SerpClassifierTest, OnlyClassifyAllowedSearchEngines) {
TEST(SerpClassifierTest, ClassifyStartpageSearchEngine) {
// Startpage uses a path-based SERP URL that Chromium's query-based detection
// does not support for real-world navigations to the results page.
SerpClassifier classifier;
EXPECT_TRUE(
classifier.MaybeClassify(GURL(R"(https://www.startpage.com/sp/search)")));
// does not support.
EXPECT_TRUE(MaybeClassifySearchEngine(
GURL(R"(https://www.startpage.com/sp/search)")));
}
TEST(SerpClassifierTest, DoNotClassifyNonSearchEngine) {
SerpClassifier classifier;
EXPECT_FALSE(classifier.MaybeClassify(
EXPECT_FALSE(MaybeClassifySearchEngine(
GURL(R"(https://www.perplexity.ai/search/new/foo)")));
EXPECT_FALSE(classifier.MaybeClassify(GURL(R"(https://brave.com/)")));
EXPECT_FALSE(classifier.MaybeClassify(GURL(R"(https://bar.com/baz)")));
EXPECT_FALSE(classifier.MaybeClassify(GURL(R"(https://qux.quux.com/corge)")));
EXPECT_FALSE(MaybeClassifySearchEngine(GURL(R"(https://brave.com/)")));
EXPECT_FALSE(MaybeClassifySearchEngine(GURL(R"(https://bar.com/baz)")));
EXPECT_FALSE(
classifier.MaybeClassify(GURL(R"(https://startpage.com/grault)")));
MaybeClassifySearchEngine(GURL(R"(https://qux.quux.com/corge)")));
EXPECT_FALSE(
classifier.MaybeClassify(GURL(R"(https://uk.search.yahoo.com/garply)")));
MaybeClassifySearchEngine(GURL(R"(https://startpage.com/grault)")));
EXPECT_FALSE(
classifier.MaybeClassify(GURL(R"(https://search.yahoo.com/waldo)")));
MaybeClassifySearchEngine(GURL(R"(https://uk.search.yahoo.com/garply)")));
EXPECT_FALSE(
MaybeClassifySearchEngine(GURL(R"(https://search.yahoo.com/waldo)")));
}
} // namespace serp_metrics
components/serp_metrics/serp_classifier_utils.cc
+26
View File
@@ -0,0 +1,26 @@
/* Copyright (c) 2026 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/serp_metrics/serp_classifier_utils.h"
#include "base/containers/fixed_flat_set.h"
namespace serp_metrics {
namespace {
constexpr auto kAllowedSearchEngines = base::MakeFixedFlatSet<SearchEngineType>(
base::sorted_unique,
{SEARCH_ENGINE_BING, SEARCH_ENGINE_GOOGLE, SEARCH_ENGINE_YAHOO,
SEARCH_ENGINE_DUCKDUCKGO, SEARCH_ENGINE_QWANT, SEARCH_ENGINE_ECOSIA,
SEARCH_ENGINE_BRAVE, SEARCH_ENGINE_STARTPAGE});
} // namespace
bool IsAllowedSearchEngine(SearchEngineType type) {
return kAllowedSearchEngines.contains(type);
}
} // namespace serp_metrics
components/serp_metrics/serp_classifier_utils.h
+18
View File
@@ -0,0 +1,18 @@
/* Copyright (c) 2026 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/. */
#ifndef BRAVE_COMPONENTS_SERP_METRICS_SERP_CLASSIFIER_UTILS_H_
#define BRAVE_COMPONENTS_SERP_METRICS_SERP_CLASSIFIER_UTILS_H_
#include "components/search_engines/search_engine_type.h"
namespace serp_metrics {
// Returns `true` if the search engine type is allowed to be classified.
bool IsAllowedSearchEngine(SearchEngineType type);
} // namespace serp_metrics
#endif // BRAVE_COMPONENTS_SERP_METRICS_SERP_CLASSIFIER_UTILS_H_
components/serp_metrics/serp_classifier_utils_unittest.cc
+30
View File
@@ -0,0 +1,30 @@
/* Copyright (c) 2026 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/serp_metrics/serp_classifier_utils.h"
#include "base/containers/fixed_flat_set.h"
#include "components/search_engines/search_engine_type.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace serp_metrics {
TEST(SerpClassifierUtilsTest, IsAllowedSearchEngine) {
constexpr auto kAllowedSearchEngines =
base::MakeFixedFlatSet<SearchEngineType>(
base::sorted_unique,
{SEARCH_ENGINE_BING, SEARCH_ENGINE_GOOGLE, SEARCH_ENGINE_YAHOO,
SEARCH_ENGINE_DUCKDUCKGO, SEARCH_ENGINE_QWANT, SEARCH_ENGINE_ECOSIA,
SEARCH_ENGINE_BRAVE, SEARCH_ENGINE_STARTPAGE});
for (int i = 0; i <= SEARCH_ENGINE_MAX; ++i) {
const SearchEngineType search_engine_type =
static_cast<SearchEngineType>(i);
EXPECT_EQ(IsAllowedSearchEngine(search_engine_type),
kAllowedSearchEngines.contains(search_engine_type));
}
}
} // namespace serp_metrics