Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Brave search param changes and extra tests #8537

Merged
merged 5 commits into from Apr 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
68 changes: 35 additions & 33 deletions browser/brave_search/brave_search_browsertest.cc
Expand Up @@ -3,9 +3,12 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "base/path_service.h"
#include "base/task/post_task.h"
#include "base/test/thread_test_helper.h"
#include "brave/browser/brave_browser_process_impl.h"
#include "brave/common/brave_paths.h"
#include "brave/common/pref_names.h"
#include "brave/components/brave_search/browser/brave_search_host.h"
#include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/ui/browser.h"
Expand All @@ -23,25 +26,23 @@ using extensions::ExtensionBrowserTest;

namespace {

const char kEmbeddedTestServerDirectory[] = "brave-search";
const char kAllowedDomain[] = "search.brave.com";
const char kAllowedDomainDev[] = "search-dev.brave.com";
const char kNotAllowedDomain[] = "brave.com";
const char kBackupSearchContent[] = "<html><body>results</body></html>";

std::string GetChromeFetchBackupResultsAvailScript() {
return base::StringPrintf(R"(function waitForFunction() {
if (window.chrome.fetchBackupResults != undefined) {
console.log('calling fetch backup results')
window.chrome.fetchBackupResults('test', 'en', 'us', 'US')
.then((result) => {
const expected = result === '%s'
window.domAutomationController.send(expected);
})
} else {
console.log('still waiting for the function');
setTimeout(waitForFunction, 200);
}
} waitForFunction();)",
setTimeout(waitForFunction, 200);
}
navigator.serviceWorker.addEventListener('message', msg => {
if (msg.data && msg.data.result === 'INJECTED') {
window.domAutomationController.send(msg.data.response === '%s');
} else if (msg.data && msg.data.result === 'FAILED') {
window.domAutomationController.send(false);
}});
waitForFunction();)",
kBackupSearchContent);
}

Expand All @@ -61,6 +62,12 @@ class BraveSearchTest : public InProcessBrowserTest {
https_server_->RegisterRequestHandler(base::BindRepeating(
&BraveSearchTest::HandleRequest, base::Unretained(this)));

brave::RegisterPathProvider();
base::FilePath test_data_dir;
base::PathService::Get(brave::DIR_TEST_DATA, &test_data_dir);
test_data_dir = test_data_dir.AppendASCII(kEmbeddedTestServerDirectory);
https_server_->ServeFilesFromDirectory(test_data_dir);

ASSERT_TRUE(https_server_->Start());
GURL url = https_server()->GetURL("a.com", "/search");
brave_search::BraveSearchHost::SetBackupProviderForTest(url);
Expand All @@ -74,16 +81,17 @@ class BraveSearchTest : public InProcessBrowserTest {

std::unique_ptr<net::test_server::HttpResponse> HandleRequest(
bbondy marked this conversation as resolved.
Show resolved Hide resolved
const net::test_server::HttpRequest& request) {
if (request.GetURL().path_piece() == "/sw.js" ||
request.GetURL().path_piece() == "/bravesearch.html")
return nullptr;

GURL url = request.GetURL();
auto http_response =
std::make_unique<net::test_server::BasicHttpResponse>();
if (request.GetURL().path_piece() == "/simple.html") {
if (url.path() + "?" + url.query() ==
"/search?q=test&hl=en&gl=us&self=active") {
http_response->set_code(net::HTTP_OK);
http_response->set_content_type("text/html");
http_response->set_content("simple.html");
return http_response;
} else if (request.GetURL().path_piece() == "/search") {
http_response->set_code(net::HTTP_OK);
http_response->set_content_type("application/json");
http_response->set_content(kBackupSearchContent);
return http_response;
}
Expand All @@ -97,8 +105,8 @@ class BraveSearchTest : public InProcessBrowserTest {
std::unique_ptr<net::EmbeddedTestServer> https_server_;
};

IN_PROC_BROWSER_TEST_F(BraveSearchTest, DISABLED_CheckForAFunction) {
GURL url = https_server()->GetURL(kAllowedDomain, "/simple.html");
IN_PROC_BROWSER_TEST_F(BraveSearchTest, CheckForAFunction) {
GURL url = https_server()->GetURL(kAllowedDomain, "/bravesearch.html");
ui_test_utils::NavigateToURL(browser(), url);
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
Expand All @@ -109,8 +117,8 @@ IN_PROC_BROWSER_TEST_F(BraveSearchTest, DISABLED_CheckForAFunction) {
EXPECT_EQ(base::Value(true), result_first.value);
}

IN_PROC_BROWSER_TEST_F(BraveSearchTest, DISABLED_CheckForAFunctionDev) {
GURL url = https_server()->GetURL(kAllowedDomainDev, "/simple.html");
IN_PROC_BROWSER_TEST_F(BraveSearchTest, CheckForAFunctionDev) {
GURL url = https_server()->GetURL(kAllowedDomainDev, "/bravesearch.html");
ui_test_utils::NavigateToURL(browser(), url);
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
Expand All @@ -121,20 +129,14 @@ IN_PROC_BROWSER_TEST_F(BraveSearchTest, DISABLED_CheckForAFunctionDev) {
EXPECT_EQ(base::Value(true), result_first.value);
}

IN_PROC_BROWSER_TEST_F(BraveSearchTest, DISABLED_CheckForAnUndefinedFunction) {
GURL url = https_server()->GetURL(kNotAllowedDomain, "/simple.html");
IN_PROC_BROWSER_TEST_F(BraveSearchTest, CheckForAnUndefinedFunction) {
GURL url = https_server()->GetURL(kNotAllowedDomain, "/bravesearch.html");
ui_test_utils::NavigateToURL(browser(), url);
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
WaitForLoadStop(contents);

auto result_first = EvalJsWithManualReply(contents,
R"(function waitForFunction() {
if (window.chrome.fetchBackupResults != undefined) {
window.domAutomationController.send(false);
} else {
window.domAutomationController.send(true);
}
} setTimeout(waitForFunction, 1000);)");
EXPECT_EQ(base::Value(true), result_first.value);
auto result_first =
EvalJsWithManualReply(contents, GetChromeFetchBackupResultsAvailScript());
EXPECT_EQ(base::Value(false), result_first.value);
}
35 changes: 27 additions & 8 deletions components/brave_search/browser/brave_search_host.cc
Expand Up @@ -7,8 +7,8 @@

#include <utility>

#include "base/strings/stringprintf.h"
#include "net/base/load_flags.h"
#include "net/base/url_util.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/simple_url_loader.h"

Expand Down Expand Up @@ -52,21 +52,40 @@ BraveSearchHost::BraveSearchHost(

BraveSearchHost::~BraveSearchHost() {}

// [static]
GURL BraveSearchHost::GetBackupResultURL(const GURL& baseURL,
const std::string& query,
const std::string& lang,
const std::string& country,
const std::string& geo,
bool filter_explicit_results) {
GURL url = baseURL;
url = net::AppendQueryParameter(url, "q", query);
if (!lang.empty()) {
url = net::AppendQueryParameter(url, "hl", lang);
}
if (!country.empty()) {
url = net::AppendQueryParameter(url, "gl", country);
}
if (filter_explicit_results) {
url = net::AppendQueryParameter(url, "self", "active");
}
return url;
}

void BraveSearchHost::FetchBackupResults(const std::string& query,
const std::string& lang,
const std::string& country,
const std::string& geo,
bool filter_explicit_results,
FetchBackupResultsCallback callback) {
auto request = std::make_unique<network::ResourceRequest>();

if (backup_provider_for_test.is_empty()) {
std::string spec(
base::StringPrintf("https://www.google.com/search?q=%s&hl=%s&gl=%s",
query.c_str(), lang.c_str(), country.c_str()));
request->url = GURL(spec);
} else {
request->url = GURL("https://www.google.com/search");
if (!backup_provider_for_test.is_empty()) {
request->url = backup_provider_for_test;
}
request->url = GetBackupResultURL(request->url, query, lang, country, geo,
filter_explicit_results);
request->load_flags = net::LOAD_BYPASS_CACHE | net::LOAD_DISABLE_CACHE;
request->credentials_mode = network::mojom::CredentialsMode::kOmit;
request->load_flags |= net::LOAD_DO_NOT_SAVE_COOKIES;
Expand Down
8 changes: 8 additions & 0 deletions components/brave_search/browser/brave_search_host.h
Expand Up @@ -34,7 +34,15 @@ class BraveSearchHost final : public brave_search::mojom::BraveSearchFallback {
const std::string& lang,
const std::string& country,
const std::string& geo,
bool filter_explicit_results,
FetchBackupResultsCallback callback) override;

static GURL GetBackupResultURL(const GURL& baseURL,
const std::string& query,
const std::string& lang,
const std::string& country,
const std::string& geo,
bool filter_explicit_results);
static void SetBackupProviderForTest(const GURL&);

private:
Expand Down
48 changes: 48 additions & 0 deletions components/brave_search/browser/brave_search_host_unittest.cc
@@ -0,0 +1,48 @@
/* Copyright (c) 2021 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 http://mozilla.org/MPL/2.0/. */

#include "brave/components/brave_search/browser/brave_search_host.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"

namespace brave_search {

TEST(BraveSearchHost, GetBackupResultURL) {
GURL base_url("https://www.google.com/search/");
ASSERT_EQ(
BraveSearchHost::GetBackupResultURL(base_url, "test", "en", "ca", "32,32",
true),
GURL("https://www.google.com/search/?q=test&hl=en&gl=ca&self=active"));
}

TEST(BraveSearchHost, GetBackupResultURLNoLang) {
GURL base_url("https://www.google.com/search/");
ASSERT_EQ(BraveSearchHost::GetBackupResultURL(base_url, "test", "", "ca",
"32,32", true),
GURL("https://www.google.com/search/?q=test&gl=ca&self=active"));
}

TEST(BraveSearchHost, GetBackupResultURLNoCountry) {
GURL base_url("https://www.google.com/search/");
ASSERT_EQ(BraveSearchHost::GetBackupResultURL(base_url, "test", "en", "",
"32,32", true),
GURL("https://www.google.com/search/?q=test&hl=en&self=active"));
}

TEST(BraveSearchHost, GetBackupResultURLNoFilter) {
GURL base_url("https://www.google.com/search/");
ASSERT_EQ(BraveSearchHost::GetBackupResultURL(base_url, "test", "en", "ca",
"32,32", false),
GURL("https://www.google.com/search/?q=test&hl=en&gl=ca"));
}

TEST(BraveSearchHost, GetBackupResultURLMinimal) {
GURL base_url("https://www.google.com/search/");
ASSERT_EQ(
BraveSearchHost::GetBackupResultURL(base_url, "test", "", "", "", false),
GURL("https://www.google.com/search/?q=test"));
}

} // namespace brave_search
3 changes: 2 additions & 1 deletion components/brave_search/common/brave_search.mojom
Expand Up @@ -6,5 +6,6 @@ interface BraveSearchFallback {
FetchBackupResults(string query_string,
string lang,
string country,
string geo) => (string response);
string geo,
bool filter_explicit_results) => (string response);
};
4 changes: 2 additions & 2 deletions components/brave_search/renderer/BUILD.gn
Expand Up @@ -2,8 +2,8 @@ source_set("renderer") {
sources = [
"brave_search_js_handler.cc",
"brave_search_js_handler.h",
"brave_search_sw_holder.cc",
"brave_search_sw_holder.h",
"brave_search_service_worker_holder.cc",
"brave_search_service_worker_holder.h",
]

deps = [
Expand Down
5 changes: 3 additions & 2 deletions components/brave_search/renderer/brave_search_js_handler.cc
Expand Up @@ -96,7 +96,8 @@ v8::Local<v8::Promise> BraveSearchJSHandler::FetchBackupResults(
const std::string& query_string,
const std::string& lang,
const std::string& country,
const std::string& geo) {
const std::string& geo,
bool filter_explicit_results) {
if (!EnsureConnected())
return v8::Local<v8::Promise>();

Expand All @@ -107,7 +108,7 @@ v8::Local<v8::Promise> BraveSearchJSHandler::FetchBackupResults(
std::make_unique<v8::Global<v8::Promise::Resolver>>();
promise_resolver->Reset(isolate_, resolver.ToLocalChecked());
brave_search_fallback_->FetchBackupResults(
query_string, lang, country, geo,
query_string, lang, country, geo, filter_explicit_results,
base::BindOnce(&BraveSearchJSHandler::OnFetchBackupResults,
base::Unretained(this), std::move(promise_resolver)));

Expand Down
Expand Up @@ -46,7 +46,8 @@ class BraveSearchJSHandler {
v8::Local<v8::Promise> FetchBackupResults(const std::string& query_string,
const std::string& lang,
const std::string& country,
const std::string& geo);
const std::string& geo,
bool filter_explicit_results);
void OnFetchBackupResults(
std::unique_ptr<v8::Global<v8::Promise::Resolver>> promise_resolver,
const std::string& response);
Expand Down
Expand Up @@ -3,7 +3,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/components/brave_search/renderer/brave_search_sw_holder.h"
#include "brave/components/brave_search/renderer/brave_search_service_worker_holder.h"

#include <string>
#include <utility>
Expand All @@ -15,7 +15,7 @@
namespace {

static base::NoDestructor<std::vector<std::string>> g_vetted_hosts(
{"search.brave.com", "search-dev.brave.com"});
{"search.brave.com", "search-dev.brave.com", "search-dev-local.brave.com"});

bool IsAllowedHost(const GURL& url) {
std::string host = url.host();
Expand Down Expand Up @@ -46,16 +46,17 @@ JSHandlersVector::iterator FindContext(JSHandlersVector* contexts,
return std::find_if(contexts->begin(), contexts->end(), context_matches);
}

BraveSearchSWHolder::BraveSearchSWHolder() : broker_(nullptr) {}
BraveSearchServiceWorkerHolder::BraveSearchServiceWorkerHolder()
: broker_(nullptr) {}

BraveSearchSWHolder::~BraveSearchSWHolder() = default;
BraveSearchServiceWorkerHolder::~BraveSearchServiceWorkerHolder() = default;

void BraveSearchSWHolder::SetBrowserInterfaceBrokerProxy(
void BraveSearchServiceWorkerHolder::SetBrowserInterfaceBrokerProxy(
blink::ThreadSafeBrowserInterfaceBrokerProxy* broker) {
broker_ = broker;
}

void BraveSearchSWHolder::WillEvaluateServiceWorkerOnWorkerThread(
void BraveSearchServiceWorkerHolder::WillEvaluateServiceWorkerOnWorkerThread(
blink::WebServiceWorkerContextProxy* context_proxy,
v8::Local<v8::Context> v8_context,
int64_t service_worker_version_id,
Expand All @@ -80,11 +81,12 @@ void BraveSearchSWHolder::WillEvaluateServiceWorkerOnWorkerThread(
js_handlers->push_back(std::move(js_handler));
}

void BraveSearchSWHolder::WillDestroyServiceWorkerContextOnWorkerThread(
v8::Local<v8::Context> v8_context,
int64_t service_worker_version_id,
const GURL& service_worker_scope,
const GURL& script_url) {
void BraveSearchServiceWorkerHolder::
WillDestroyServiceWorkerContextOnWorkerThread(
v8::Local<v8::Context> v8_context,
int64_t service_worker_version_id,
const GURL& service_worker_scope,
const GURL& script_url) {
if (!service_worker_scope.is_valid() ||
!service_worker_scope.SchemeIsHTTPOrHTTPS() ||
!IsAllowedHost(service_worker_scope))
Expand All @@ -98,7 +100,7 @@ void BraveSearchSWHolder::WillDestroyServiceWorkerContextOnWorkerThread(
js_handlers->erase(context_it);
}

void BraveSearchSWHolder::WillStopCurrentWorkerThread() {
void BraveSearchServiceWorkerHolder::WillStopCurrentWorkerThread() {
content::WorkerThread::RemoveObserver(this);
JSHandlersVector* js_handlers = js_handlers_tls_.Get();
DCHECK(js_handlers);
Expand Down