cc: Split ThreadProxy into ProxyMain and ProxyImpl

cc/trees/threaded_channel.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CC_TREES_THREADED_CHANNEL_H_
 #define CC_TREES_THREADED_CHANNEL_H_
 
 #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;
 }
 
 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.
 //
 // LayerTreeHost creates ThreadedChannel and passes the ownership to ProxyMain.
 // The object life cycle and communication across threads is as follows:
 //
 //
 //           Main Thread              |               Impl Thread
 //   LayerTreeHost->InitializeProxy   |
 //               |                    |
 //        ProxyMain->Start()          |
 //               |              ThreadedChannel
 // ---------------------------------------------------------------------------
-//  ChannelMain::InitializeImpl ---PostTask---> ThreadedChannel::
-//                                                   InitializeImplOnImplThread
+//  ChannelMain::SynchronouslyInitializeImpl ---PostTask---> ThreadedChannel::
+//                                                    InitializeImplOnImpl
 //                                                          |
 //                                                   ProxyImpl::Create
 //                                                          |
 //                                                   ProxyImpl->Initialize()
 //                                                          .
 //                                                          .
 //                                          ProxyImpl::ScheduledActionBegin
 //                                                     OutputSurfaceCreation
 //                                                          |
 //                                         ChannelImpl::RequestNewOutputSurface
 // ----------------------------------------------------------------------------
 //                                                          |
 // ProxyMain->RequestNewOutputSurface()<----PostTask--------
 //              .
 //              .
-// ProxyMain->LayerTreeHostClosed
+// ProxyMain->Stop()
 //              |
 // ---------------------------------------------------------------------------
-// ChannelMain::SetLayerTreeClosedOnImpl---PostTask---> ProxyImpl->
-//                                                        SetLayerTreeClosed
+// ChannelMain::SynchronouslyCloseImpl ---PostTask---> ThreadedChannel::
+//                                                      CloseImplOnImpl
 // ----------------------------------------------------------------------------
+//
+// ThreadedChannel is created and destroyed on the main thread but can be
+// called from main or impl thread. It is safe for the Threadedchannel to be
+// called on the impl thread because:
+// 1) The only impl-threaded callers of ThreadedChannel are the ThreadedChannel
+// itself and ProxyImpl which is created and owned by the ThreadedChannel.
+// 2) ThreadedChannel blocks the main thread in
+// ThreadedChannel::SynchronouslyCloseImpl to wait for the impl-thread teardown
+// to complete, so there is no risk of any queued tasks calling it on the impl
+// thread after it has been deleted on the main thread.
 
 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;
 
   // ChannelMain Implementation
   void SetThrottleFrameProductionOnImpl(bool throttle) override;
   void UpdateTopControlsStateOnImpl(TopControlsState constraints,
                                     TopControlsState current,
                                     bool animate) override;
   void InitializeOutputSurfaceOnImpl(OutputSurface* output_surface) override;
   void MainThreadHasStoppedFlingingOnImpl() override;
   void SetInputThrottledUntilCommitOnImpl(bool is_throttled) override;
   void SetDeferCommitsOnImpl(bool defer_commits) override;
   void SetNeedsCommitOnImpl() override;
   void BeginMainFrameAbortedOnImpl(
       CommitEarlyOutReason reason,
       base::TimeTicks main_thread_start_time) override;
   void SetNeedsRedrawOnImpl(const gfx::Rect& damage_rect) override;
   void SetVisibleOnImpl(bool visible) override;
 
   // 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;
   void StartCommitOnImpl(CompletionEvent* completion,
                          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;
   void SetRendererCapabilitiesMainCopy(
       const RendererCapabilities& capabilities) override;
   void BeginMainFrameNotExpectedSoon() override;
   void DidCommitAndDrawFrame() override;
   void SetAnimationEvents(scoped_ptr<AnimationEventsVector> queue) override;
   void DidLoseOutputSurface() override;
   void RequestNewOutputSurface() override;
   void DidInitializeOutputSurface(
       bool success,
       const RendererCapabilities& capabilities) override;
   void DidCompletePageScaleAnimation() override;
   void PostFrameTimingEventsOnMain(
       scoped_ptr<FrameTimingTracker::CompositeTimingSet> composite_events,
       scoped_ptr<FrameTimingTracker::MainFrameTimingSet> main_frame_events)
       override;
   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:
+  // 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;

[vmpstr, 2015/12/04 00:25:41]

Can you leave a comment here explaining that this is initialized after
construction (hence scoped ptr to weak ptr factor) and also explaining why the
weak ptr factory is here and not in proxy impl.

[Khushal, 2015/12/04 22:45:23]

Done.

+
+    // Used on the impl thread to queue calls to ProxyMain to be run on the main
+    // thread. Since the weak pointer is invalidated after the impl-thread tear
+    // down in SynchronouslyCloseImpl, this ensures that any tasks posted to
+    // ProxyMain from the impl thread are abandoned after the impl side has been
+    // destroyed.
+    base::WeakPtr<ProxyMain> proxy_main_weak_ptr;
+
+    explicit CompositorThreadOnly(base::WeakPtr<ProxyMain> proxy_main_weak_ptr);

[vmpstr, 2015/12/04 00:25:41]

Move these to the top please

[Khushal, 2015/12/04 22:45:22]

Done.

+    ~CompositorThreadOnly();
+  };
+
+  // Called on impl thread.
+  void InitializeImplOnImpl(
+      CompletionEvent* completion,
+      LayerTreeHost* layer_tree_host,
+      scoped_ptr<BeginFrameSource> external_begin_frame_source);
+  void CloseImplOnImpl(CompletionEvent* completion);
+
+  bool IsInitialized() const;
+
   base::SingleThreadTaskRunner* MainThreadTaskRunner() const;
   base::SingleThreadTaskRunner* ImplThreadTaskRunner() const;
-
-  ProxyMain* proxy_main_;
-
-  ProxyImpl* proxy_impl_;
+  bool IsMainThread() const;
+  bool IsImplThread() const;
 
   TaskRunnerProvider* task_runner_provider_;
 
-  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+  // Use accessors instead of this variable directly.
+  MainThreadOnly main_thread_only_vars_unsafe_;
+  MainThreadOnly& main();
+  const MainThreadOnly& main() const;

[vmpstr, 2015/12/04 00:25:41]

I understand the logical grouping, but I think we should still have functions
before variables. It makes it easier to find functions.

[Khushal, 2015/12/04 22:45:23]

Done.

 
-  scoped_refptr<base::SingleThreadTaskRunner> impl_task_runner_;
+  // Use accessors instead of this variable directly.
+  CompositorThreadOnly compositor_thread_vars_unsafe_;
+  CompositorThreadOnly& impl();
+  const CompositorThreadOnly& impl() const;
+
+  // Used on the main thread to safely queue calls to ProxyImpl to be run on the
+  // impl thread.
+  base::WeakPtr<ProxyImpl> proxy_impl_weak_ptr_;
 
   DISALLOW_COPY_AND_ASSIGN(ThreadedChannel);
 };
 
 }  // namespace cc
 
 #endif  // CC_TREES_THREADED_CHANNEL_H_