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

Use thread local with BraveSearchServiceWorkerHolder (#17656)

Browse Source 
A CL has been landed in `cr113` to remove `base::ThreadLocal` variants,
and replace their use with C++'s `thread_local`.

In this change, `base::ThreadLocal` is being replaced with
`thread_local` in `BraveSearchServiceWorkerHolder`.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/4f39823862fe722c32bd72bdb0e126e952574b79

commit 4f39823862fe722c32bd72bdb0e126e952574b79
Author: Peter Kasting <pkasting@chromium.org>
Date:   Thu Mar 16 01:34:24 2023 +0000

    Remove ThreadLocal{Pointer,Boolean}.

    These are now unused; `thread_local T*/bool` is generally a better
    option.

    Bug: 1416710
This commit is contained in:
cdesouza-chromium
2023-03-24 12:03:38 +00:00
committed by GitHub
parent ea80f050ab
commit 12c1ffb353
2 changed files with 104 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
components/brave_search/renderer/brave_search_service_worker_holder.cc
+102 -35
View File
@@ -5,34 +5,118 @@
#include "brave/components/brave_search/renderer/brave_search_service_worker_holder.h"
#include <string>
#include <memory>
#include <utility>
#include <vector>
#include "base/no_destructor.h"
#include "base/auto_reset.h"
#include "base/ranges/algorithm.h"
#include "base/threading/thread_checker.h"
#include "brave/components/brave_search/common/brave_search_utils.h"
#include "brave/components/brave_search/renderer/brave_search_fallback_js_handler.h"
#include "content/public/renderer/worker_thread.h"
#include "third_party/abseil-cpp/absl/base/attributes.h"
#include "url/gurl.h"
namespace brave_search {
using JSHandlersVector =
std::vector<std::unique_ptr<BraveSearchFallbackJSHandler>>;
namespace {
JSHandlersVector::iterator FindContext(JSHandlersVector* contexts,
v8::Local<v8::Context> v8_context) {
auto context_matches =
[&v8_context](
const std::unique_ptr<BraveSearchFallbackJSHandler>& context) {
v8::HandleScope handle_scope(context->GetIsolate());
v8::Context::Scope context_scope(context->Context());
class JsHandlersForCurrentThread;
return context->Context() == v8_context;
};
// A thread local pointer for the js handlers available.
ABSL_CONST_INIT thread_local JsHandlersForCurrentThread* current_js_handlers =
nullptr;
return base::ranges::find_if(*contexts, context_matches);
// A scoping class to create a local thread storage for the JS handlers, storing
// BraveSearchFallbackJSHandler instance in local thread storage, and providing
// ways to add/remove them. The instance self deletes on
// content::WorkerThread::Observer::WillStopCurrentWorkerThread.
class [[maybe_unused, nodiscard]] JsHandlersForCurrentThread
: public content::WorkerThread::Observer {
public:
JsHandlersForCurrentThread(const JsHandlersForCurrentThread&) = delete;
JsHandlersForCurrentThread& operator=(const JsHandlersForCurrentThread&) =
delete;
~JsHandlersForCurrentThread() override;
// Gets the current js handlers, or creates an instance of it for the current
// thread.
static JsHandlersForCurrentThread* Get();
// Adds a JS handler to the scope.
void AddJsHandler(std::unique_ptr<BraveSearchFallbackJSHandler> js_handler);
// Searches and deletes
void RemoveContext(const v8::Local<v8::Context>& v8_context);
private:
JsHandlersForCurrentThread();
// WorkerThread::Observer:
void WillStopCurrentWorkerThread() override;
// A vector for the handlers being held.
std::vector<std::unique_ptr<BraveSearchFallbackJSHandler>> js_handlers_;
// A resetter responsible to make sure the local thread storage is set back to
// nullptr once the scope is gone.
const base::AutoReset<JsHandlersForCurrentThread*> resetter_;
// This object should always be constructed and destructed on the same thread.
THREAD_CHECKER(thread_checker_);
};
JsHandlersForCurrentThread::JsHandlersForCurrentThread()
: resetter_(&current_js_handlers, this) {
content::WorkerThread::AddObserver(this);
}
JsHandlersForCurrentThread::~JsHandlersForCurrentThread() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
content::WorkerThread::RemoveObserver(this);
for (auto& js_handler : js_handlers_) {
js_handler->Invalidate();
}
}
// static
JsHandlersForCurrentThread* JsHandlersForCurrentThread::Get() {
if (current_js_handlers) {
return current_js_handlers;
}
return new JsHandlersForCurrentThread();
}
void JsHandlersForCurrentThread::AddJsHandler(
std::unique_ptr<BraveSearchFallbackJSHandler> js_handler) {
js_handlers_.push_back(std::move(js_handler));
}
void JsHandlersForCurrentThread::RemoveContext(
const v8::Local<v8::Context>& v8_context) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
js_handlers_.erase(
base::ranges::remove_if(
js_handlers_,
[&v8_context](
const std::unique_ptr<BraveSearchFallbackJSHandler>& js_handler) {
v8::HandleScope handle_scope(js_handler->GetIsolate());
v8::Context::Scope context_scope(js_handler->Context());
return js_handler->Context() == v8_context;
}),
js_handlers_.end());
}
void JsHandlersForCurrentThread::WillStopCurrentWorkerThread() {
DCHECK(current_js_handlers);
delete this;
}
} // namespace
BraveSearchServiceWorkerHolder::BraveSearchServiceWorkerHolder()
: broker_(nullptr) {}
@@ -59,13 +143,7 @@ void BraveSearchServiceWorkerHolder::WillEvaluateServiceWorkerOnWorkerThread(
new BraveSearchFallbackJSHandler(v8_context, broker_));
js_handler->AddJavaScriptObject();
JSHandlersVector* js_handlers = js_handlers_tls_.Get();
if (!js_handlers) {
js_handlers = new JSHandlersVector();
js_handlers_tls_.Set(js_handlers);
content::WorkerThread::AddObserver(this);
}
js_handlers->push_back(std::move(js_handler));
JsHandlersForCurrentThread::Get()->AddJsHandler(std::move(js_handler));
}
void BraveSearchServiceWorkerHolder::
@@ -79,22 +157,11 @@ void BraveSearchServiceWorkerHolder::
!IsAllowedHost(service_worker_scope))
return;
JSHandlersVector* js_handlers = js_handlers_tls_.Get();
if (!js_handlers)
if (!current_js_handlers) {
return;
}
auto context_it = FindContext(js_handlers, v8_context);
js_handlers->erase(context_it);
}
void BraveSearchServiceWorkerHolder::WillStopCurrentWorkerThread() {
content::WorkerThread::RemoveObserver(this);
JSHandlersVector* js_handlers = js_handlers_tls_.Get();
DCHECK(js_handlers);
for (const auto& context : *js_handlers)
context->Invalidate();
js_handlers_tls_.Set(nullptr);
delete js_handlers;
current_js_handlers->RemoveContext(v8_context);
}
} // namespace brave_search
components/brave_search/renderer/brave_search_service_worker_holder.h
+2 -17
View File
@@ -6,12 +6,7 @@
#ifndef BRAVE_COMPONENTS_BRAVE_SEARCH_RENDERER_BRAVE_SEARCH_SERVICE_WORKER_HOLDER_H_
#define BRAVE_COMPONENTS_BRAVE_SEARCH_RENDERER_BRAVE_SEARCH_SERVICE_WORKER_HOLDER_H_
#include <memory>
#include <vector>
#include "base/memory/raw_ptr.h"
#include "base/threading/thread_local.h"
#include "content/public/renderer/worker_thread.h"
#include "v8/include/v8.h"
class GURL;
@@ -23,16 +18,14 @@ class ThreadSafeBrowserInterfaceBrokerProxy;
namespace brave_search {
class BraveSearchFallbackJSHandler;
class BraveSearchServiceWorkerHolder : public content::WorkerThread::Observer {
class BraveSearchServiceWorkerHolder {
public:
BraveSearchServiceWorkerHolder();
BraveSearchServiceWorkerHolder(const BraveSearchServiceWorkerHolder&) =
delete;
BraveSearchServiceWorkerHolder& operator=(
const BraveSearchServiceWorkerHolder&) = delete;
~BraveSearchServiceWorkerHolder() override;
~BraveSearchServiceWorkerHolder();
void SetBrowserInterfaceBrokerProxy(
blink::ThreadSafeBrowserInterfaceBrokerProxy* broker);
@@ -49,14 +42,6 @@ class BraveSearchServiceWorkerHolder : public content::WorkerThread::Observer {
const GURL& script_url);
private:
// WorkerThread::Observer:
void WillStopCurrentWorkerThread() override;
// Implement thread safety by storing each BraveSearchFallbackJSHandler
// in TLS. The vector is called from worker threads.
base::ThreadLocalPointer<
std::vector<std::unique_ptr<BraveSearchFallbackJSHandler>>>
js_handlers_tls_;
raw_ptr<blink::ThreadSafeBrowserInterfaceBrokerProxy> broker_ =
nullptr; // not owned
};