cc: Split ThreadProxy into ProxyMain and ProxyImpl

cc/trees/proxy_impl.h

Index: cc/trees/proxy_impl.h
diff --git a/cc/trees/proxy_impl.h b/cc/trees/proxy_impl.h
index 4b0cdaa7c819b790bed03f869b30309d5815af9d..403b06c4f1336e69107412d7b6e27b3745d19446 100644
--- a/cc/trees/proxy_impl.h
+++ b/cc/trees/proxy_impl.h
@@ -5,59 +5,187 @@
 #ifndef CC_TREES_PROXY_IMPL_H_
 #define CC_TREES_PROXY_IMPL_H_
 
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
-#include "cc/base/cc_export.h"
+#include "base/time/time.h"
+#include "cc/animation/animation_events.h"
 #include "cc/base/completion_event.h"
+#include "cc/base/delayed_unique_notifier.h"
 #include "cc/input/top_controls_state.h"
-#include "cc/output/output_surface.h"
 #include "cc/scheduler/commit_earlyout_reason.h"
-#include "cc/trees/proxy_common.h"
+#include "cc/scheduler/scheduler.h"
+#include "cc/trees/channel_impl.h"
+#include "cc/trees/layer_tree_host_impl.h"
 
 namespace cc {
-// TODO(khushalsagar): The impl side of ThreadProxy. It is currently defined as
-// an interface with the implementation provided by ThreadProxy and will be
-// made an independent class.
-// The methods added to this interface should only use the CompositorThreadOnly
-// variables from ThreadProxy.
-// See crbug/527200
-class CC_EXPORT ProxyImpl {
- private:
-  friend class ThreadedChannel;
 
-  // Callback for impl side commands received from the channel.
-  virtual void SetThrottleFrameProductionOnImpl(bool throttle) = 0;
+// This class aggregates all the interactions that the main side of the
+// compositor needs to have with the impl side. It is created and owned by the
+// ChannelImpl implementation. The class lives entirely on the impl thread.
+class CC_EXPORT ProxyImpl : public NON_EXPORTED_BASE(LayerTreeHostImplClient),
+                            public NON_EXPORTED_BASE(SchedulerClient) {
+ public:
+  static scoped_ptr<ProxyImpl> Create(
+      ChannelImpl* channel_impl,
+      LayerTreeHost* layer_tree_host,
+      TaskRunnerProvider* task_runner_provider,
+      scoped_ptr<BeginFrameSource> external_begin_frame_source);
+
+  ~ProxyImpl() override;
+
+  // Virtual for testing.

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

Do all of these need to be virtual for testing?

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

We want to make sure that all channel implementations actually route the calls
between the main and the impl side. We could run those tests by verifying the
effect of a call but the intent was to have them as smoke tests, independent of
the internal details of ProxyMain/Impl. So these needed to be made virtual to
let TestHooks know about each call in the tests.

[vmpstr, 2015/12/07 22:14:42]

Sounds good. Thanks for explaining.

+  virtual void SetThrottleFrameProductionOnImpl(bool throttle);
   virtual void UpdateTopControlsStateOnImpl(TopControlsState constraints,
                                             TopControlsState current,
-                                            bool animate) = 0;
-  virtual void InitializeOutputSurfaceOnImpl(OutputSurface* output_surface) = 0;
-  virtual void MainThreadHasStoppedFlingingOnImpl() = 0;
-  virtual void SetInputThrottledUntilCommitOnImpl(bool is_throttled) = 0;
-  virtual void SetDeferCommitsOnImpl(bool defer_commits) const = 0;
-  virtual void SetNeedsRedrawOnImpl(const gfx::Rect& damage_rect) = 0;
-  virtual void SetNeedsCommitOnImpl() = 0;
-  virtual void FinishAllRenderingOnImpl(CompletionEvent* completion) = 0;
-  virtual void SetVisibleOnImpl(bool visible) = 0;
-  virtual void ReleaseOutputSurfaceOnImpl(CompletionEvent* completion) = 0;
-  virtual void FinishGLOnImpl(CompletionEvent* completion) = 0;
-  virtual void MainFrameWillHappenOnImplForTesting(
-      CompletionEvent* completion,
-      bool* main_frame_will_happen) = 0;
+                                            bool animate);
+  virtual void InitializeOutputSurfaceOnImpl(OutputSurface* output_surface);
+  virtual void MainThreadHasStoppedFlingingOnImpl();
+  virtual void SetInputThrottledUntilCommitOnImpl(bool is_throttled);
+  virtual void SetDeferCommitsOnImpl(bool defer_commits) const;
+  virtual void SetNeedsRedrawOnImpl(const gfx::Rect& damage_rect);
+  virtual void SetNeedsCommitOnImpl();
   virtual void BeginMainFrameAbortedOnImpl(
       CommitEarlyOutReason reason,
-      base::TimeTicks main_thread_start_time) = 0;
+      base::TimeTicks main_thread_start_time);
+  virtual void FinishAllRenderingOnImpl(CompletionEvent* completion);
+  virtual void SetVisibleOnImpl(bool visible);
+  virtual void ReleaseOutputSurfaceOnImpl(CompletionEvent* completion);
+  virtual void FinishGLOnImpl(CompletionEvent* completion);
+  virtual void MainFrameWillHappenOnImplForTesting(
+      CompletionEvent* completion,
+      bool* main_frame_will_happen);
   virtual void StartCommitOnImpl(CompletionEvent* completion,
                                  LayerTreeHost* layer_tree_host,
                                  base::TimeTicks main_thread_start_time,
-                                 bool hold_commit_for_activation) = 0;
-  virtual void InitializeImplOnImpl(CompletionEvent* completion,
-                                    LayerTreeHost* layer_tree_host) = 0;
-  virtual void LayerTreeHostClosedOnImpl(CompletionEvent* completion) = 0;
+                                 bool hold_commit_for_activation);
 
-  // TODO(khushalsagar): Rename as GetWeakPtr() once ThreadProxy is split.
-  virtual base::WeakPtr<ProxyImpl> GetImplWeakPtr() = 0;
+  bool HasCommitCompletionEventForTesting() const;
+  bool GetNextCommitWaitsForActivationForTesting() const;
 
  protected:
-  virtual ~ProxyImpl() {}
+  // protected for testing.

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

If we're going to expose more variables and if the only override for this is in
test, then I would be ok with making the rest of the functionality here
protected instead of private as well.

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

Since the variables and all the methods will be exposed through
ProxyImplForTest, we can keep this all private and just have that as the only
friend class.

[vmpstr, 2015/12/07 22:14:42]

I was thinking along the lines of someone writing more tests somewhere and
writing their own version of FakeProxyImpl or something... In that case, making
everything protected seems kind of better.

Since we don't have other uses though, it's your call.

[Khushal, 2015/12/08 02:19:35]

Ah, makes sense. I'll keep it private for now. We can change it if we end up
adding more test classes.

+  ProxyImpl(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 impl thread only when
+  // the main thread is blocked for a commit.
+  struct BlockedMainCommitOnly {
+    BlockedMainCommitOnly();
+    ~BlockedMainCommitOnly();
+    LayerTreeHost* layer_tree_host;
+  };
+
+  friend class ProxyImplForTest;
+  friend class ThreadedChannelTestBeginMainFrameNotExpectedSoon;
+  friend class ThreadedChannelTestSetAnimationEvents;
+  friend class ThreadedChannelTestLoseOutputSurface;
+  friend class ThreadedChannelTestPageScaleAnimation;
+
+  // LayerTreeHostImplClient implementation
+  void UpdateRendererCapabilitiesOnImplThread() override;
+  void DidLoseOutputSurfaceOnImplThread() override;
+  void CommitVSyncParameters(base::TimeTicks timebase,
+                             base::TimeDelta interval) override;
+  void SetEstimatedParentDrawTime(base::TimeDelta draw_time) override;
+  void DidSwapBuffersOnImplThread() override;
+  void DidSwapBuffersCompleteOnImplThread() override;
+  void OnResourcelessSoftareDrawStateChanged(bool resourceless_draw) override;
+  void OnCanDrawStateChanged(bool can_draw) override;
+  void NotifyReadyToActivate() override;
+  void NotifyReadyToDraw() override;
+  // Please call these 3 functions through
+  // LayerTreeHostImpl's SetNeedsRedraw(), SetNeedsRedrawRect() and
+  // SetNeedsAnimate().
+  void SetNeedsRedrawOnImplThread() override;
+  void SetNeedsRedrawRectOnImplThread(const gfx::Rect& dirty_rect) override;
+  void SetNeedsAnimateOnImplThread() override;
+  void SetNeedsPrepareTilesOnImplThread() override;
+  void SetNeedsCommitOnImplThread() override;
+  void SetVideoNeedsBeginFrames(bool needs_begin_frames) override;
+  void PostAnimationEventsToMainThreadOnImplThread(
+      scoped_ptr<AnimationEventsVector> queue) override;
+  bool IsInsideDraw() override;
+  void RenewTreePriority() override;
+  void PostDelayedAnimationTaskOnImplThread(const base::Closure& task,
+                                            base::TimeDelta delay) override;
+  void DidActivateSyncTree() override;
+  void WillPrepareTiles() override;
+  void DidPrepareTiles() override;
+  void DidCompletePageScaleAnimationOnImplThread() override;
+  void OnDrawForOutputSurface() override;
+  // This should only be called by LayerTreeHostImpl::PostFrameTimingEvents.
+  void PostFrameTimingEventsOnImplThread(
+      scoped_ptr<FrameTimingTracker::CompositeTimingSet> composite_events,
+      scoped_ptr<FrameTimingTracker::MainFrameTimingSet> main_frame_events)
+      override;
+
+  // SchedulerClient implementation
+  void WillBeginImplFrame(const BeginFrameArgs& args) override;
+  void DidFinishImplFrame() override;
+  void ScheduledActionSendBeginMainFrame(const BeginFrameArgs& args) override;
+  DrawResult ScheduledActionDrawAndSwapIfPossible() override;
+  DrawResult ScheduledActionDrawAndSwapForced() override;
+  void ScheduledActionAnimate() override;
+  void ScheduledActionCommit() override;
+  void ScheduledActionActivateSyncTree() override;
+  void ScheduledActionBeginOutputSurfaceCreation() override;
+  void ScheduledActionPrepareTiles() override;
+  void ScheduledActionInvalidateOutputSurface() override;
+  void SendBeginFramesToChildren(const BeginFrameArgs& args) override;
+  void SendBeginMainFrameNotExpectedSoon() override;
+
+  DrawResult DrawAndSwapInternal(bool forced_draw);
+
+  bool IsImplThread() const;
+  bool IsMainThreadBlocked() const;
+
+  const int layer_tree_host_id_;
+
+  scoped_ptr<Scheduler> scheduler_;
+
+  // Set when the main thread is waiting on a pending tree activation.
+  bool next_commit_waits_for_activation_;
+
+  // Set when the main thread is waiting on a commit to complete or on a
+  // pending tree activation.
+  CompletionEvent* commit_completion_event_;
+
+  // Set when the next draw should post DidCommitAndDrawFrame to the main
+  // thread.
+  bool next_frame_is_newly_committed_frame_;
+
+  bool inside_draw_;
+  bool input_throttled_until_commit_;
+
+  // Whether a commit has been completed since the last time animations were
+  // ticked. If this happens, we need to animate again.
+  bool did_commit_after_animating_;
+
+  TaskRunnerProvider* task_runner_provider_;
+
+  DelayedUniqueNotifier smoothness_priority_expiration_notifier_;
+
+  scoped_ptr<BeginFrameSource> external_begin_frame_source_;
+
+  RenderingStatsInstrumentation* rendering_stats_instrumentation_;
+
+  // Values used to keep track of frame durations. Used only in frame timing.
+  BeginFrameArgs last_begin_main_frame_args_;
+  BeginFrameArgs last_processed_begin_main_frame_args_;
+
+  scoped_ptr<LayerTreeHostImpl> layer_tree_host_impl_;
+
+  ChannelImpl* channel_impl_;
+
+  // Use accessors instead of this variable directly.
+  BlockedMainCommitOnly main_thread_blocked_commit_vars_unsafe_;
+  BlockedMainCommitOnly& blocked_main_commit();
+
+  DISALLOW_COPY_AND_ASSIGN(ProxyImpl);
 };
 
 }  // namespace cc