cc: Split ThreadProxy into ProxyMain and ProxyImpl

cc/trees/threaded_channel.h

Index: cc/trees/threaded_channel.h
diff --git a/cc/trees/threaded_channel.h b/cc/trees/threaded_channel.h
index ca7fc04c0a7319f5aa6ce7bfe0bc2ca4b0c102e1..0f61e901028e0fde450efad507fd9dbc0852188b 100644
--- a/cc/trees/threaded_channel.h
+++ b/cc/trees/threaded_channel.h
@@ -7,13 +7,13 @@
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "cc/base/cc_export.h"
 #include "cc/trees/channel_impl.h"
 #include "cc/trees/channel_main.h"
 #include "cc/trees/proxy_common.h"
 #include "cc/trees/proxy_impl.h"
 #include "cc/trees/proxy_main.h"
-#include "cc/trees/thread_proxy.h"
 
 namespace base {
 class SingleThreadTaskRunner;
@@ -22,9 +22,9 @@ class SingleThreadTaskRunner;
 namespace cc {
 class ChannelImpl;
 class ChannelMain;
+class LayerTreeHost;
 class ProxyImpl;
 class ProxyMain;
-class ThreadProxy;
 
 // An implementation of ChannelMain and ChannelImpl that sends commands between
 // ProxyMain and ProxyImpl across thread boundaries.
@@ -66,14 +66,29 @@ class ThreadProxy;
 class CC_EXPORT ThreadedChannel : public ChannelMain, public ChannelImpl {
  public:
   static scoped_ptr<ThreadedChannel> Create(
-      // TODO(khushalsagar): Make this ProxyMain* and write the initialization
-      // sequence. Currently ThreadProxy implements both so we pass the pointer
-      // and set ProxyImpl.
-      ThreadProxy* thread_proxy,
+      ProxyMain* proxy_main,
       TaskRunnerProvider* task_runner_provider);
 
   ~ThreadedChannel() override;
 
+  // The members of this struct should be accessed on the main thread only.
+  struct MainThreadOnly {
+    base::WeakPtrFactory<ProxyMain> proxy_main_weak_factory;
+    bool initialized;
+
+    explicit MainThreadOnly(ProxyMain* proxy_main);
+    ~MainThreadOnly();
+  };
+
+  // The members of this struct should be accessed on the impl thread only.
+  struct CompositorThreadOnly {
+    scoped_ptr<ProxyImpl> proxy_impl;
+    scoped_ptr<base::WeakPtrFactory<ProxyImpl>> proxy_impl_weak_factory;
+
+    CompositorThreadOnly();
+    ~CompositorThreadOnly();
+  };
+
   // ChannelMain Implementation
   void SetThrottleFrameProductionOnImpl(bool throttle) override;
   void UpdateTopControlsStateOnImpl(TopControlsState constraints,
@@ -93,7 +108,6 @@ class CC_EXPORT ThreadedChannel : public ChannelMain, public ChannelImpl {
   // Blocking calls to ProxyImpl
   void FinishAllRenderingOnImpl(CompletionEvent* completion) override;
   void ReleaseOutputSurfaceOnImpl(CompletionEvent* completion) override;
-  void FinishGLOnImpl(CompletionEvent* completion) override;
   void MainFrameWillHappenOnImplForTesting(
       CompletionEvent* completion,
       bool* main_frame_will_happen) override;
@@ -101,9 +115,10 @@ class CC_EXPORT ThreadedChannel : public ChannelMain, public ChannelImpl {
                          LayerTreeHost* layer_tree_host,
                          base::TimeTicks main_thread_start_time,
                          bool hold_commit_for_activation) override;
-  void InitializeImplOnImpl(CompletionEvent* completion,
-                            LayerTreeHost* layer_tree_host) override;
-  void LayerTreeHostClosedOnImpl(CompletionEvent* completion) override;
+  void SynchronouslyInitializeImpl(
+      LayerTreeHost* layer_tree_host,
+      scoped_ptr<BeginFrameSource> external_begin_frame_source) override;
+  void SynchronouslyCloseImpl() override;
 
   // ChannelImpl Implementation
   void DidCompleteSwapBuffers() override;
@@ -125,23 +140,48 @@ class CC_EXPORT ThreadedChannel : public ChannelMain, public ChannelImpl {
   void BeginMainFrame(
       scoped_ptr<BeginMainFrameAndCommitState> begin_main_frame_state) override;
 
+  // Should be called on impl thread only.
+  ProxyImpl* GetProxyImplForTesting() const;
+
  protected:
-  ThreadedChannel(ThreadProxy* thread_proxy,
+  ThreadedChannel(ProxyMain* proxy_main,
                   TaskRunnerProvider* task_runner_provider);
 
+  // virtual for testing.
+  virtual scoped_ptr<ProxyImpl> CreateProxyImpl(
+      ChannelImpl* channel_impl,
+      LayerTreeHost* layer_tree_host,
+      TaskRunnerProvider* task_runner_provider,
+      scoped_ptr<BeginFrameSource> external_begin_frame_source);
+
  private:
+  // called on impl thread.
+  void InitializeImplOnImplThread(

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

I thought we had agreed to break ThreadedChannel into the external
ThreadedChannel that runs exclusively on the Main thread, and an internal
sub-component that runs on the Impl thread?

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

For this patch, It would make sense to keep the split as-is. It makes it easy to
follow that we have pulled out the parts from ThreadProxy that separate the
thread boundary into the ThreadedChannel.

The idea of ThreadProxy was to separate the main and impl side and have
ThreadProxy as the class that exists on both threads and glues them together.
Now we've pushed the logic that was exclusive to each thread into ProxyMain and
ProxyImpl and have ThreadedChannel as the class that exists on both threads and
deals exclusively with thread hopping. I think the current split makes it easier
to follow the aim of this refactoring and would keep it readable. If there is a
need to split ThreadedChannel further, how about we address that in a subsequent
patch?

+      CompletionEvent* completion,
+      LayerTreeHost* layer_tree_host,
+      scoped_ptr<BeginFrameSource> external_begin_frame_source);
+  void CloseImplOnImplThread(CompletionEvent* completion);
+
+  bool IsInitialized() const;
+
   base::SingleThreadTaskRunner* MainThreadTaskRunner() const;
   base::SingleThreadTaskRunner* ImplThreadTaskRunner() const;
 
-  ProxyMain* proxy_main_;
+  // Use accessors instead of this variable directly.
+  MainThreadOnly main_thread_only_vars_unsafe_;
+  MainThreadOnly& main();
+  const MainThreadOnly& main() const;
 
-  ProxyImpl* proxy_impl_;
+  // Use accessors instead of this variable directly.
+  CompositorThreadOnly compositor_thread_vars_unsafe_;
+  CompositorThreadOnly& impl();
+  const CompositorThreadOnly& impl() const;
 
   TaskRunnerProvider* task_runner_provider_;
 
-  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+  base::WeakPtr<ProxyMain> proxy_main_weak_ptr_;
 
-  scoped_refptr<base::SingleThreadTaskRunner> impl_task_runner_;
+  base::WeakPtr<ProxyImpl> proxy_impl_weak_ptr_;
 
   DISALLOW_COPY_AND_ASSIGN(ThreadedChannel);
 };