Skip to content

Commit

Permalink
Remove MessageLoop from content::RenderThreadImpl.
Browse files Browse the repository at this point in the history 
It only needs main thread scheduler to initialize,
MessageLoop was unnecessary there.

Bug: 828835
Change-Id: I70235bbd06af1ca6c576cc77e3e4722a27225069
Reviewed-on: https://chromium-review.googlesource.com/1169211
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Greg Kraynov <kraynov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582125}
  • Loading branch information
Greg Kraynov authored and Commit Bot committed Aug 10, 2018
1 parent 11dbee4 commit f5d6027
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 122 deletions.
9 changes: 8 additions & 1 deletion content/renderer/in_process_renderer_thread.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "content/renderer/render_process.h"
#include "content/renderer/render_process_impl.h"
#include "content/renderer/render_thread_impl.h"
#include "third_party/blink/public/platform/scheduler/web_thread_scheduler.h"

#if defined(OS_ANDROID)
#include "base/android/jni_android.h"
Expand Down Expand Up @@ -46,8 +47,14 @@ void InProcessRendererThread::Init() {
// Android. Temporary CHECK() to debug http://crbug.com/514141
CHECK(!render_process_);
#endif
std::unique_ptr<blink::scheduler::WebThreadScheduler> main_thread_scheduler =
blink::scheduler::WebThreadScheduler::CreateMainThreadScheduler();

render_process_ = RenderProcessImpl::Create();
RenderThreadImpl::Create(params_, message_loop());
// RenderThreadImpl doesn't currently support a proper shutdown sequence
// and it's okay when we're running in multi-process mode because renderers
// get killed by the OS. In-process mode is used for test and debug only.
new RenderThreadImpl(params_, std::move(main_thread_scheduler));
}

void InProcessRendererThread::CleanUp() {
Expand Down
77 changes: 20 additions & 57 deletions content/renderer/render_thread_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "base/memory/discardable_memory_allocator.h"
#include "base/memory/memory_coordinator_client_registry.h"
#include "base/memory/shared_memory.h"
#include "base/message_loop/message_loop.h"
#include "base/metrics/field_trial.h"
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h"
Expand Down Expand Up @@ -637,28 +636,6 @@ bool RenderThreadImpl::HistogramCustomizer::IsAlexaTop10NonGoogleSite(
return false;
}

// static
RenderThreadImpl* RenderThreadImpl::Create(
const InProcessChildThreadParams& params,
base::MessageLoop* unowned_message_loop) {
TRACE_EVENT0("startup", "RenderThreadImpl::Create");
std::unique_ptr<blink::scheduler::WebThreadScheduler> main_thread_scheduler =
blink::scheduler::WebThreadScheduler::CreateMainThreadScheduler();
scoped_refptr<base::SingleThreadTaskRunner> test_task_counter;
return new RenderThreadImpl(params, std::move(main_thread_scheduler),
test_task_counter, unowned_message_loop);
}

// static
RenderThreadImpl* RenderThreadImpl::Create(
std::unique_ptr<base::MessageLoop> main_message_loop,
std::unique_ptr<blink::scheduler::WebThreadScheduler>
main_thread_scheduler) {
TRACE_EVENT0("startup", "RenderThreadImpl::Create");
return new RenderThreadImpl(std::move(main_message_loop),
std::move(main_thread_scheduler));
}

// static
RenderThreadImpl* RenderThreadImpl::current() {
return lazy_tls.Pointer()->Get();
Expand Down Expand Up @@ -703,62 +680,49 @@ RenderThreadImpl::DeprecatedGetMainTaskRunner() {
// the browser
RenderThreadImpl::RenderThreadImpl(
const InProcessChildThreadParams& params,
std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler,
const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue,
base::MessageLoop* unowned_message_loop)
: ChildThreadImpl(
Options::Builder()
.InBrowserProcess(params)
.AutoStartServiceManagerConnection(false)
.ConnectToBrowser(true)
.IPCTaskRunner(scheduler ? scheduler->IPCTaskRunner() : nullptr)
.Build()),
std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler)
: ChildThreadImpl(Options::Builder()
.InBrowserProcess(params)
.AutoStartServiceManagerConnection(false)
.ConnectToBrowser(true)
.IPCTaskRunner(scheduler->IPCTaskRunner())
.Build()),
main_thread_scheduler_(std::move(scheduler)),
main_message_loop_(unowned_message_loop),
categorized_worker_pool_(new CategorizedWorkerPool()),
renderer_binding_(this),
client_id_(1),
compositing_mode_watcher_binding_(this),
weak_factory_(this) {
Init(resource_task_queue);
TRACE_EVENT0("startup", "RenderThreadImpl::Create");
Init();
}

// When we run plugins in process, we actually run them on the render thread,
// which means that we need to make the render thread pump UI events.
// Multi-process mode.
RenderThreadImpl::RenderThreadImpl(
std::unique_ptr<base::MessageLoop> owned_message_loop,
std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler)
: ChildThreadImpl(
Options::Builder()
.AutoStartServiceManagerConnection(false)
.ConnectToBrowser(true)
.IPCTaskRunner(scheduler ? scheduler->IPCTaskRunner() : nullptr)
.Build()),
: ChildThreadImpl(Options::Builder()
.AutoStartServiceManagerConnection(false)
.ConnectToBrowser(true)
.IPCTaskRunner(scheduler->IPCTaskRunner())
.Build()),
main_thread_scheduler_(std::move(scheduler)),
owned_message_loop_(std::move(owned_message_loop)),
main_message_loop_(owned_message_loop_.get()),
categorized_worker_pool_(new CategorizedWorkerPool()),
is_scroll_animator_enabled_(false),
renderer_binding_(this),
compositing_mode_watcher_binding_(this),
weak_factory_(this) {
scoped_refptr<base::SingleThreadTaskRunner> test_task_counter;
TRACE_EVENT0("startup", "RenderThreadImpl::Create");
DCHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kRendererClientId));
base::StringToInt(base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
switches::kRendererClientId),
&client_id_);
Init(test_task_counter);
Init();
}

void RenderThreadImpl::Init(
const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue) {
void RenderThreadImpl::Init() {
TRACE_EVENT0("startup", "RenderThreadImpl::Init");

// Whether owned or unowned, |main_message_loop_| needs to be initialized in
// all constructors.
DCHECK(main_message_loop_);

GetContentClient()->renderer()->PostIOThreadCreated(GetIOTaskRunner().get());

base::trace_event::TraceLog::GetInstance()->SetThreadSortIndex(
Expand All @@ -771,7 +735,7 @@ void RenderThreadImpl::Init(
#endif

lazy_tls.Pointer()->Set(this);
g_main_task_runner.Get() = main_message_loop_->task_runner();
g_main_task_runner.Get() = base::ThreadTaskRunnerHandle::Get();

// Register this object as the main thread.
ChildProcess::current()->set_main_thread(this);
Expand All @@ -792,7 +756,7 @@ void RenderThreadImpl::Init(
URLLoaderThrottleProviderType::kFrame);

auto registry = std::make_unique<service_manager::BinderRegistry>();
InitializeWebKit(resource_task_queue, registry.get());
InitializeWebKit(registry.get());

// In single process the single process is all there is.
webkit_shared_timer_suspended_ = false;
Expand Down Expand Up @@ -1230,7 +1194,6 @@ void RenderThreadImpl::InitializeCompositorThread() {
}

void RenderThreadImpl::InitializeWebKit(
const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue,
service_manager::BinderRegistry* registry) {
DCHECK(!blink_platform_impl_);

Expand Down
39 changes: 7 additions & 32 deletions content/renderer/render_thread_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,6 @@ class CONTENT_EXPORT RenderThreadImpl
public viz::mojom::CompositingModeWatcher,
public CompositorDependencies {
public:
static RenderThreadImpl* Create(const InProcessChildThreadParams& params,
base::MessageLoop* unowned_message_loop);
static RenderThreadImpl* Create(
std::unique_ptr<base::MessageLoop> main_message_loop,
std::unique_ptr<blink::scheduler::WebThreadScheduler>
main_thread_scheduler);
static RenderThreadImpl* current();
static mojom::RenderMessageFilter* current_render_message_filter();
static RendererBlinkPlatformImpl* current_blink_platform_impl();
Expand All @@ -189,6 +183,11 @@ class CONTENT_EXPORT RenderThreadImpl
static scoped_refptr<base::SingleThreadTaskRunner>
DeprecatedGetMainTaskRunner();

explicit RenderThreadImpl(
std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler);
RenderThreadImpl(
const InProcessChildThreadParams& params,
std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler);
~RenderThreadImpl() override;
void Shutdown() override;
bool ShouldBeDestroyed() override;
Expand Down Expand Up @@ -515,16 +514,6 @@ class CONTENT_EXPORT RenderThreadImpl
// Sets the current pipeline rendering color space.
void SetRenderingColorSpace(const gfx::ColorSpace& color_space);

protected:
RenderThreadImpl(
const InProcessChildThreadParams& params,
std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler,
const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue,
base::MessageLoop* unowned_message_loop);
RenderThreadImpl(
std::unique_ptr<base::MessageLoop> main_message_loop,
std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler);

private:
void OnProcessFinalRelease() override;
// IPC::Listener
Expand All @@ -543,14 +532,9 @@ class CONTENT_EXPORT RenderThreadImpl

void RecordPurgeMemory(RendererMemoryMetrics before);

void Init(
const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue);

void Init();
void InitializeCompositorThread();

void InitializeWebKit(
const scoped_refptr<base::SingleThreadTaskRunner>& resource_task_queue,
service_manager::BinderRegistry* registry);
void InitializeWebKit(service_manager::BinderRegistry* registry);

void OnTransferBitmap(const SkBitmap& bitmap, int resource_id);
void OnGetAccessibilityTree();
Expand Down Expand Up @@ -683,15 +667,6 @@ class CONTENT_EXPORT RenderThreadImpl
// software-based.
bool is_gpu_compositing_disabled_ = false;

// The message loop of the renderer main thread.
// This message loop should be destructed before the RenderThreadImpl
// shuts down Blink.
// Some test users (e.g. InProcessRenderThread) own the MessageLoop used by
// their RenderThreadImpls. |main_message_loop_| is always non-nulll,
// |owned_message_loop_| is non-null if handed in at creation.
const std::unique_ptr<base::MessageLoop> owned_message_loop_;
base::MessageLoop* const main_message_loop_;

// May be null if overridden by ContentRendererClient.
std::unique_ptr<blink::scheduler::WebThreadBase> compositor_thread_;

Expand Down
36 changes: 7 additions & 29 deletions content/renderer/render_thread_impl_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,31 +122,6 @@ class TestTaskCounter : public base::SingleThreadTaskRunner {
int count_;
};

#if defined(COMPILER_MSVC)
// See explanation for other RenderViewHostImpl which is the same issue.
#pragma warning(push)
#pragma warning(disable: 4250)
#endif

class RenderThreadImplForTest : public RenderThreadImpl {
public:
RenderThreadImplForTest(
const InProcessChildThreadParams& params,
std::unique_ptr<blink::scheduler::WebThreadScheduler> scheduler,
scoped_refptr<base::SingleThreadTaskRunner>& test_task_counter,
base::MessageLoop* unowned_message_loop)
: RenderThreadImpl(params,
std::move(scheduler),
test_task_counter,
unowned_message_loop) {}

~RenderThreadImplForTest() override {}
};

#if defined(COMPILER_MSVC)
#pragma warning(pop)
#endif

class QuitOnTestMsgFilter : public IPC::MessageFilter {
public:
explicit QuitOnTestMsgFilter(base::OnceClosure quit_closure)
Expand Down Expand Up @@ -256,11 +231,10 @@ class RenderThreadImplBrowserTest : public testing::Test {

base::FieldTrialList::CreateTrialsFromCommandLine(
*cmd, switches::kFieldTrialHandle, -1);
thread_ = new RenderThreadImplForTest(
thread_ = new RenderThreadImpl(
InProcessChildThreadParams(io_task_runner, &invitation,
child_connection_->service_token()),
std::move(main_thread_scheduler), test_task_counter,
main_message_loop_.get());
std::move(main_thread_scheduler));
cmd->InitFromArgv(old_argv);

run_loop_ = std::make_unique<base::RunLoop>();
Expand Down Expand Up @@ -297,7 +271,11 @@ class RenderThreadImplBrowserTest : public testing::Test {

std::unique_ptr<MockRenderProcess> mock_process_;
scoped_refptr<QuitOnTestMsgFilter> test_msg_filter_;
RenderThreadImplForTest* thread_; // Owned by mock_process_.

// RenderThreadImpl doesn't currently support a proper shutdown sequence
// and it's okay when we're running in multi-process mode because renderers
// get killed by the OS. Memory leaks aren't nice but it's test-only.
RenderThreadImpl* thread_;

base::FieldTrialList field_trial_list_;

Expand Down
7 changes: 4 additions & 3 deletions content/renderer/renderer_main.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,10 @@ int RendererMain(const MainFunctionParams& parameters) {
}
#endif

auto render_process = RenderProcessImpl::Create();
RenderThreadImpl::Create(std::move(main_message_loop),
std::move(main_thread_scheduler));
std::unique_ptr<RenderProcess> render_process = RenderProcessImpl::Create();
// It's not a memory leak since RenderThread has the same lifetime
// as a renderer process.
new RenderThreadImpl(std::move(main_thread_scheduler));

if (need_sandbox)
run_loop = platform.EnableSandbox();
Expand Down

0 comments on commit f5d6027

Please sign in to comment.