Commit 19571c08 authored by Francois Doray's avatar Francois Doray Committed by Chromium LUCI CQ

[blink scheduler] Remove blink::Thread::ThreadId.

To support blink::Thread::ThreadId(), a synchronous wait is necessary
on blink::WorkerThread creation. To make it possible to remove the
synchronous wait in an upcoming CL, we remove
blink::Thread::ThreadId() in this CL.

Usage is replaced by other mechanisms:

1. Change the priority of the compositor thread
After creating the compositor thread, we post a task to it to obtain its
tid via base::PlatformThread::CurrentId(). We send the obtained tid to
the browser process so it can set the desired thread priority.

2. Store the tid in WorkerInspectorController
Since the code runs on the worker thread, we can just invoke
base::PlatformThread::CurrentId() instead of blink::Thread::ThreadId().

3. Generate an AgentId for testing
Instead of using the tid of the main thread as a unique identifier,
we use base::GetCurrentProcId().

Bug: 1080709
Change-Id: If92718a2e28b1d9e8ab9f259c95a5f27aee87e66
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2580005
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843041}
parent b8888319
...@@ -233,15 +233,15 @@ RendererBlinkPlatformImpl::WrapSharedURLLoaderFactory( ...@@ -233,15 +233,15 @@ RendererBlinkPlatformImpl::WrapSharedURLLoaderFactory(
std::move(factory)); std::move(factory));
} }
#if defined(OS_LINUX) || defined(OS_CHROMEOS)
void RendererBlinkPlatformImpl::SetDisplayThreadPriority( void RendererBlinkPlatformImpl::SetDisplayThreadPriority(
base::PlatformThreadId thread_id) { base::PlatformThreadId thread_id) {
#if defined(OS_LINUX) || defined(OS_CHROMEOS)
if (RenderThreadImpl* render_thread = RenderThreadImpl::current()) { if (RenderThreadImpl* render_thread = RenderThreadImpl::current()) {
render_thread->render_message_filter()->SetThreadPriority( render_thread->render_message_filter()->SetThreadPriority(
thread_id, base::ThreadPriority::DISPLAY); thread_id, base::ThreadPriority::DISPLAY);
} }
#endif
} }
#endif
blink::BlameContext* RendererBlinkPlatformImpl::GetTopLevelBlameContext() { blink::BlameContext* RendererBlinkPlatformImpl::GetTopLevelBlameContext() {
return &top_level_blame_context_; return &top_level_blame_context_;
......
...@@ -183,7 +183,9 @@ class CONTENT_EXPORT RendererBlinkPlatformImpl : public BlinkPlatformImpl { ...@@ -183,7 +183,9 @@ class CONTENT_EXPORT RendererBlinkPlatformImpl : public BlinkPlatformImpl {
const blink::WebURL& top_document_web_url) override; const blink::WebURL& top_document_web_url) override;
gpu::GpuMemoryBufferManager* GetGpuMemoryBufferManager() override; gpu::GpuMemoryBufferManager* GetGpuMemoryBufferManager() override;
blink::WebString ConvertIDNToUnicode(const blink::WebString& host) override; blink::WebString ConvertIDNToUnicode(const blink::WebString& host) override;
#if defined(OS_LINUX) || defined(OS_CHROMEOS)
void SetDisplayThreadPriority(base::PlatformThreadId thread_id) override; void SetDisplayThreadPriority(base::PlatformThreadId thread_id) override;
#endif
blink::BlameContext* GetTopLevelBlameContext() override; blink::BlameContext* GetTopLevelBlameContext() override;
std::unique_ptr<blink::WebDedicatedWorkerHostFactoryClient> std::unique_ptr<blink::WebDedicatedWorkerHostFactoryClient>
CreateDedicatedWorkerHostFactoryClient( CreateDedicatedWorkerHostFactoryClient(
......
...@@ -393,12 +393,14 @@ class BLINK_PLATFORM_EXPORT Platform { ...@@ -393,12 +393,14 @@ class BLINK_PLATFORM_EXPORT Platform {
return nullptr; return nullptr;
} }
#if defined(OS_LINUX) || defined(OS_CHROMEOS)
// This is called after the compositor thread is created, so the embedder // This is called after the compositor thread is created, so the embedder
// can initiate an IPC to change its thread priority (on Linux we can't // can initiate an IPC to change its thread priority (on Linux we can't
// increase the nice value, so we need to ask the browser process). This // increase the nice value, so we need to ask the browser process). This
// function is only called from the main thread (where InitializeCompositor- // function is only called from the main thread (where InitializeCompositor-
// Thread() is called). // Thread() is called).
virtual void SetDisplayThreadPriority(base::PlatformThreadId) {} virtual void SetDisplayThreadPriority(base::PlatformThreadId) {}
#endif
// Returns a blame context for attributing top-level work which does not // Returns a blame context for attributing top-level work which does not
// belong to a particular frame scope. // belong to a particular frame scope.
......
...@@ -74,7 +74,13 @@ WorkerInspectorController::WorkerInspectorController( ...@@ -74,7 +74,13 @@ WorkerInspectorController::WorkerInspectorController(
: debugger_(debugger), : debugger_(debugger),
thread_(thread), thread_(thread),
inspected_frames_(nullptr), inspected_frames_(nullptr),
probe_sink_(MakeGarbageCollected<CoreProbeSink>()) { probe_sink_(MakeGarbageCollected<CoreProbeSink>()),
worker_thread_id_(base::PlatformThread::CurrentId()) {
// The constructor must run on the backing thread of |thread|. Otherwise, it
// would be incorrect to initialize |worker_thread_id_| with the current
// thread id.
DCHECK(thread->IsCurrentThread());
probe_sink_->AddInspectorIssueReporter( probe_sink_->AddInspectorIssueReporter(
MakeGarbageCollected<InspectorIssueReporter>( MakeGarbageCollected<InspectorIssueReporter>(
thread->GetInspectorIssueStorage())); thread->GetInspectorIssueStorage()));
...@@ -83,7 +89,6 @@ WorkerInspectorController::WorkerInspectorController( ...@@ -83,7 +89,6 @@ WorkerInspectorController::WorkerInspectorController(
worker_devtools_token_ = devtools_params->devtools_worker_token; worker_devtools_token_ = devtools_params->devtools_worker_token;
parent_devtools_token_ = thread->GlobalScope()->GetParentDevToolsToken(); parent_devtools_token_ = thread->GlobalScope()->GetParentDevToolsToken();
url_ = url; url_ = url;
worker_thread_id_ = thread->GetPlatformThreadId();
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner = scoped_refptr<base::SingleThreadTaskRunner> io_task_runner =
Platform::Current()->GetIOTaskRunner(); Platform::Current()->GetIOTaskRunner();
if (!parent_devtools_token_.is_empty() && io_task_runner) { if (!parent_devtools_token_.is_empty() && io_task_runner) {
......
...@@ -109,7 +109,7 @@ class WorkerInspectorController final ...@@ -109,7 +109,7 @@ class WorkerInspectorController final
base::UnguessableToken worker_devtools_token_; base::UnguessableToken worker_devtools_token_;
base::UnguessableToken parent_devtools_token_; base::UnguessableToken parent_devtools_token_;
KURL url_; KURL url_;
PlatformThreadId worker_thread_id_; const PlatformThreadId worker_thread_id_;
DISALLOW_COPY_AND_ASSIGN(WorkerInspectorController); DISALLOW_COPY_AND_ASSIGN(WorkerInspectorController);
}; };
......
include_rules = [ include_rules = [
"+base/run_loop.h", "+base/run_loop.h",
"+base/process/process_handle.h",
"+cc", "+cc",
"+components/ukm/test_ukm_recorder.h", "+components/ukm/test_ukm_recorder.h",
# TODO(crbug.com/838693): Test harnesses use LayerTreeView # TODO(crbug.com/838693): Test harnesses use LayerTreeView
......
...@@ -30,6 +30,7 @@ ...@@ -30,6 +30,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/process/process_handle.h"
#include "cc/layers/picture_layer.h" #include "cc/layers/picture_layer.h"
#include "cc/trees/layer_tree_host.h" #include "cc/trees/layer_tree_host.h"
#include "gpu/command_buffer/client/gles2_interface.h" #include "gpu/command_buffer/client/gles2_interface.h"
...@@ -3592,10 +3593,8 @@ String Internals::getAgentId(DOMWindow* window) { ...@@ -3592,10 +3593,8 @@ String Internals::getAgentId(DOMWindow* window) {
if (!window->IsLocalDOMWindow()) if (!window->IsLocalDOMWindow())
return String(); return String();
// Sounds like there's no notion of "process ID" in Blink, but the main // Create a unique id from the process id and the address of the agent.
// thread's thread ID serves for that purpose. const base::ProcessId process_id = base::GetCurrentProcId();
PlatformThreadId process_id = Thread::MainThread()->ThreadId();
uintptr_t agent_address = uintptr_t agent_address =
reinterpret_cast<uintptr_t>(To<LocalDOMWindow>(window)->GetAgent()); reinterpret_cast<uintptr_t>(To<LocalDOMWindow>(window)->GetAgent());
......
...@@ -400,10 +400,6 @@ HashSet<WorkerThread*>& WorkerThread::WorkerThreads() { ...@@ -400,10 +400,6 @@ HashSet<WorkerThread*>& WorkerThread::WorkerThreads() {
return threads; return threads;
} }
PlatformThreadId WorkerThread::GetPlatformThreadId() {
return GetWorkerBackingThread().BackingThread().ThreadId();
}
bool WorkerThread::IsForciblyTerminated() { bool WorkerThread::IsForciblyTerminated() {
MutexLocker lock(mutex_); MutexLocker lock(mutex_);
switch (exit_code_) { switch (exit_code_) {
......
...@@ -219,8 +219,6 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver { ...@@ -219,8 +219,6 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
int GetWorkerThreadId() const { return worker_thread_id_; } int GetWorkerThreadId() const { return worker_thread_id_; }
PlatformThreadId GetPlatformThreadId();
bool IsForciblyTerminated() LOCKS_EXCLUDED(mutex_); bool IsForciblyTerminated() LOCKS_EXCLUDED(mutex_);
void WaitForShutdownForTesting(); void WaitForShutdownForTesting();
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/synchronization/waitable_event.h" #include "base/synchronization/waitable_event.h"
#include "base/threading/platform_thread.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "third_party/blink/public/common/features.h" #include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/platform/platform.h" #include "third_party/blink/public/platform/platform.h"
...@@ -94,16 +95,22 @@ void Thread::CreateAndSetCompositorThread() { ...@@ -94,16 +95,22 @@ void Thread::CreateAndSetCompositorThread() {
auto compositor_thread = auto compositor_thread =
std::make_unique<scheduler::CompositorThread>(params); std::make_unique<scheduler::CompositorThread>(params);
compositor_thread->Init(); compositor_thread->Init();
GetCompositorThread() = std::move(compositor_thread);
#if defined(OS_LINUX) || defined(OS_CHROMEOS)
if (base::FeatureList::IsEnabled( if (base::FeatureList::IsEnabled(
features::kBlinkCompositorUseDisplayThreadPriority)) { features::kBlinkCompositorUseDisplayThreadPriority)) {
// Chrome OS moves tasks between control groups on thread priority changes. compositor_thread->GetTaskRunner()->PostTaskAndReplyWithResult(
// This is not possible inside the sandbox, so ask the browser to do it. FROM_HERE, base::BindOnce(&base::PlatformThread::CurrentId),
// TODO(spang): Check if we can remove this on non-Chrome OS builds. base::BindOnce([](base::PlatformThreadId compositor_thread_id) {
Platform::Current()->SetDisplayThreadPriority( // Chrome OS moves tasks between control groups on thread priority
GetCompositorThread()->ThreadId()); // changes. This is not possible inside the sandbox, so ask the
// browser to do it.
Platform::Current()->SetDisplayThreadPriority(compositor_thread_id);
}));
} }
#endif
GetCompositorThread() = std::move(compositor_thread);
} }
Thread* Thread::Current() { Thread* Thread::Current() {
......
...@@ -12,16 +12,10 @@ namespace blink { ...@@ -12,16 +12,10 @@ namespace blink {
namespace scheduler { namespace scheduler {
MainThread::MainThread(MainThreadSchedulerImpl* scheduler) MainThread::MainThread(MainThreadSchedulerImpl* scheduler)
: task_runner_(scheduler->DefaultTaskRunner()), : task_runner_(scheduler->DefaultTaskRunner()), scheduler_(scheduler) {}
scheduler_(scheduler),
thread_id_(base::PlatformThread::CurrentId()) {}
MainThread::~MainThread() = default; MainThread::~MainThread() = default;
blink::PlatformThreadId MainThread::ThreadId() const {
return thread_id_;
}
blink::ThreadScheduler* MainThread::Scheduler() { blink::ThreadScheduler* MainThread::Scheduler() {
return scheduler_; return scheduler_;
} }
......
...@@ -25,7 +25,6 @@ class PLATFORM_EXPORT MainThread : public Thread { ...@@ -25,7 +25,6 @@ class PLATFORM_EXPORT MainThread : public Thread {
// Thread implementation. // Thread implementation.
ThreadScheduler* Scheduler() override; ThreadScheduler* Scheduler() override;
PlatformThreadId ThreadId() const override;
scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner() const override; scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner() const override;
void AddTaskTimeObserver(base::sequence_manager::TaskTimeObserver*) override; void AddTaskTimeObserver(base::sequence_manager::TaskTimeObserver*) override;
...@@ -35,7 +34,6 @@ class PLATFORM_EXPORT MainThread : public Thread { ...@@ -35,7 +34,6 @@ class PLATFORM_EXPORT MainThread : public Thread {
private: private:
scoped_refptr<base::SingleThreadTaskRunner> task_runner_; scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
MainThreadSchedulerImpl* scheduler_; // Not owned. MainThreadSchedulerImpl* scheduler_; // Not owned.
PlatformThreadId thread_id_;
}; };
} // namespace scheduler } // namespace scheduler
......
...@@ -126,7 +126,6 @@ class PLATFORM_EXPORT Thread { ...@@ -126,7 +126,6 @@ class PLATFORM_EXPORT Thread {
} }
bool IsCurrentThread() const; bool IsCurrentThread() const;
virtual PlatformThreadId ThreadId() const { return 0; }
// TaskObserver is an object that receives task notifications from the // TaskObserver is an object that receives task notifications from the
// MessageLoop. // MessageLoop.
......
...@@ -73,10 +73,6 @@ WorkerThread::CreateNonMainThreadScheduler( ...@@ -73,10 +73,6 @@ WorkerThread::CreateNonMainThreadScheduler(
worker_scheduler_proxy_.get()); worker_scheduler_proxy_.get());
} }
blink::PlatformThreadId WorkerThread::ThreadId() const {
return thread_->tid();
}
blink::ThreadScheduler* WorkerThread::Scheduler() { blink::ThreadScheduler* WorkerThread::Scheduler() {
return thread_->GetNonMainThreadScheduler(); return thread_->GetNonMainThreadScheduler();
} }
......
...@@ -42,7 +42,6 @@ class PLATFORM_EXPORT WorkerThread : public Thread { ...@@ -42,7 +42,6 @@ class PLATFORM_EXPORT WorkerThread : public Thread {
// Thread implementation. // Thread implementation.
void Init() override; void Init() override;
ThreadScheduler* Scheduler() override; ThreadScheduler* Scheduler() override;
PlatformThreadId ThreadId() const override;
scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner() const override; scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner() const override;
scheduler::NonMainThreadSchedulerImpl* GetNonMainThreadScheduler() { scheduler::NonMainThreadSchedulerImpl* GetNonMainThreadScheduler() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment