cc: Split ThreadProxy into ProxyMain and ProxyImpl

cc/trees/threaded_channel.cc

Index: cc/trees/threaded_channel.cc
diff --git a/cc/trees/threaded_channel.cc b/cc/trees/threaded_channel.cc
index a2cb7831b61238dc0c42631ef4bde7b1e4691c30..6866f15d44da32769cb166669e4ee0d8e91b2c31 100644
--- a/cc/trees/threaded_channel.cc
+++ b/cc/trees/threaded_channel.cc
@@ -7,26 +7,37 @@
 #include "base/bind.h"
 #include "base/single_thread_task_runner.h"
 #include "base/trace_event/trace_event.h"
+#include "cc/trees/layer_tree_host.h"
 
 namespace cc {
 
+ThreadedChannel::MainThreadOnly::MainThreadOnly(ProxyMain* proxy_main)
+    : proxy_main_weak_factory(proxy_main), initialized(false) {}
+
+ThreadedChannel::MainThreadOnly::~MainThreadOnly() {}
+
+ThreadedChannel::CompositorThreadOnly::CompositorThreadOnly() {}
+
+ThreadedChannel::CompositorThreadOnly::~CompositorThreadOnly() {}
+
 scoped_ptr<ThreadedChannel> ThreadedChannel::Create(
-    ThreadProxy* thread_proxy,
+    ProxyMain* proxy_main,
     TaskRunnerProvider* task_runner_provider) {
-  return make_scoped_ptr(
-      new ThreadedChannel(thread_proxy, task_runner_provider));
+  return make_scoped_ptr(new ThreadedChannel(proxy_main, task_runner_provider));
 }
 
-ThreadedChannel::ThreadedChannel(ThreadProxy* thread_proxy,
+ThreadedChannel::ThreadedChannel(ProxyMain* proxy_main,
                                  TaskRunnerProvider* task_runner_provider)
-    : proxy_main_(thread_proxy),
-      proxy_impl_(thread_proxy),
-      task_runner_provider_(task_runner_provider) {}
+    : main_thread_only_vars_unsafe_(proxy_main),
+      task_runner_provider_(task_runner_provider) {
+  DCHECK(task_runner_provider_);
+  DCHECK(task_runner_provider_->IsMainThread());
+}
 
 void ThreadedChannel::SetThrottleFrameProductionOnImpl(bool throttle) {
   ImplThreadTaskRunner()->PostTask(
       FROM_HERE, base::Bind(&ProxyImpl::SetThrottleFrameProductionOnImpl,
-                            proxy_impl_->GetImplWeakPtr(), throttle));
+                            proxy_impl_weak_ptr_, throttle));
 }
 
 void ThreadedChannel::UpdateTopControlsStateOnImpl(TopControlsState constraints,
@@ -34,60 +45,60 @@ void ThreadedChannel::UpdateTopControlsStateOnImpl(TopControlsState constraints,
                                                    bool animate) {
   ImplThreadTaskRunner()->PostTask(
       FROM_HERE,
-      base::Bind(&ProxyImpl::UpdateTopControlsStateOnImpl,
-                 proxy_impl_->GetImplWeakPtr(), constraints, current, animate));
+      base::Bind(&ProxyImpl::UpdateTopControlsStateOnImpl, proxy_impl_weak_ptr_,
+                 constraints, current, animate));
 }
 
 void ThreadedChannel::InitializeOutputSurfaceOnImpl(
     OutputSurface* output_surface) {
   ImplThreadTaskRunner()->PostTask(
       FROM_HERE, base::Bind(&ProxyImpl::InitializeOutputSurfaceOnImpl,
-                            proxy_impl_->GetImplWeakPtr(), output_surface));
+                            proxy_impl_weak_ptr_, output_surface));
 }
 
 void ThreadedChannel::MainThreadHasStoppedFlingingOnImpl() {
   ImplThreadTaskRunner()->PostTask(
       FROM_HERE, base::Bind(&ProxyImpl::MainThreadHasStoppedFlingingOnImpl,
-                            proxy_impl_->GetImplWeakPtr()));
+                            proxy_impl_weak_ptr_));
 }
 
 void ThreadedChannel::SetInputThrottledUntilCommitOnImpl(bool is_throttled) {
   ImplThreadTaskRunner()->PostTask(
       FROM_HERE, base::Bind(&ProxyImpl::SetInputThrottledUntilCommitOnImpl,
-                            proxy_impl_->GetImplWeakPtr(), is_throttled));
+                            proxy_impl_weak_ptr_, is_throttled));
 }
 
 void ThreadedChannel::SetDeferCommitsOnImpl(bool defer_commits) {
   ImplThreadTaskRunner()->PostTask(
       FROM_HERE, base::Bind(&ProxyImpl::SetDeferCommitsOnImpl,
-                            proxy_impl_->GetImplWeakPtr(), defer_commits));
+                            proxy_impl_weak_ptr_, defer_commits));
 }
 
 void ThreadedChannel::FinishAllRenderingOnImpl(CompletionEvent* completion) {
   ImplThreadTaskRunner()->PostTask(
       FROM_HERE, base::Bind(&ProxyImpl::FinishAllRenderingOnImpl,
-                            proxy_impl_->GetImplWeakPtr(), completion));
+                            proxy_impl_weak_ptr_, completion));
 }
 
 void ThreadedChannel::SetNeedsCommitOnImpl() {
-  ImplThreadTaskRunner()->PostTask(FROM_HERE,
-                                   base::Bind(&ProxyImpl::SetNeedsCommitOnImpl,
-                                              proxy_impl_->GetImplWeakPtr()));
+  ImplThreadTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(&ProxyImpl::SetNeedsCommitOnImpl, proxy_impl_weak_ptr_));
 }
 
 void ThreadedChannel::BeginMainFrameAbortedOnImpl(
     CommitEarlyOutReason reason,
     base::TimeTicks main_thread_start_time) {
   ImplThreadTaskRunner()->PostTask(
-      FROM_HERE, base::Bind(&ProxyImpl::BeginMainFrameAbortedOnImpl,
-                            proxy_impl_->GetImplWeakPtr(), reason,
-                            main_thread_start_time));
+      FROM_HERE,
+      base::Bind(&ProxyImpl::BeginMainFrameAbortedOnImpl, proxy_impl_weak_ptr_,
+                 reason, main_thread_start_time));
 }
 
 void ThreadedChannel::SetNeedsRedrawOnImpl(const gfx::Rect& damage_rect) {
   ImplThreadTaskRunner()->PostTask(
       FROM_HERE, base::Bind(&ProxyImpl::SetNeedsRedrawOnImpl,
-                            proxy_impl_->GetImplWeakPtr(), damage_rect));
+                            proxy_impl_weak_ptr_, damage_rect));
 }
 
 void ThreadedChannel::StartCommitOnImpl(CompletionEvent* completion,
@@ -95,134 +106,199 @@ void ThreadedChannel::StartCommitOnImpl(CompletionEvent* completion,
                                         base::TimeTicks main_thread_start_time,
                                         bool hold_commit_for_activation) {
   ImplThreadTaskRunner()->PostTask(
-      FROM_HERE,
-      base::Bind(&ProxyImpl::StartCommitOnImpl, proxy_impl_->GetImplWeakPtr(),
-                 completion, layer_tree_host, main_thread_start_time,
-                 hold_commit_for_activation));
-}
-
-void ThreadedChannel::InitializeImplOnImpl(CompletionEvent* completion,
-                                           LayerTreeHost* layer_tree_host) {
-  ImplThreadTaskRunner()->PostTask(
-      FROM_HERE,
-      base::Bind(&ProxyImpl::InitializeImplOnImpl,
-                 base::Unretained(proxy_impl_), completion, layer_tree_host));
-}
-
-void ThreadedChannel::LayerTreeHostClosedOnImpl(CompletionEvent* completion) {
-  ImplThreadTaskRunner()->PostTask(
-      FROM_HERE, base::Bind(&ProxyImpl::LayerTreeHostClosedOnImpl,
-                            proxy_impl_->GetImplWeakPtr(), completion));
-  proxy_impl_ = nullptr;
+      FROM_HERE, base::Bind(&ProxyImpl::StartCommitOnImpl, proxy_impl_weak_ptr_,
+                            completion, layer_tree_host, main_thread_start_time,
+                            hold_commit_for_activation));
+}
+
+void ThreadedChannel::SynchronouslyInitializeImpl(

[Wez, 2015/11/21 01:00:30]

Why does this need to be Synchronous? Does the Impl-thread init require calling
back into Main-thread-owned objects...?

[Khushal, 2015/11/23 20:07:20]

The initialization uses the LayerTreeHost which is a main-thread owned object.
One reason I can see for having it blocking is that the input_handler is set
during initialization here:
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/layer_tre...
And the embedder can try to grab it immediately after creating the
LayerTreeHost. There may be other reasons that I don't know.

I think it would be better if we let this patch only refactor the plumbing of
calls between the thread boundary. Changing any calls from synchronous to
asynchronous should be addressed in separate patches so we can carefully review
any assumptions that different components in the system might have made about
that call being synchronous.

+    LayerTreeHost* layer_tree_host,
+    scoped_ptr<BeginFrameSource> external_begin_frame_source) {
+  TRACE_EVENT0("cc", "ThreadChannel::SynchronouslyInitializeImpl");
+  {
+    DebugScopedSetMainThreadBlocked main_thread_blocked(task_runner_provider_);
+    CompletionEvent completion;
+    ImplThreadTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&ThreadedChannel::InitializeImplOnImplThread,
+                   base::Unretained(this), &completion, layer_tree_host,
+                   base::Passed(&external_begin_frame_source)));
+    completion.Wait();
+  }
+  proxy_main_weak_ptr_ = main().proxy_main_weak_factory.GetWeakPtr();
+  main().initialized = true;
+}
+
+void ThreadedChannel::InitializeImplOnImplThread(
+    CompletionEvent* completion,
+    LayerTreeHost* layer_tree_host,
+    scoped_ptr<BeginFrameSource> external_begin_frame_source) {
+  DCHECK(task_runner_provider_->IsImplThread());
+  impl().proxy_impl =
+      CreateProxyImpl(this, layer_tree_host, task_runner_provider_,
+                      external_begin_frame_source.Pass());

[Wez, 2015/11/21 01:00:30]

AFAICT the only reason that this needs to run on the Impl thread is that the
ProxyImpl constructor calls into the supplied LayerTreeHost, which is
Impl-thread-only?  But I see you're doing that with the Main thread blocked - so
are those calls actually ones that would make sense to do on the Main thread? 

The model I was originally proposing was to have ThreadedChannel create an
object (possibly ProxyImpl) on the Main thread, and then for all subsequent
actions, including initialization, to take place on the Impl thread. That way
you can create the object, create a WeakPtrFactory to wrap it and take a WeakPtr
directly in the Main-thread init function, then just PostTask the ProxyImpl &
WeakPtrFactory to the Impl thread to be torn down.  That way the ThreadedChannel
gets to operate exclusively on the Main thread.

[Khushal, 2015/11/23 20:07:20]

The LayerTreeHost is main-thread-only and should be accessed on the impl thread
only when the main thread is blocked.
Its okay to have ThreadedChannel operate on both threads. I think it would be
cleaner to have a single class implement thread-hopping for both the sides.
Splitting it will not change the synchronous nature of any calls between the
threads.

+  impl().proxy_impl_weak_factory = make_scoped_ptr(
+      new base::WeakPtrFactory<ProxyImpl>(impl().proxy_impl.get()));
+  proxy_impl_weak_ptr_ = impl().proxy_impl_weak_factory->GetWeakPtr();
+  completion->Signal();
+}
+
+scoped_ptr<ProxyImpl> ThreadedChannel::CreateProxyImpl(
+    ChannelImpl* channel_impl,
+    LayerTreeHost* layer_tree_host,
+    TaskRunnerProvider* task_runner_provider,
+    scoped_ptr<BeginFrameSource> external_begin_frame_source) {
+  return ProxyImpl::Create(channel_impl, layer_tree_host, task_runner_provider,
+                           external_begin_frame_source.Pass());
+}
+
+void ThreadedChannel::SynchronouslyCloseImpl() {
+  TRACE_EVENT0("cc", "ThreadChannel::~SynchronouslyCloseImpl");
+
+  // Synchronously finishes pending GL operations and deletes the impl.
+  // The two steps are done as separate post tasks, so that tasks posted
+  // by the GL implementation due to the Finish can be executed by the
+  // renderer before shutting it down.
+  {
+    DebugScopedSetMainThreadBlocked main_thread_blocked(task_runner_provider_);
+    CompletionEvent completion;
+    ImplThreadTaskRunner()->PostTask(
+        FROM_HERE, base::Bind(&ProxyImpl::FinishGLOnImpl, proxy_impl_weak_ptr_,
+                              &completion));
+    completion.Wait();
+  }
+  {
+    DebugScopedSetMainThreadBlocked main_thread_blocked(task_runner_provider_);
+    CompletionEvent completion;
+    ImplThreadTaskRunner()->PostTask(
+        FROM_HERE, base::Bind(&ThreadedChannel::CloseImplOnImplThread,
+                              base::Unretained(this), &completion));
+    completion.Wait();
+  }
+  main().proxy_main_weak_factory.InvalidateWeakPtrs();
+  main().initialized = false;
+}
+
+void ThreadedChannel::CloseImplOnImplThread(CompletionEvent* completion) {
+  DCHECK(task_runner_provider_->IsImplThread());
+
+  // We must invalidate the proxy_impl_weak_ptrs and destroy the factory before
+  // destroying proxy_impl.
+  impl().proxy_impl_weak_factory->InvalidateWeakPtrs();
+  impl().proxy_impl_weak_factory.reset();
+
+  impl().proxy_impl.reset();
+  completion->Signal();
 }
 
 void ThreadedChannel::SetVisibleOnImpl(bool visible) {
   ImplThreadTaskRunner()->PostTask(
-      FROM_HERE, base::Bind(&ProxyImpl::SetVisibleOnImpl,
-                            proxy_impl_->GetImplWeakPtr(), visible));
+      FROM_HERE,
+      base::Bind(&ProxyImpl::SetVisibleOnImpl, proxy_impl_weak_ptr_, visible));
 }
 
 void ThreadedChannel::ReleaseOutputSurfaceOnImpl(CompletionEvent* completion) {
   ImplThreadTaskRunner()->PostTask(
       FROM_HERE, base::Bind(&ProxyImpl::ReleaseOutputSurfaceOnImpl,
-                            proxy_impl_->GetImplWeakPtr(), completion));
-}
-
-void ThreadedChannel::FinishGLOnImpl(CompletionEvent* completion) {
-  ImplThreadTaskRunner()->PostTask(
-      FROM_HERE, base::Bind(&ProxyImpl::FinishGLOnImpl,
-                            proxy_impl_->GetImplWeakPtr(), completion));
+                            proxy_impl_weak_ptr_, completion));
 }
 
 void ThreadedChannel::MainFrameWillHappenOnImplForTesting(
     CompletionEvent* completion,
     bool* main_frame_will_happen) {
   ImplThreadTaskRunner()->PostTask(
-      FROM_HERE, base::Bind(&ProxyImpl::MainFrameWillHappenOnImplForTesting,
-                            proxy_impl_->GetImplWeakPtr(), completion,
-                            main_frame_will_happen));
+      FROM_HERE,
+      base::Bind(&ProxyImpl::MainFrameWillHappenOnImplForTesting,
+                 proxy_impl_weak_ptr_, completion, main_frame_will_happen));
 }
 
 void ThreadedChannel::DidCompleteSwapBuffers() {
   MainThreadTaskRunner()->PostTask(
-      FROM_HERE, base::Bind(&ProxyMain::DidCompleteSwapBuffers,
-                            proxy_main_->GetMainWeakPtr()));
+      FROM_HERE,
+      base::Bind(&ProxyMain::DidCompleteSwapBuffers, proxy_main_weak_ptr_));
 }
 
 void ThreadedChannel::SetRendererCapabilitiesMainCopy(
     const RendererCapabilities& capabilities) {
   MainThreadTaskRunner()->PostTask(
       FROM_HERE, base::Bind(&ProxyMain::SetRendererCapabilitiesMainCopy,
-                            proxy_main_->GetMainWeakPtr(), capabilities));
+                            proxy_main_weak_ptr_, capabilities));
 }
 
 void ThreadedChannel::BeginMainFrameNotExpectedSoon() {
   MainThreadTaskRunner()->PostTask(
       FROM_HERE, base::Bind(&ProxyMain::BeginMainFrameNotExpectedSoon,
-                            proxy_main_->GetMainWeakPtr()));
+                            proxy_main_weak_ptr_));
 }
 
 void ThreadedChannel::DidCommitAndDrawFrame() {
-  MainThreadTaskRunner()->PostTask(FROM_HERE,
-                                   base::Bind(&ProxyMain::DidCommitAndDrawFrame,
-                                              proxy_main_->GetMainWeakPtr()));
+  MainThreadTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(&ProxyMain::DidCommitAndDrawFrame, proxy_main_weak_ptr_));
 }
 
 void ThreadedChannel::SetAnimationEvents(
     scoped_ptr<AnimationEventsVector> queue) {
   MainThreadTaskRunner()->PostTask(
-      FROM_HERE,
-      base::Bind(&ProxyMain::SetAnimationEvents, proxy_main_->GetMainWeakPtr(),
-                 base::Passed(&queue)));
+      FROM_HERE, base::Bind(&ProxyMain::SetAnimationEvents,
+                            proxy_main_weak_ptr_, base::Passed(&queue)));
 }
 
 void ThreadedChannel::DidLoseOutputSurface() {
-  MainThreadTaskRunner()->PostTask(FROM_HERE,
-                                   base::Bind(&ProxyMain::DidLoseOutputSurface,
-                                              proxy_main_->GetMainWeakPtr()));
+  MainThreadTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(&ProxyMain::DidLoseOutputSurface, proxy_main_weak_ptr_));
 }
 
 void ThreadedChannel::RequestNewOutputSurface() {
   MainThreadTaskRunner()->PostTask(
-      FROM_HERE, base::Bind(&ProxyMain::RequestNewOutputSurface,
-                            proxy_main_->GetMainWeakPtr()));
+      FROM_HERE,
+      base::Bind(&ProxyMain::RequestNewOutputSurface, proxy_main_weak_ptr_));
 }
 
 void ThreadedChannel::DidInitializeOutputSurface(
     bool success,
     const RendererCapabilities& capabilities) {
   MainThreadTaskRunner()->PostTask(
-      FROM_HERE,
-      base::Bind(&ProxyMain::DidInitializeOutputSurface,
-                 proxy_main_->GetMainWeakPtr(), success, capabilities));
+      FROM_HERE, base::Bind(&ProxyMain::DidInitializeOutputSurface,
+                            proxy_main_weak_ptr_, success, capabilities));
 }
 
 void ThreadedChannel::DidCompletePageScaleAnimation() {
   MainThreadTaskRunner()->PostTask(
       FROM_HERE, base::Bind(&ProxyMain::DidCompletePageScaleAnimation,
-                            proxy_main_->GetMainWeakPtr()));
+                            proxy_main_weak_ptr_));
 }
 
 void ThreadedChannel::PostFrameTimingEventsOnMain(
     scoped_ptr<FrameTimingTracker::CompositeTimingSet> composite_events,
     scoped_ptr<FrameTimingTracker::MainFrameTimingSet> main_frame_events) {
   MainThreadTaskRunner()->PostTask(
-      FROM_HERE, base::Bind(&ProxyMain::PostFrameTimingEventsOnMain,
-                            proxy_main_->GetMainWeakPtr(),
-                            base::Passed(composite_events.Pass()),
-                            base::Passed(main_frame_events.Pass())));
+      FROM_HERE,
+      base::Bind(&ProxyMain::PostFrameTimingEventsOnMain, proxy_main_weak_ptr_,
+                 base::Passed(composite_events.Pass()),
+                 base::Passed(main_frame_events.Pass())));
 }
 
 void ThreadedChannel::BeginMainFrame(
     scoped_ptr<BeginMainFrameAndCommitState> begin_main_frame_state) {
   MainThreadTaskRunner()->PostTask(
-      FROM_HERE,
-      base::Bind(&ProxyMain::BeginMainFrame, proxy_main_->GetMainWeakPtr(),
-                 base::Passed(&begin_main_frame_state)));
+      FROM_HERE, base::Bind(&ProxyMain::BeginMainFrame, proxy_main_weak_ptr_,
+                            base::Passed(&begin_main_frame_state)));
 }
 
 ThreadedChannel::~ThreadedChannel() {
   TRACE_EVENT0("cc", "ThreadChannel::~ThreadChannel");
+  DCHECK(task_runner_provider_->IsMainThread());
+  DCHECK(!IsInitialized());
+}
+
+bool ThreadedChannel::IsInitialized() const {
+  return main().initialized;
+}
+
+ProxyImpl* ThreadedChannel::GetProxyImplForTesting() const {
+  return impl().proxy_impl.get();
 }
 
 base::SingleThreadTaskRunner* ThreadedChannel::MainThreadTaskRunner() const {
@@ -233,4 +309,24 @@ base::SingleThreadTaskRunner* ThreadedChannel::ImplThreadTaskRunner() const {
   return task_runner_provider_->ImplThreadTaskRunner();
 }
 
+ThreadedChannel::MainThreadOnly& ThreadedChannel::main() {
+  DCHECK(task_runner_provider_->IsMainThread());
+  return main_thread_only_vars_unsafe_;
+}
+
+const ThreadedChannel::MainThreadOnly& ThreadedChannel::main() const {
+  DCHECK(task_runner_provider_->IsMainThread());
+  return main_thread_only_vars_unsafe_;
+}
+
+ThreadedChannel::CompositorThreadOnly& ThreadedChannel::impl() {
+  DCHECK(task_runner_provider_->IsImplThread());
+  return compositor_thread_vars_unsafe_;
+}
+
+const ThreadedChannel::CompositorThreadOnly& ThreadedChannel::impl() const {
+  DCHECK(task_runner_provider_->IsImplThread());
+  return compositor_thread_vars_unsafe_;
+}
+
 }  // namespace cc