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

Moves backup results function to service workers #8493

Merged
merged 4 commits into from
Apr 13, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ include_rules = [
"+chrome/installer/util",
"+chrome/services/mac_notifications/public/cpp",
"+chrome/test",
"+content/browser/service_worker",
"+content/public/browser",
"+content/public/common",
"+content/public/test",
Expand Down
17 changes: 12 additions & 5 deletions browser/brave_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "components/services/heap_profiling/public/mojom/heap_profiling_client.mojom.h"
#include "components/version_info/version_info.h"
#include "content/browser/renderer_host/render_frame_host_impl.h"
#include "content/browser/service_worker/service_worker_host.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
Expand All @@ -65,6 +66,7 @@
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "net/cookies/site_for_cookies.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_registry.h"
#include "third_party/blink/public/common/loader/url_loader_throttle.h"
#include "ui/base/l10n/l10n_util.h"

Expand Down Expand Up @@ -267,14 +269,12 @@ void MaybeBindBraveWalletProvider(
#endif

void BindBraveSearchHost(
content::RenderFrameHost* const frame_host,
mojo::PendingReceiver<brave_search::mojom::BraveSearchFallback> receiver) {
mojo::MakeSelfOwnedReceiver(
std::make_unique<brave_search::BraveSearchHost>(
g_brave_browser_process->shared_url_loader_factory()),
std::move(receiver));
}

} // namespace

BraveContentBrowserClient::BraveContentBrowserClient()
Expand Down Expand Up @@ -327,6 +327,16 @@ BraveContentBrowserClient::AllowWebBluetooth(
return ContentBrowserClient::AllowWebBluetoothResult::BLOCK_GLOBALLY_DISABLED;
}

void BraveContentBrowserClient::ExposeInterfacesToRenderer(
service_manager::BinderRegistry* registry,
blink::AssociatedInterfaceRegistry* associated_registry,
content::RenderProcessHost* render_process_host) {
ChromeContentBrowserClient::ExposeInterfacesToRenderer(
registry, associated_registry, render_process_host);
registry->AddInterface(base::BindRepeating(&BindBraveSearchHost),
content::GetUIThreadTaskRunner({}));
}

void BraveContentBrowserClient::RegisterBrowserInterfaceBindersForFrame(
content::RenderFrameHost* render_frame_host,
mojo::BinderMapWithContext<content::RenderFrameHost*>* map) {
Expand All @@ -341,9 +351,6 @@ void BraveContentBrowserClient::RegisterBrowserInterfaceBindersForFrame(
base::BindRepeating(&MaybeBindBraveWalletProvider));
}
#endif

map->Add<brave_search::mojom::BraveSearchFallback>(
base::BindRepeating(&BindBraveSearchHost));
}

bool BraveContentBrowserClient::HandleExternalProtocol(
Expand Down
10 changes: 9 additions & 1 deletion browser/brave_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ class PrefChangeRegistrar;
namespace content {
class BrowserContext;
class RenderProcessHost;
}
} // namespace content

namespace blink {
class AssociatedInterfaceRegistry;
} // namespace blink

class BraveContentBrowserClient : public ChromeContentBrowserClient {
public:
Expand Down Expand Up @@ -109,6 +113,10 @@ class BraveContentBrowserClient : public ChromeContentBrowserClient {

std::string GetEffectiveUserAgent(content::BrowserContext* browser_context,
const GURL& url) override;
void ExposeInterfacesToRenderer(
service_manager::BinderRegistry* registry,
blink::AssociatedInterfaceRegistry* associated_registry,
content::RenderProcessHost* render_process_host) override;

private:
friend class ::BraveNavigatorUserAgentFarblingBrowserTest;
Expand Down
6 changes: 3 additions & 3 deletions browser/brave_search/brave_search_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class BraveSearchTest : public InProcessBrowserTest {
std::unique_ptr<net::EmbeddedTestServer> https_server_;
};

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

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

IN_PROC_BROWSER_TEST_F(BraveSearchTest, CheckForAnUndefinedFunction) {
IN_PROC_BROWSER_TEST_F(BraveSearchTest, DISABLED_CheckForAnUndefinedFunction) {
GURL url = https_server()->GetURL(kNotAllowedDomain, "/simple.html");
ui_test_utils::NavigateToURL(browser(), url);
content::WebContents* contents =
Expand Down
4 changes: 2 additions & 2 deletions components/brave_search/renderer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ source_set("renderer") {
sources = [
"brave_search_js_handler.cc",
"brave_search_js_handler.h",
"brave_search_render_frame_observer.cc",
"brave_search_render_frame_observer.h",
"brave_search_sw_holder.cc",
"brave_search_sw_holder.h",
]

deps = [
Expand Down
99 changes: 55 additions & 44 deletions components/brave_search/renderer/brave_search_js_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,72 +13,86 @@
#include "gin/arguments.h"
#include "gin/function_template.h"
#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
#include "third_party/blink/public/common/thread_safe_browser_interface_broker_proxy.h"
#include "third_party/blink/public/web/blink.h"
#include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/public/web/web_script_source.h"

namespace brave_search {

BraveSearchJSHandler::BraveSearchJSHandler(content::RenderFrame* render_frame)
: render_frame_(render_frame) {}
BraveSearchJSHandler::BraveSearchJSHandler(
v8::Local<v8::Context> v8_context,
blink::ThreadSafeBrowserInterfaceBrokerProxy* broker)
: broker_(broker),
context_(v8_context->GetIsolate(), v8_context),
isolate_(v8_context->GetIsolate()) {}

BraveSearchJSHandler::~BraveSearchJSHandler() = default;

bool BraveSearchJSHandler::EnsureConnected() {
if (!brave_search_fallback_.is_bound()) {
render_frame_->GetBrowserInterfaceBroker()->GetInterface(
brave_search_fallback_.BindNewPipeAndPassReceiver());
if (!brave_search_fallback_.is_bound() && broker_) {
broker_->GetInterface(brave_search_fallback_.BindNewPipeAndPassReceiver());
}

return brave_search_fallback_.is_bound();
}

void BraveSearchJSHandler::AddJavaScriptObjectToFrame(
v8::Local<v8::Context> context) {
v8::Isolate* isolate = blink::MainThreadIsolate();
v8::HandleScope handle_scope(isolate);
if (context.IsEmpty())
v8::Local<v8::Context> BraveSearchJSHandler::Context() {
return v8::Local<v8::Context>::New(isolate_, context_);
}

v8::Isolate* BraveSearchJSHandler::GetIsolate() {
return isolate_;
}

void BraveSearchJSHandler::Invalidate() {
context_.Reset();
}

void BraveSearchJSHandler::AddJavaScriptObject() {
v8::HandleScope handle_scope(isolate_);
if (Context().IsEmpty())
return;

v8::Context::Scope context_scope(context);
v8::Context::Scope context_scope(Context());

BindFunctionsToObject(isolate, context);
BindFunctionsToObject();
}

void BraveSearchJSHandler::BindFunctionsToObject(
v8::Isolate* isolate,
v8::Local<v8::Context> context) {
v8::Local<v8::Object> global = context->Global();
v8::Local<v8::Value> chrome_value;
if (global->Get(context, gin::StringToV8(isolate, "chrome"))
.ToLocal(&chrome_value) &&
chrome_value->IsObject()) {
BindFunctionToObject(
isolate, chrome_value->ToObject(context).ToLocalChecked(),
"fetchBackupResults",
base::BindRepeating(&BraveSearchJSHandler::FetchBackupResults,
base::Unretained(this), isolate));
void BraveSearchJSHandler::BindFunctionsToObject() {
v8::Local<v8::Object> global = Context()->Global();
v8::Local<v8::Object> brave_obj;
v8::Local<v8::Value> brave_value;
if (!global->Get(Context(), gin::StringToV8(isolate_, "brave"))
.ToLocal(&brave_value) ||
!brave_value->IsObject()) {
brave_obj = v8::Object::New(isolate_);
global->Set(Context(), gin::StringToSymbol(isolate_, "brave"), brave_obj)
.Check();
} else {
brave_obj = brave_value->ToObject(Context()).ToLocalChecked();
}

BindFunctionToObject(
brave_obj, "fetchBackupResults",
base::BindRepeating(&BraveSearchJSHandler::FetchBackupResults,
base::Unretained(this)));
}

template <typename Sig>
void BraveSearchJSHandler::BindFunctionToObject(
v8::Isolate* isolate,
v8::Local<v8::Object> javascript_object,
const std::string& name,
const base::RepeatingCallback<Sig>& callback) {
v8::Local<v8::Context> context = isolate->GetCurrentContext();
// Get the isolate associated with this object.
javascript_object
->Set(context, gin::StringToSymbol(isolate, name),
gin::CreateFunctionTemplate(isolate, callback)
->GetFunction(context)
->Set(Context(), gin::StringToSymbol(isolate_, name),
gin::CreateFunctionTemplate(isolate_, callback)
->GetFunction(Context())
.ToLocalChecked())
.Check();
}

v8::Local<v8::Promise> BraveSearchJSHandler::FetchBackupResults(
v8::Isolate* isolate,
const std::string& query_string,
const std::string& lang,
const std::string& country,
Expand All @@ -87,18 +101,15 @@ v8::Local<v8::Promise> BraveSearchJSHandler::FetchBackupResults(
return v8::Local<v8::Promise>();

v8::MaybeLocal<v8::Promise::Resolver> resolver =
v8::Promise::Resolver::New(isolate->GetCurrentContext());
v8::Promise::Resolver::New(Context());
if (!resolver.IsEmpty()) {
auto promise_resolver =
std::make_unique<v8::Global<v8::Promise::Resolver>>();
promise_resolver->Reset(isolate, resolver.ToLocalChecked());
auto context_old = std::make_unique<v8::Global<v8::Context>>(
isolate, isolate->GetCurrentContext());
promise_resolver->Reset(isolate_, resolver.ToLocalChecked());
brave_search_fallback_->FetchBackupResults(
query_string, lang, country, geo,
base::BindOnce(&BraveSearchJSHandler::OnFetchBackupResults,
base::Unretained(this), std::move(promise_resolver),
isolate, std::move(context_old)));
base::Unretained(this), std::move(promise_resolver)));

return resolver.ToLocalChecked()->GetPromise();
}
Expand All @@ -108,16 +119,16 @@ v8::Local<v8::Promise> BraveSearchJSHandler::FetchBackupResults(

void BraveSearchJSHandler::OnFetchBackupResults(
std::unique_ptr<v8::Global<v8::Promise::Resolver>> promise_resolver,
v8::Isolate* isolate,
std::unique_ptr<v8::Global<v8::Context>> context_old,
const std::string& response) {
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Context> context = context_old->Get(isolate);
v8::HandleScope handle_scope(isolate_);
v8::Local<v8::Context> context = context_.Get(isolate_);
v8::Context::Scope context_scope(context);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you also want v8::MicrotasksScope microtasks(isolate(), v8::MicrotasksScope::kDoNotRunMicrotasks); here.

v8::MicrotasksScope microtasks(isolate_,
v8::MicrotasksScope::kDoNotRunMicrotasks);

v8::Local<v8::Promise::Resolver> resolver = promise_resolver->Get(isolate);
v8::Local<v8::Promise::Resolver> resolver = promise_resolver->Get(isolate_);
v8::Local<v8::String> result;
result = v8::String::NewFromUtf8(isolate, response.c_str()).ToLocalChecked();
result = v8::String::NewFromUtf8(isolate_, response.c_str()).ToLocalChecked();

ALLOW_UNUSED_LOCAL(resolver->Resolve(context, result));
}
Expand Down
30 changes: 16 additions & 14 deletions components/brave_search/renderer/brave_search_js_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,48 +11,50 @@
#include <vector>

#include "brave/components/brave_search/common/brave_search.mojom.h"
#include "content/public/renderer/render_frame.h"
#include "content/public/renderer/render_frame_observer.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "url/gurl.h"
#include "v8/include/v8.h"

namespace blink {
class ThreadSafeBrowserInterfaceBrokerProxy;
}

namespace brave_search {

class BraveSearchJSHandler {
public:
explicit BraveSearchJSHandler(content::RenderFrame* render_frame);
BraveSearchJSHandler(v8::Local<v8::Context> v8_context,
blink::ThreadSafeBrowserInterfaceBrokerProxy* broker);
BraveSearchJSHandler(const BraveSearchJSHandler&) = delete;
BraveSearchJSHandler& operator=(const BraveSearchJSHandler&) = delete;
~BraveSearchJSHandler();

void AddJavaScriptObjectToFrame(v8::Local<v8::Context> context);
void AddJavaScriptObject();
void Invalidate();
v8::Local<v8::Context> Context();
v8::Isolate* GetIsolate();

private:
// Adds a function to the provided object.
template <typename Sig>
void BindFunctionToObject(v8::Isolate* isolate,
v8::Local<v8::Object> javascript_object,
void BindFunctionToObject(v8::Local<v8::Object> javascript_object,
const std::string& name,
const base::RepeatingCallback<Sig>& callback);
void BindFunctionsToObject(v8::Isolate* isolate,
v8::Local<v8::Context> context);
void BindFunctionsToObject();
bool EnsureConnected();

// A function to be called from JS
v8::Local<v8::Promise> FetchBackupResults(v8::Isolate* isolate,
const std::string& query_string,
v8::Local<v8::Promise> FetchBackupResults(const std::string& query_string,
const std::string& lang,
const std::string& country,
const std::string& geo);
void OnFetchBackupResults(
std::unique_ptr<v8::Global<v8::Promise::Resolver>> promise_resolver,
v8::Isolate* isolate,
std::unique_ptr<v8::Global<v8::Context>> context_old,
const std::string& response);

content::RenderFrame* render_frame_;
blink::ThreadSafeBrowserInterfaceBrokerProxy* broker_; // not owned
mojo::Remote<brave_search::mojom::BraveSearchFallback> brave_search_fallback_;
v8::Global<v8::Context> context_;
v8::Isolate* isolate_;
};

} // namespace brave_search
Expand Down
Loading