blimp: Move compositing, input and render widget feature to client/core.

blimp/client/core/compositor/blimp_compositor.h

Index: blimp/client/core/compositor/blimp_compositor.h
diff --git a/blimp/client/feature/compositor/blimp_compositor.h b/blimp/client/core/compositor/blimp_compositor.h
similarity index 54%
rename from blimp/client/feature/compositor/blimp_compositor.h
rename to blimp/client/core/compositor/blimp_compositor.h
index c9aeda8ee69c9abdc36f2c5ddb8834f5d033c638..4199b9825d80f933c146455316c4a43b9cb4bccb 100644
--- a/blimp/client/feature/compositor/blimp_compositor.h
+++ b/blimp/client/core/compositor/blimp_compositor.h
@@ -2,72 +2,75 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef BLIMP_CLIENT_FEATURE_COMPOSITOR_BLIMP_COMPOSITOR_H_
-#define BLIMP_CLIENT_FEATURE_COMPOSITOR_BLIMP_COMPOSITOR_H_
+#ifndef BLIMP_CLIENT_CORE_COMPOSITOR_BLIMP_COMPOSITOR_H_
+#define BLIMP_CLIENT_CORE_COMPOSITOR_BLIMP_COMPOSITOR_H_
 
 #include <memory>
 #include <vector>
 
+#include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
-#include "blimp/client/feature/compositor/blimp_input_manager.h"
+#include "blimp/client/core/compositor/blob_image_serialization_processor.h"
+#include "blimp/client/core/input/blimp_input_manager.h"
 #include "cc/trees/layer_tree_host.h"
 #include "cc/trees/layer_tree_host_client.h"
-#include "cc/trees/layer_tree_settings.h"
 #include "cc/trees/remote_proto_channel.h"
-#include "ui/gfx/geometry/size.h"
-#include "ui/gfx/native_widget_types.h"
 
 namespace base {
 class SingleThreadTaskRunner;
 class Thread;
-}
+}  // namespace base
 
 namespace cc {
 namespace proto {
 class CompositorMessage;
 class InitializeImpl;
-}
+}  // namespace proto
+
+class ImageSerializationProcessor;
 class LayerTreeHost;
-}
+class LayerTreeSettings;
+class TaskGraphRunner;
+}  // namespace cc
+
+namespace gpu {
+class GpuMemoryBufferManager;
+}  // namespace gpu
 
 namespace blimp {
 
 class BlimpMessage;
 
 namespace client {
+class BlimpInputManager;
 
 // The BlimpCompositorClient provides the BlimpCompositor with the necessary
-// dependencies for cc::LayerTreeHost owned by this compositor and for
-// communicating the compositor and input messages to the corresponding
-// render widget of this compositor on the engine.
+// dependencies for communicating the compositor and input messages to the
+// corresponding render widget of this compositor on the engine.
 class BlimpCompositorClient {
  public:
-  // These methods should provide the dependencies for cc::LayerTreeHost for
-  // this compositor.
-  virtual cc::LayerTreeSettings* GetLayerTreeSettings() = 0;
-  virtual scoped_refptr<base::SingleThreadTaskRunner>
-  GetCompositorTaskRunner() = 0;
-  virtual cc::TaskGraphRunner* GetTaskGraphRunner() = 0;
-  virtual gpu::GpuMemoryBufferManager* GetGpuMemoryBufferManager() = 0;
-  virtual cc::ImageSerializationProcessor* GetImageSerializationProcessor() = 0;
+  virtual ~BlimpCompositorClient() {}
+
   virtual void DidCompleteSwapBuffers() = 0;
+
+  // Called when a compositor frame update from the engine is committed.
+  // TODO(khushalsagar): Don't listen to commits directly once a remote
+  // compositor is used. This method is used for tracking the number of frame
+  // updates from the engine, don't want to mix that with local commits.
   virtual void DidCommitAndDrawFrame() = 0;
 
   // Should send web gesture events which could not be handled locally by the
   // compositor to the engine.
   virtual void SendWebGestureEvent(
-      int render_widget_id,
       const blink::WebGestureEvent& gesture_event) = 0;
 
   // Should send the compositor messages from the remote client LayerTreeHost of
   // this compositor to the corresponding remote server LayerTreeHost.
   virtual void SendCompositorMessage(
-      int render_widget_id,
       const cc::proto::CompositorMessage& message) = 0;
 
- protected:
-  virtual ~BlimpCompositorClient() {}
+  virtual void RequestOutputSurface() = 0;

[nyquist, 2016/08/16 23:14:57]

What does this do?

[Khushal, 2016/08/18 03:16:32]

Added a comment.

 };
 
 // BlimpCompositor provides the basic framework and setup to host a
@@ -85,40 +88,27 @@ class BlimpCompositorClient {
 // RenderWidget, identified by a custom |render_widget_id| generated on the
 // engine. The lifetime of this compositor is controlled by its corresponding
 // RenderWidget.
-class BlimpCompositor
-    : public cc::LayerTreeHostClient,
-      public cc::RemoteProtoChannel,
-      public BlimpInputManagerClient {
+class BlimpCompositor : public cc::LayerTreeHostClient,
+                        public cc::RemoteProtoChannel,
+                        public BlimpInputManagerClient {
  public:
-  BlimpCompositor(const int render_widget_id, BlimpCompositorClient* client);
+  BlimpCompositor(
+      cc::TaskGraphRunner* task_graph_runner,
+      gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager,
+      cc::ImageSerializationProcessor* image_serialization_processor,
+      cc::LayerTreeSettings* settings,
+      scoped_refptr<base::SingleThreadTaskRunner> compositor_task_runner,
+      BlimpCompositorClient* client);
 
   ~BlimpCompositor() override;
 
-  // Sets whether or not this compositor actually draws to the output surface.
-  // Setting this to false will make the compositor drop all of its resources
-  // and the output surface.  Setting it to true again will rebuild the output
-  // surface from the gfx::AcceleratedWidget (see SetAcceleratedWidget).
-  // virtual for testing.
-  virtual void SetVisible(bool visible);
-
-  // Lets this compositor know that it can draw to |widget|.  This means that,
-  // if this compositor is visible, it will build an output surface and GL
-  // context around |widget| and will draw to it.  ReleaseAcceleratedWidget()
-  // *must* be called before SetAcceleratedWidget() is called with the same
-  // gfx::AcceleratedWidget on another compositor.
-  // virtual for testing.
-  virtual void SetAcceleratedWidget(gfx::AcceleratedWidget widget);
+  void SetVisible(bool visible);

[nyquist, 2016/08/16 23:14:57]

Could we keep the comment on this one?

[Khushal, 2016/08/18 03:16:32]

The previous comments are not valid anymore, since I've moved all that logic to
the BlimpRenderWidget. This class is going to merge into cc/blimp soon, so I
don't want to keep much blimp specific logic here.

Also, the behavior of dropping the OutputSurface when visibility changes is used
when we have an onscreen context, since the native widget that we were drawing
to goes away, but it doesn't need to be done when we have an offscreen context,
which is what we'll use when blimp is embedded by Clank. So all that is in
DirectRenderingDeps in BlimpRenderWidget now.

+  bool IsVisible() const;
 
-  // Releases the internally stored gfx::AcceleratedWidget and the associated
-  // output surface.  This must be called before calling
-  // SetAcceleratedWidget() with the same gfx::AcceleratedWidget on another
-  // compositor.
-  // virtual for testing.
-  virtual void ReleaseAcceleratedWidget();
+  void SetOutputSurface(std::unique_ptr<cc::OutputSurface> output_surface);

[nyquist, 2016/08/16 23:14:57]

Should we explain what these two do and how calling them affect things?

[Khushal, 2016/08/18 03:16:32]

Done.

+  void ReleaseOutputSurface();
 
-  // Forwards the touch event to the |input_manager_|.
-  // virtual for testing.
-  virtual bool OnTouchEvent(const ui::MotionEvent& motion_event);
+  BlimpInputManager* input_manager() const { return input_manager_.get(); }
 
   // Called to forward the compositor message from the remote server
   // LayerTreeHost of the render widget for this compositor.
@@ -126,11 +116,9 @@ class BlimpCompositor
   virtual void OnCompositorMessageReceived(
       std::unique_ptr<cc::proto::CompositorMessage> message);
 
-  int render_widget_id() const { return render_widget_id_; }
+  cc::LayerTreeHost* GetHostForTesting() const { return host_.get(); }
 
  private:
-  friend class BlimpCompositorForTesting;
-
   // LayerTreeHostClient implementation.
   void WillBeginMainFrame() override;
   void DidBeginMainFrame() override;
@@ -159,43 +147,36 @@ class BlimpCompositor
   void SendWebGestureEvent(
       const blink::WebGestureEvent& gesture_event) override;
 
-  // Internal method to correctly set the visibility on the |host_|. It will
-  // make the |host_| visible if |visible| is true and we have a valid |window_|
-  // If |visible_| is false, the host will also release its output surface.
-  void SetVisibleInternal(bool visible);
-
-  // Helper method to build the internal CC compositor instance from |message|.
-  void CreateLayerTreeHost(
-      const cc::proto::InitializeImpl& initialize_message);
+  // Helper method to build the internal CC compositor .
+  void CreateLayerTreeHost();
 
   // Helper method to destroy the internal CC compositor instance and all its
   // associated state.
   void DestroyLayerTreeHost();
 
-  // Creates (if necessary) and returns a TaskRunner for a thread meant to run
-  // compositor rendering.
-  void HandlePendingOutputSurfaceRequest();
+  // The TaskGraphRunner is used to perform raster and image decode work for
+  // the compositor. For a detailed overview, see cc/raster/task_graph_runner.h
+  cc::TaskGraphRunner* task_graph_runner_;
+
+  // Used by the compositor to allocate Gpu memory buffers.
+  gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager_;
 
-  // The unique identifier for the render widget for this compositor.
-  const int render_widget_id_;
+  // Interface to provide the compositor a caching service for images.
+  cc::ImageSerializationProcessor* image_serialization_processor_;
+
+  cc::LayerTreeSettings* settings_;
+
+  // TaskRunner for the thread handling compositor drawing and input handling.
+  scoped_refptr<base::SingleThreadTaskRunner> compositor_task_runner_;
 
   BlimpCompositorClient* client_;
 
   std::unique_ptr<cc::LayerTreeHost> host_;

[nyquist, 2016/08/16 23:14:57]

does this default to nullptr? Or should we set that?

[Khushal, 2016/08/18 03:16:32]

It should be initialized with nullptr for the default ctor.

 
-  gfx::AcceleratedWidget window_;
-
-  // Whether or not |host_| should be visible.  This is stored in case |host_|
-  // is null when SetVisible() is called or if we don't have a
-  // gfx::AcceleratedWidget to build an output surface from.
+  // This is set in case the visibility changes while we don't have a host.
+  // TODO(khushalsagar): We should always create the compositor with the host.
   bool host_should_be_visible_;
 
-  // Whether there is an OutputSurface request pending from the current
-  // |host_|. Becomes |true| if RequestNewOutputSurface is called, and |false|
-  // if |host_| is deleted or we succeed in creating *and* initializing an
-  // OutputSurface (which is essentially the contract with cc).
-  bool output_surface_request_pending_;
-
   // To be notified of any incoming compositor protos that are specifically sent
   // to |render_widget_id_|.
   cc::RemoteProtoChannel::ProtoReceiver* remote_proto_channel_receiver_;
@@ -205,6 +186,8 @@ class BlimpCompositor
   // cc::InputHandler. The input events are forwarded to this input handler by
   // the manager to be handled by the client compositor for the current render
   // widget.
+  // TODO(khushalsagar): Move this out to the render widget once the cc/blimp
+  // refactor work is done.
   std::unique_ptr<BlimpInputManager> input_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(BlimpCompositor);
@@ -213,4 +196,4 @@ class BlimpCompositor
 }  // namespace client
 }  // namespace blimp
 
-#endif  // BLIMP_CLIENT_FEATURE_COMPOSITOR_BLIMP_COMPOSITOR_H_
+#endif  // BLIMP_CLIENT_CORE_COMPOSITOR_BLIMP_COMPOSITOR_H_