exo: Use one LayerTreeFrameSink per toplevel window

Having one LayerTreeFrameSink for every sub-surface
causes synchronizing problem between sub-surfaces, so
we decide to only have one LayerTreeFrameSink per
toplevel window.

Bug: 731742
Change-Id: I6f3e09c33f6f98aab281ce2fab032f9df29f1e7d
Reviewed-on: https://chromium-review.googlesource.com/552719
Commit-Queue: Peng Huang <penghuang@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486788}

components/exo/buffer.cc

@@ -411,7 +411,6 @@ Buffer::~Buffer() {}
 
 bool Buffer::ProduceTransferableResource(
     LayerTreeFrameSinkHolder* layer_tree_frame_sink_holder,
-    cc::ResourceId resource_id,
     bool secure_output_only,
     bool client_usage,
     cc::TransferableResource* resource) {
@@ -439,7 +438,7 @@ bool Buffer::ProduceTransferableResource(
     return false;
   }
 
-  resource->id = resource_id;
+  resource->id = layer_tree_frame_sink_holder->AllocateResourceId();
   resource->format = viz::RGBA_8888;
   resource->filter = GL_LINEAR;
   resource->size = gpu_memory_buffer_->GetSize();
@@ -473,7 +472,7 @@ bool Buffer::ProduceTransferableResource(
     // The contents texture will be released when no longer used by the
     // compositor.
     layer_tree_frame_sink_holder->SetResourceReleaseCallback(
-        resource_id,
+        resource->id,
         base::Bind(&Buffer::Texture::ReleaseTexImage,
                    base::Unretained(contents_texture),
                    base::Bind(&Buffer::ReleaseContentsTexture, AsWeakPtr(),
@@ -503,7 +502,7 @@ bool Buffer::ProduceTransferableResource(
   // The mailbox texture will be released when no longer used by the
   // compositor.
   layer_tree_frame_sink_holder->SetResourceReleaseCallback(
-      resource_id,
+      resource->id,
       base::Bind(&Buffer::Texture::Release, base::Unretained(texture),
                  base::Bind(&Buffer::ReleaseTexture, AsWeakPtr(),
                             base::Passed(&texture_))));

components/exo/buffer.h

@@ -54,7 +54,6 @@ class Buffer : public base::SupportsWeakPtr<Buffer> {
   // |non_client_usage| is true.
   bool ProduceTransferableResource(
       LayerTreeFrameSinkHolder* layer_tree_frame_sink_holder,
-      cc::ResourceId resource_id,
       bool secure_output_only,
       bool client_usage,
       cc::TransferableResource* resource);

components/exo/buffer_unittest.cc

@@ -8,7 +8,7 @@
 #include "cc/output/context_provider.h"
 #include "cc/resources/single_release_callback.h"
 #include "components/exo/buffer.h"
-#include "components/exo/surface.h"
+#include "components/exo/surface_tree_host.h"
 #include "components/exo/test/exo_test_base.h"
 #include "components/exo/test/exo_test_helper.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
@@ -29,12 +29,12 @@ void Release(int* release_call_count) {
 
 TEST_F(BufferTest, ReleaseCallback) {
   gfx::Size buffer_size(256, 256);
-  std::unique_ptr<Buffer> buffer(
-      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
-  std::unique_ptr<Surface> surface(new Surface);
-  const viz::FrameSinkId arbitrary_frame_sink_id(1, 1);
-  LayerTreeFrameSinkHolder* layer_tree_frame_sink_holder =
-      surface->layer_tree_frame_sink_holder();
+  auto buffer = base::MakeUnique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+  auto surface_tree_host =
+      base::MakeUnique<SurfaceTreeHost>("BufferTest", nullptr);
+  LayerTreeFrameSinkHolder* frame_sink_holder =
+      surface_tree_host->layer_tree_frame_sink_holder();
 
   // Set the release callback.
   int release_call_count = 0;
@@ -44,8 +44,8 @@ TEST_F(BufferTest, ReleaseCallback) {
   buffer->OnAttach();
   cc::TransferableResource resource;
   // Produce a transferable resource for the contents of the buffer.
-  bool rv = buffer->ProduceTransferableResource(layer_tree_frame_sink_holder, 0,
-                                                false, true, &resource);
+  bool rv = buffer->ProduceTransferableResource(frame_sink_holder, false, true,
+                                                &resource);
   ASSERT_TRUE(rv);
 
   // Release buffer.
@@ -54,7 +54,7 @@ TEST_F(BufferTest, ReleaseCallback) {
   returned_resource.sync_token = resource.mailbox_holder.sync_token;
   returned_resource.lost = false;
   std::vector<cc::ReturnedResource> resources = {returned_resource};
-  layer_tree_frame_sink_holder->ReclaimResources(resources);
+  frame_sink_holder->ReclaimResources(resources);
 
   RunAllPendingInMessageLoop();
   ASSERT_EQ(release_call_count, 0);
@@ -67,19 +67,18 @@ TEST_F(BufferTest, ReleaseCallback) {
 
 TEST_F(BufferTest, IsLost) {
   gfx::Size buffer_size(256, 256);
-  std::unique_ptr<Buffer> buffer(
-      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
-  const viz::FrameSinkId arbitrary_frame_sink_id(1, 1);
-  std::unique_ptr<Surface> surface(new Surface);
-  LayerTreeFrameSinkHolder* layer_tree_frame_sink_holder =
-      surface->layer_tree_frame_sink_holder();
-  cc::ResourceId resource_id = 0;
+  auto buffer = base::MakeUnique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+  auto surface_tree_host =
+      base::MakeUnique<SurfaceTreeHost>("BufferTest", nullptr);
+  LayerTreeFrameSinkHolder* frame_sink_holder =
+      surface_tree_host->layer_tree_frame_sink_holder();
 
   buffer->OnAttach();
   // Acquire a texture transferable resource for the contents of the buffer.
   cc::TransferableResource resource;
-  bool rv = buffer->ProduceTransferableResource(
-      layer_tree_frame_sink_holder, resource_id, false, true, &resource);
+  bool rv = buffer->ProduceTransferableResource(frame_sink_holder, false, true,
+                                                &resource);
   ASSERT_TRUE(rv);
 
   scoped_refptr<cc::ContextProvider> context_provider =
@@ -95,28 +94,27 @@ TEST_F(BufferTest, IsLost) {
   // Release buffer.
   bool is_lost = true;
   cc::ReturnedResource returned_resource;
-  returned_resource.id = resource_id;
+  returned_resource.id = resource.id;
   returned_resource.sync_token = gpu::SyncToken();
   returned_resource.lost = is_lost;
   std::vector<cc::ReturnedResource> resources = {returned_resource};
-  layer_tree_frame_sink_holder->ReclaimResources(resources);
+  frame_sink_holder->ReclaimResources(resources);
   RunAllPendingInMessageLoop();
 
   // Producing a new texture transferable resource for the contents of the
   // buffer.
-  ++resource_id;
   cc::TransferableResource new_resource;
-  rv = buffer->ProduceTransferableResource(
-      layer_tree_frame_sink_holder, resource_id, false, false, &new_resource);
+  rv = buffer->ProduceTransferableResource(frame_sink_holder, false, false,
+                                           &new_resource);
   ASSERT_TRUE(rv);
   buffer->OnDetach();
 
   cc::ReturnedResource returned_resource2;
-  returned_resource2.id = resource_id;
+  returned_resource2.id = new_resource.id;
   returned_resource2.sync_token = gpu::SyncToken();
   returned_resource2.lost = false;
   std::vector<cc::ReturnedResource> resources2 = {returned_resource2};
-  layer_tree_frame_sink_holder->ReclaimResources(resources2);
+  frame_sink_holder->ReclaimResources(resources2);
   RunAllPendingInMessageLoop();
 }
 

components/exo/layer_tree_frame_sink_holder.cc

@@ -6,7 +6,7 @@
 
 #include "cc/output/layer_tree_frame_sink.h"
 #include "cc/resources/returned_resource.h"
-#include "components/exo/surface.h"
+#include "components/exo/surface_tree_host.h"
 
 namespace exo {
 
@@ -14,19 +14,16 @@ namespace exo {
 // LayerTreeFrameSinkHolder, public:
 
 LayerTreeFrameSinkHolder::LayerTreeFrameSinkHolder(
-    Surface* surface,
+    SurfaceTreeHost* surface_tree_host,
     std::unique_ptr<cc::LayerTreeFrameSink> frame_sink)
-    : surface_(surface),
+    : surface_tree_host_(surface_tree_host),
       frame_sink_(std::move(frame_sink)),
       weak_factory_(this) {
-  surface_->AddSurfaceObserver(this);
   frame_sink_->BindToClient(this);
 }
 
 LayerTreeFrameSinkHolder::~LayerTreeFrameSinkHolder() {
   frame_sink_->DetachFromClient();
-  if (surface_)
-    surface_->RemoveSurfaceObserver(this);
 
   // Release all resources which aren't returned from LayerTreeFrameSink.
   for (auto& callback : release_callbacks_)
@@ -58,8 +55,7 @@ base::WeakPtr<LayerTreeFrameSinkHolder> LayerTreeFrameSinkHolder::GetWeakPtr() {
 
 void LayerTreeFrameSinkHolder::SetBeginFrameSource(
     cc::BeginFrameSource* source) {
-  if (surface_)
-    surface_->SetBeginFrameSource(source);
+  surface_tree_host_->SetBeginFrameSource(source);
 }
 
 void LayerTreeFrameSinkHolder::ReclaimResources(
@@ -75,16 +71,7 @@ void LayerTreeFrameSinkHolder::ReclaimResources(
 }
 
 void LayerTreeFrameSinkHolder::DidReceiveCompositorFrameAck() {
-  if (surface_)
-    surface_->DidReceiveCompositorFrameAck();
-}
-
-////////////////////////////////////////////////////////////////////////////////
-// SurfaceObserver overrides:
-
-void LayerTreeFrameSinkHolder::OnSurfaceDestroying(Surface* surface) {
-  surface_->RemoveSurfaceObserver(this);
-  surface_ = nullptr;
+  surface_tree_host_->DidReceiveCompositorFrameAck();
 }
 
 }  // namespace exo

components/exo/layer_tree_frame_sink_holder.h

@@ -10,23 +10,19 @@
 #include "base/containers/flat_map.h"
 #include "cc/output/layer_tree_frame_sink_client.h"
 #include "cc/resources/release_callback.h"
-#include "components/exo/surface_observer.h"
 
 namespace cc {
 class LayerTreeFrameSink;
 }
 
 namespace exo {
-class Surface;
+class SurfaceTreeHost;
 
 // This class talks to CompositorFrameSink and keeps track of references to
-// the contents of Buffers. It's keeped alive by references from
-// release_callbacks_. It's destroyed when its owning Surface is destroyed and
-// the last outstanding release callback is called.
-class LayerTreeFrameSinkHolder : public cc::LayerTreeFrameSinkClient,
-                                 public SurfaceObserver {
+// the contents of Buffers.
+class LayerTreeFrameSinkHolder : public cc::LayerTreeFrameSinkClient {
  public:
-  LayerTreeFrameSinkHolder(Surface* surface,
+  LayerTreeFrameSinkHolder(SurfaceTreeHost* surface_tree_host,
                            std::unique_ptr<cc::LayerTreeFrameSink> frame_sink);
   ~LayerTreeFrameSinkHolder() override;
 
@@ -53,15 +49,12 @@ class LayerTreeFrameSinkHolder : public cc::LayerTreeFrameSinkClient,
       const gfx::Rect& viewport_rect,
       const gfx::Transform& transform) override {}
 
-  // Overridden from SurfaceObserver:
-  void OnSurfaceDestroying(Surface* surface) override;
-
  private:
   // A collection of callbacks used to release resources.
   using ResourceReleaseCallbackMap = base::flat_map<int, cc::ReleaseCallback>;
   ResourceReleaseCallbackMap release_callbacks_;
 
-  Surface* surface_;
+  SurfaceTreeHost* surface_tree_host_;
   std::unique_ptr<cc::LayerTreeFrameSink> frame_sink_;
 
   // The next resource id the buffer is attached to.

components/exo/shell_surface.cc

@@ -956,6 +956,9 @@ void ShellSurface::OnPostWindowStateTypeChange(
 void ShellSurface::OnWindowBoundsChanged(aura::Window* window,
                                          const gfx::Rect& old_bounds,
                                          const gfx::Rect& new_bounds) {
+  if (window == host_window())
+    return;
+
   // TODO(domlaskowski): For BoundsMode::CLIENT, the configure callback does not
   // yet support resizing. See crbug.com/699746.
   if (bounds_mode_ == BoundsMode::CLIENT)
@@ -987,7 +990,23 @@ void ShellSurface::OnWindowBoundsChanged(aura::Window* window,
   }
 }
 
+void ShellSurface::OnWindowAddedToRootWindow(aura::Window* window) {
+  if (window == host_window())
+    SurfaceTreeHost::OnWindowAddedToRootWindow(window);
+}
+
+void ShellSurface::OnWindowRemovingFromRootWindow(aura::Window* window,
+                                                  aura::Window* new_root) {
+  if (window == host_window())
+    SurfaceTreeHost::OnWindowRemovingFromRootWindow(window, new_root);
+}
+
 void ShellSurface::OnWindowDestroying(aura::Window* window) {
+  if (window == host_window()) {
+    SurfaceTreeHost::OnWindowDestroying(window);
+    return;
+  }
+
   if (window == parent_) {
     parent_ = nullptr;
     // |parent_| being set to null effects the ability to maximize the window.

components/exo/shell_surface.h

@@ -16,7 +16,6 @@
 #include "components/exo/surface_observer.h"
 #include "components/exo/surface_tree_host.h"
 #include "components/exo/wm_helper.h"
-#include "ui/aura/window_observer.h"
 #include "ui/base/hit_test.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
@@ -47,7 +46,6 @@ class ShellSurface : public SurfaceTreeHost,
                      public views::WidgetDelegate,
                      public views::View,
                      public ash::wm::WindowStateObserver,
-                     public aura::WindowObserver,
                      public WMHelper::ActivationObserver,
                      public WMHelper::DisplayConfigurationObserver {
  public:
@@ -258,6 +256,9 @@ class ShellSurface : public SurfaceTreeHost,
   void OnWindowBoundsChanged(aura::Window* window,
                              const gfx::Rect& old_bounds,
                              const gfx::Rect& new_bounds) override;
+  void OnWindowAddedToRootWindow(aura::Window* window) override;
+  void OnWindowRemovingFromRootWindow(aura::Window* window,
+                                      aura::Window* new_root) override;
   void OnWindowDestroying(aura::Window* window) override;
 
   // Overridden from WMHelper::ActivationObserver:

components/exo/sub_surface.cc

@@ -96,7 +96,8 @@ void SubSurface::OnSurfaceCommit() {
   if (IsSurfaceSynchronized())
     return;
 
-  surface_->CommitSurfaceHierarchy();
+  // TODO(penghuang): http://crbug.com/740110 Support async mode.
+  NOTIMPLEMENTED() << "Async subsurface is not supported!";
 }
 
 bool SubSurface::IsSurfaceSynchronized() const {

components/exo/sub_surface_unittest.cc

@@ -5,6 +5,7 @@
 #include "components/exo/sub_surface.h"
 
 #include "base/memory/ptr_util.h"
+#include "components/exo/shell_surface.h"
 #include "components/exo/surface.h"
 #include "components/exo/test/exo_test_base.h"
 #include "components/exo/test/exo_test_helper.h"
@@ -16,10 +17,10 @@ namespace {
 using SubSurfaceTest = test::ExoTestBase;
 
 TEST_F(SubSurfaceTest, SetPosition) {
-  std::unique_ptr<Surface> parent(new Surface);
-  std::unique_ptr<Surface> surface(new Surface);
-  std::unique_ptr<SubSurface> sub_surface(
-      new SubSurface(surface.get(), parent.get()));
+  auto parent = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(parent.get());
+  auto surface = base::MakeUnique<Surface>();
+  auto sub_surface = base::MakeUnique<SubSurface>(surface.get(), parent.get());
 
   // Initial position is at the origin.
   EXPECT_EQ(gfx::Point().ToString(),
@@ -49,14 +50,15 @@ TEST_F(SubSurfaceTest, SetPosition) {
 }
 
 TEST_F(SubSurfaceTest, PlaceAbove) {
-  std::unique_ptr<Surface> parent(new Surface);
-  std::unique_ptr<Surface> surface1(new Surface);
-  std::unique_ptr<Surface> surface2(new Surface);
-  std::unique_ptr<Surface> non_sibling_surface(new Surface);
-  std::unique_ptr<SubSurface> sub_surface1(
-      new SubSurface(surface1.get(), parent.get()));
-  std::unique_ptr<SubSurface> sub_surface2(
-      new SubSurface(surface2.get(), parent.get()));
+  auto parent = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(parent.get());
+  auto surface1 = base::MakeUnique<Surface>();
+  auto surface2 = base::MakeUnique<Surface>();
+  auto non_sibling_surface = base::MakeUnique<Surface>();
+  auto sub_surface1 =
+      base::MakeUnique<SubSurface>(surface1.get(), parent.get());
+  auto sub_surface2 =
+      base::MakeUnique<SubSurface>(surface2.get(), parent.get());
 
   ASSERT_EQ(2u, parent->window()->children().size());
   EXPECT_EQ(surface1->window(), parent->window()->children()[0]);
@@ -80,14 +82,15 @@ TEST_F(SubSurfaceTest, PlaceAbove) {
 }
 
 TEST_F(SubSurfaceTest, PlaceBelow) {
-  std::unique_ptr<Surface> parent(new Surface);
-  std::unique_ptr<Surface> surface1(new Surface);
-  std::unique_ptr<Surface> surface2(new Surface);
-  std::unique_ptr<Surface> non_sibling_surface(new Surface);
-  std::unique_ptr<SubSurface> sub_surface1(
-      new SubSurface(surface1.get(), parent.get()));
-  std::unique_ptr<SubSurface> sub_surface2(
-      new SubSurface(surface2.get(), parent.get()));
+  auto parent = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(parent.get());
+  auto surface1 = base::MakeUnique<Surface>();
+  auto surface2 = base::MakeUnique<Surface>();
+  auto non_sibling_surface = base::MakeUnique<Surface>();
+  auto sub_surface1 =
+      base::MakeUnique<SubSurface>(surface1.get(), parent.get());
+  auto sub_surface2 =
+      base::MakeUnique<SubSurface>(surface2.get(), parent.get());
 
   ASSERT_EQ(2u, parent->window()->children().size());
   EXPECT_EQ(surface1->window(), parent->window()->children()[0]);
@@ -111,13 +114,14 @@ TEST_F(SubSurfaceTest, PlaceBelow) {
 }
 
 TEST_F(SubSurfaceTest, SetCommitBehavior) {
-  std::unique_ptr<Surface> parent(new Surface);
-  std::unique_ptr<Surface> child(new Surface);
-  std::unique_ptr<Surface> grandchild(new Surface);
-  std::unique_ptr<SubSurface> child_sub_surface(
-      new SubSurface(child.get(), parent.get()));
-  std::unique_ptr<SubSurface> grandchild_sub_surface(
-      new SubSurface(grandchild.get(), child.get()));
+  auto parent = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(parent.get());
+  auto child = base::MakeUnique<Surface>();
+  auto grandchild = base::MakeUnique<Surface>();
+  auto child_sub_surface =
+      base::MakeUnique<SubSurface>(child.get(), parent.get());
+  auto grandchild_sub_surface =
+      base::MakeUnique<SubSurface>(grandchild.get(), child.get());
 
   // Initial position is at the origin.
   EXPECT_EQ(gfx::Point().ToString(),
@@ -141,19 +145,20 @@ TEST_F(SubSurfaceTest, SetCommitBehavior) {
   EXPECT_EQ(position1.ToString(),
             grandchild->window()->bounds().origin().ToString());
 
+  // TODO(penghuang): http://crbug.com/740110 Support async mode.
   // Disable synchronous commit behavior.
-  bool synchronized = false;
-  child_sub_surface->SetCommitBehavior(synchronized);
+  // bool synchronized = false;
+  // child_sub_surface->SetCommitBehavior(synchronized);
 
   // Set position to 20, 20.
-  gfx::Point position2(20, 20);
-  grandchild_sub_surface->SetPosition(position2);
-  child->Commit();
+  // gfx::Point position2(20, 20);
+  // grandchild_sub_surface->SetPosition(position2);
+  // child->Commit();
 
   // A Commit() call on child should be sufficient for the position of
   // grandchild to take effect when synchronous is disabled.
-  EXPECT_EQ(position2.ToString(),
-            grandchild->window()->bounds().origin().ToString());
+  // EXPECT_EQ(position2.ToString(),
+  //           grandchild->window()->bounds().origin().ToString());
 }
 
 }  // namespace

components/exo/surface.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/callback_helpers.h"
+#include "base/containers/adapters.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
@@ -27,7 +28,6 @@
 #include "components/viz/common/surfaces/sequence_surface_reference_factory.h"
 #include "third_party/khronos/GLES2/gl2.h"
 #include "ui/aura/client/aura_constants.h"
-#include "ui/aura/env.h"
 #include "ui/aura/window_delegate.h"
 #include "ui/aura/window_targeter.h"
 #include "ui/base/class_property.h"
@@ -189,42 +189,23 @@ Surface::Surface() : window_(new aura::Window(new CustomWindowDelegate(this))) {
   window_->SetType(aura::client::WINDOW_TYPE_CONTROL);
   window_->SetName("ExoSurface");
   window_->SetProperty(kSurfaceKey, this);
-  window_->Init(ui::LAYER_SOLID_COLOR);
+  window_->Init(ui::LAYER_NOT_DRAWN);
   window_->SetEventTargeter(base::WrapUnique(new CustomWindowTargeter));
   window_->set_owned_by_parent(false);
-  window_->AddObserver(this);
-  aura::Env::GetInstance()->context_factory()->AddObserver(this);
-  layer_tree_frame_sink_holder_ = base::MakeUnique<LayerTreeFrameSinkHolder>(
-      this, window_->CreateLayerTreeFrameSink());
 }
 
 Surface::~Surface() {
-  aura::Env::GetInstance()->context_factory()->RemoveObserver(this);
   for (SurfaceObserver& observer : observers_)
     observer.OnSurfaceDestroying(this);
 
-  window_->RemoveObserver(this);
-  if (window_->layer()->GetCompositor())
-    window_->layer()->GetCompositor()->vsync_manager()->RemoveObserver(this);
-  window_->layer()->SetShowSolidColorContent();
-
-  frame_callbacks_.splice(frame_callbacks_.end(), pending_frame_callbacks_);
-  active_frame_callbacks_.splice(active_frame_callbacks_.end(),
-                                 frame_callbacks_);
   // Call all frame callbacks with a null frame time to indicate that they
   // have been cancelled.
-  for (const auto& frame_callback : active_frame_callbacks_)
+  for (const auto& frame_callback : pending_frame_callbacks_)
     frame_callback.Run(base::TimeTicks());
 
-  presentation_callbacks_.splice(presentation_callbacks_.end(),
-                                 pending_presentation_callbacks_);
-  swapping_presentation_callbacks_.splice(
-      swapping_presentation_callbacks_.end(), presentation_callbacks_);
-  swapped_presentation_callbacks_.splice(swapped_presentation_callbacks_.end(),
-                                         swapping_presentation_callbacks_);
   // Call all presentation callbacks with a null presentation time to indicate
   // that they have been cancelled.
-  for (const auto& presentation_callback : swapped_presentation_callbacks_)
+  for (const auto& presentation_callback : pending_presentation_callbacks_)
     presentation_callback.Run(base::TimeTicks(), base::TimeDelta());
 }
 
@@ -233,10 +214,6 @@ Surface* Surface::AsSurface(const aura::Window* window) {
   return window->GetProperty(kSurfaceKey);
 }
 
-viz::SurfaceId Surface::GetSurfaceId() const {
-  return window_->GetSurfaceId();
-}
-
 void Surface::Attach(Buffer* buffer) {
   TRACE_EVENT1("exo", "Surface::Attach", "buffer",
                buffer ? buffer->GetSize().ToString() : "null");
@@ -290,11 +267,13 @@ void Surface::AddSubSurface(Surface* sub_surface) {
 
   DCHECK(!sub_surface->window()->parent());
   DCHECK(!sub_surface->window()->IsVisible());
+  sub_surface->window()->SetBounds(
+      gfx::Rect(sub_surface->window()->bounds().size()));
   window_->AddChild(sub_surface->window());
 
   DCHECK(!ListContainsEntry(pending_sub_surfaces_, sub_surface));
   pending_sub_surfaces_.push_back(std::make_pair(sub_surface, gfx::Point()));
-  has_pending_layer_changes_ = true;
+  sub_surfaces_.push_back(std::make_pair(sub_surface, gfx::Point()));
 }
 
 void Surface::RemoveSubSurface(Surface* sub_surface) {
@@ -308,7 +287,8 @@ void Surface::RemoveSubSurface(Surface* sub_surface) {
   DCHECK(ListContainsEntry(pending_sub_surfaces_, sub_surface));
   pending_sub_surfaces_.erase(
       FindListEntry(pending_sub_surfaces_, sub_surface));
-  has_pending_layer_changes_ = true;
+  DCHECK(ListContainsEntry(sub_surfaces_, sub_surface));
+  sub_surfaces_.erase(FindListEntry(sub_surfaces_, sub_surface));
 }
 
 void Surface::SetSubSurfacePosition(Surface* sub_surface,
@@ -321,7 +301,7 @@ void Surface::SetSubSurfacePosition(Surface* sub_surface,
   if (it->second == position)
     return;
   it->second = position;
-  has_pending_layer_changes_ = true;
+  sub_surfaces_changed_ = true;
 }
 
 void Surface::PlaceSubSurfaceAbove(Surface* sub_surface, Surface* reference) {
@@ -353,7 +333,7 @@ void Surface::PlaceSubSurfaceAbove(Surface* sub_surface, Surface* reference) {
   if (it == position_it)
     return;
   pending_sub_surfaces_.splice(position_it, pending_sub_surfaces_, it);
-  has_pending_layer_changes_ = true;
+  sub_surfaces_changed_ = true;
 }
 
 void Surface::PlaceSubSurfaceBelow(Surface* sub_surface, Surface* sibling) {
@@ -378,7 +358,7 @@ void Surface::PlaceSubSurfaceBelow(Surface* sub_surface, Surface* sibling) {
   if (it == sibling_it)
     return;
   pending_sub_surfaces_.splice(sibling_it, pending_sub_surfaces_, it);
-  has_pending_layer_changes_ = true;
+  sub_surfaces_changed_ = true;
 }
 
 void Surface::SetViewport(const gfx::Size& viewport) {
@@ -421,46 +401,21 @@ void Surface::Commit() {
   TRACE_EVENT0("exo", "Surface::Commit");
 
   needs_commit_surface_hierarchy_ = true;
-
-  if (state_ != pending_state_)
-    has_pending_layer_changes_ = true;
-
-  if (has_pending_contents_) {
-    if (pending_buffer_.buffer()) {
-      if (current_resource_.size != pending_buffer_.buffer()->GetSize())
-        has_pending_layer_changes_ = true;
-      // Whether layer fills bounds opaquely or not might have changed.
-      if (current_resource_has_alpha_ !=
-          FormatHasAlpha(pending_buffer_.buffer()->GetFormat()))
-        has_pending_layer_changes_ = true;
-    } else if (!current_resource_.size.IsEmpty()) {
-      has_pending_layer_changes_ = true;
-    }
-  }
-
-  if (delegate_) {
+  if (delegate_)
     delegate_->OnSurfaceCommit();
-  } else {
-    CommitSurfaceHierarchy();
-  }
-
-  if (current_begin_frame_ack_.sequence_number !=
-      cc::BeginFrameArgs::kInvalidFrameNumber) {
-    if (!current_begin_frame_ack_.has_damage) {
-      layer_tree_frame_sink_holder_->frame_sink()->DidNotProduceFrame(
-          current_begin_frame_ack_);
-    }
-    current_begin_frame_ack_.sequence_number =
-        cc::BeginFrameArgs::kInvalidFrameNumber;
-    if (begin_frame_source_)
-      begin_frame_source_->DidFinishFrame(this);
-  }
 }
 
-void Surface::CommitSurfaceHierarchy() {
-  DCHECK(needs_commit_surface_hierarchy_);
+void Surface::CommitSurfaceHierarchy(
+    const gfx::Point& origin,
+    FrameType frame_type,
+    LayerTreeFrameSinkHolder* frame_sink_holder,
+    cc::CompositorFrame* frame,
+    std::list<FrameCallback>* frame_callbacks,
+    std::list<PresentationCallback>* presentation_callbacks) {
+  bool needs_commit =
+      frame_type == FRAME_TYPE_COMMIT && needs_commit_surface_hierarchy_;
+  if (needs_commit) {
     needs_commit_surface_hierarchy_ = false;
-  has_pending_layer_changes_ = false;
 
     state_ = pending_state_;
     pending_state_.only_visible_on_secure_output = false;
@@ -471,47 +426,27 @@ void Surface::CommitSurfaceHierarchy() {
 
       current_buffer_ = std::move(pending_buffer_);
 
-    UpdateResource(true);
+      UpdateResource(frame_sink_holder, true);
     }
 
-  // Move pending frame callbacks to the end of frame_callbacks_.
-  frame_callbacks_.splice(frame_callbacks_.end(), pending_frame_callbacks_);
+    // Move pending frame callbacks to the end of frame_callbacks.
+    frame_callbacks->splice(frame_callbacks->end(), pending_frame_callbacks_);
 
-  // Move pending presentation callbacks to the end of presentation_callbacks_.
-  presentation_callbacks_.splice(presentation_callbacks_.end(),
+    // Move pending presentation callbacks to the end of presentation_callbacks.
+    presentation_callbacks->splice(presentation_callbacks->end(),
                                    pending_presentation_callbacks_);
 
-  UpdateSurface(false);
-
-  window_->layer()->SetFillsBoundsOpaquely(
-      !current_resource_has_alpha_ || state_.blend_mode == SkBlendMode::kSrc ||
-      state_.opaque_region.contains(
-          gfx::RectToSkIRect(gfx::Rect(content_size_))));
-
-  // Reset damage.
-  pending_damage_.setEmpty();
-  DCHECK(!current_resource_.id ||
-         layer_tree_frame_sink_holder_->HasReleaseCallbackForResource(
-             current_resource_.id));
+    UpdateContentSize();
 
     // Synchronize window hierarchy. This will position and update the stacking
-  // order of all sub-surfaces after committing all pending state of sub-surface
-  // descendants.
+    // order of all sub-surfaces after committing all pending state of
+    // sub-surface descendants.
+    if (sub_surfaces_changed_) {
+      sub_surfaces_.clear();
       aura::Window* stacking_target = nullptr;
-  for (auto& sub_surface_entry : pending_sub_surfaces_) {
+      for (const auto& sub_surface_entry : pending_sub_surfaces_) {
         Surface* sub_surface = sub_surface_entry.first;
-
-    // Synchronsouly commit all pending state of the sub-surface and its
-    // decendents.
-    if (sub_surface->needs_commit_surface_hierarchy())
-      sub_surface->CommitSurfaceHierarchy();
-
-    // Enable/disable sub-surface based on if it has contents.
-    if (sub_surface->has_contents())
-      sub_surface->window()->Show();
-    else
-      sub_surface->window()->Hide();
-
+        sub_surfaces_.push_back(sub_surface_entry);
         // Move sub-surface to its new position in the stack.
         if (stacking_target)
           window_->StackChildAbove(sub_surface->window(), stacking_target);
@@ -520,9 +455,33 @@ void Surface::CommitSurfaceHierarchy() {
         stacking_target = sub_surface->window();
 
         // Update sub-surface position relative to surface origin.
-    sub_surface->window()->SetBounds(
-        gfx::Rect(sub_surface_entry.second, sub_surface->content_size_));
+        sub_surface->window()->SetBounds(gfx::Rect(
+            sub_surface_entry.second, sub_surface->window()->bounds().size()));
       }
+      sub_surfaces_changed_ = false;
+    }
+  }
+
+  // The top most sub-surface is at the front of the RenderPass's quad_list,
+  // so we need composite sub-surface in reversed order.
+  for (const auto& sub_surface_entry : base::Reversed(sub_surfaces_)) {
+    auto* sub_surface = sub_surface_entry.first;
+    // Synchronsouly commit all pending state of the sub-surface and its
+    // decendents.
+    sub_surface->CommitSurfaceHierarchy(
+        origin + sub_surface_entry.second.OffsetFromOrigin(), frame_type,
+        frame_sink_holder, frame, frame_callbacks, presentation_callbacks);
+  }
+
+  AppendContentsToFrame(origin, frame_type, frame);
+
+  // Reset damage.
+  if (needs_commit)
+    pending_damage_.setEmpty();
+
+  DCHECK(
+      !current_resource_.id ||
+      frame_sink_holder->HasReleaseCallbackForResource(current_resource_.id));
 }
 
 bool Surface::IsSynchronized() const {
@@ -599,49 +558,6 @@ std::unique_ptr<base::trace_event::TracedValue> Surface::AsTracedValue() const {
   return value;
 }
 
-void Surface::DidReceiveCompositorFrameAck() {
-  active_frame_callbacks_.splice(active_frame_callbacks_.end(),
-                                 frame_callbacks_);
-  swapping_presentation_callbacks_.splice(
-      swapping_presentation_callbacks_.end(), presentation_callbacks_);
-  UpdateNeedsBeginFrame();
-}
-
-void Surface::SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source) {
-  if (needs_begin_frame_) {
-    DCHECK(begin_frame_source_);
-    begin_frame_source_->RemoveObserver(this);
-    needs_begin_frame_ = false;
-  }
-  begin_frame_source_ = begin_frame_source;
-  UpdateNeedsBeginFrame();
-}
-
-void Surface::UpdateNeedsBeginFrame() {
-  if (!begin_frame_source_)
-    return;
-
-  bool needs_begin_frame = !active_frame_callbacks_.empty();
-  if (needs_begin_frame == needs_begin_frame_)
-    return;
-
-  needs_begin_frame_ = needs_begin_frame;
-  if (needs_begin_frame_)
-    begin_frame_source_->AddObserver(this);
-  else
-    begin_frame_source_->RemoveObserver(this);
-}
-
-bool Surface::OnBeginFrameDerivedImpl(const cc::BeginFrameArgs& args) {
-  current_begin_frame_ack_ =
-      cc::BeginFrameAck(args.source_id, args.sequence_number, false);
-  while (!active_frame_callbacks_.empty()) {
-    active_frame_callbacks_.front().Run(args.frame_time);
-    active_frame_callbacks_.pop_front();
-  }
-  return true;
-}
-
 bool Surface::IsStylusOnly() {
   return window_->GetProperty(kStylusOnlyKey);
 }
@@ -650,48 +566,17 @@ void Surface::SetStylusOnly() {
   window_->SetProperty(kStylusOnlyKey, true);
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// ui::ContextFactoryObserver overrides:
-
-void Surface::OnLostResources() {
-  if (!window_->GetSurfaceId().is_valid())
-    return;
-  UpdateResource(false);
-  UpdateSurface(true);
-}
-
-////////////////////////////////////////////////////////////////////////////////
-// aura::WindowObserver overrides:
-
-void Surface::OnWindowAddedToRootWindow(aura::Window* window) {
-  window->layer()->GetCompositor()->vsync_manager()->AddObserver(this);
-}
-
-void Surface::OnWindowRemovingFromRootWindow(aura::Window* window,
-                                             aura::Window* new_root) {
-  window->layer()->GetCompositor()->vsync_manager()->RemoveObserver(this);
+void Surface::RecreateResources(LayerTreeFrameSinkHolder* frame_sink_holder) {
+  UpdateResource(frame_sink_holder, false);
+  for (const auto& sub_surface : sub_surfaces_)
+    sub_surface.first->RecreateResources(frame_sink_holder);
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// ui::CompositorVSyncManager::Observer overrides:
-
-void Surface::OnUpdateVSyncParameters(base::TimeTicks timebase,
-                                      base::TimeDelta interval) {
-  // Use current time if platform doesn't provide an accurate timebase.
-  if (timebase.is_null())
-    timebase = base::TimeTicks::Now();
-
-  while (!swapped_presentation_callbacks_.empty()) {
-    swapped_presentation_callbacks_.front().Run(timebase, interval);
-    swapped_presentation_callbacks_.pop_front();
-  }
-
-  // VSync parameters updates are generated at the start of a new swap. Move
-  // the swapping presentation callbacks to swapped callbacks so they fire
-  // at the next VSync parameters update as that will contain the presentation
-  // time for the previous frame.
-  swapped_presentation_callbacks_.splice(swapped_presentation_callbacks_.end(),
-                                         swapping_presentation_callbacks_);
+bool Surface::FillsBoundsOpaquely() const {
+  return !current_resource_has_alpha_ ||
+         state_.blend_mode == SkBlendMode::kSrc ||
+         state_.opaque_region.contains(
+             gfx::RectToSkIRect(gfx::Rect(content_size_)));
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -741,12 +626,11 @@ void Surface::BufferAttachment::Reset(base::WeakPtr<Buffer> buffer) {
   buffer_ = buffer;
 }
 
-void Surface::UpdateResource(bool client_usage) {
+void Surface::UpdateResource(LayerTreeFrameSinkHolder* frame_sink_holder,
+                             bool client_usage) {
   if (current_buffer_.buffer() &&
       current_buffer_.buffer()->ProduceTransferableResource(
-          layer_tree_frame_sink_holder_.get(),
-          layer_tree_frame_sink_holder_->AllocateResourceId(),
-          state_.only_visible_on_secure_output, client_usage,
+          frame_sink_holder, state_.only_visible_on_secure_output, client_usage,
           &current_resource_)) {
     current_resource_has_alpha_ =
         FormatHasAlpha(current_buffer_.buffer()->GetFormat());
@@ -757,36 +641,39 @@ void Surface::UpdateResource(bool client_usage) {
   }
 }
 
-void Surface::UpdateSurface(bool full_damage) {
-  gfx::Size buffer_size = current_resource_.size;
-  gfx::SizeF scaled_buffer_size(
-      gfx::ScaleSize(gfx::SizeF(buffer_size), 1.0f / state_.buffer_scale));
-
-  gfx::Size layer_size;  // Size of the output layer, in DIP.
-  if (!state_.viewport.IsEmpty()) {
-    layer_size = state_.viewport;
-  } else if (!state_.crop.IsEmpty()) {
-    DLOG_IF(WARNING, !gfx::IsExpressibleAsInt(state_.crop.width()) ||
-                         !gfx::IsExpressibleAsInt(state_.crop.height()))
-        << "Crop rectangle size (" << state_.crop.size().ToString()
-        << ") most be expressible using integers when viewport is not set";
-    layer_size = gfx::ToCeiledSize(state_.crop.size());
-  } else {
-    layer_size = gfx::ToCeiledSize(scaled_buffer_size);
+void Surface::AppendContentsToFrame(const gfx::Point& origin,
+                                    FrameType frame_type,
+                                    cc::CompositorFrame* frame) {
+  const std::unique_ptr<cc::RenderPass>& render_pass =
+      frame->render_pass_list.back();
+  gfx::Rect output_rect = gfx::Rect(origin, content_size_);
+  gfx::Rect quad_rect = output_rect;
+  gfx::Rect damage_rect;
+  switch (frame_type) {
+    case FRAME_TYPE_COMMIT:
+      // pending_damage_ is in Surface coordinates.
+      damage_rect = gfx::SkIRectToRect(pending_damage_.getBounds());
+      damage_rect.set_origin(origin);
+      damage_rect.Intersect(output_rect);
+      break;
+    case FRAME_TYPE_RECREATED_RESOURCES:
+      damage_rect = output_rect;
+      break;
   }
 
-  content_size_ = layer_size;
-  // We need update window_'s bounds with content size, because the
-  // LayerTreeFrameSink may not update the window's size base the size of
-  // the lastest submitted CompositorFrame.
-  window_->SetBounds(gfx::Rect(window_->bounds().origin(), content_size_));
-  // TODO(jbauman): Figure out how this interacts with the pixel size of
-  // CopyOutputRequests on the layer.
-  gfx::Size contents_surface_size = layer_size;
+  render_pass->damage_rect.Union(damage_rect);
+  cc::SharedQuadState* quad_state =
+      render_pass->CreateAndAppendSharedQuadState();
+  quad_state->quad_layer_rect = gfx::Rect(content_size_);
+  quad_state->visible_quad_layer_rect = quad_rect;
+  quad_state->opacity = state_.alpha;
 
+  if (current_resource_.id) {
     gfx::PointF uv_top_left(0.f, 0.f);
     gfx::PointF uv_bottom_right(1.f, 1.f);
     if (!state_.crop.IsEmpty()) {
+      gfx::SizeF scaled_buffer_size(gfx::ScaleSize(
+          gfx::SizeF(current_resource_.size), 1.0f / state_.buffer_scale));
       uv_top_left = state_.crop.origin();
 
       uv_top_left.Scale(1.f / scaled_buffer_size.width(),
@@ -795,42 +682,6 @@ void Surface::UpdateSurface(bool full_damage) {
       uv_bottom_right.Scale(1.f / scaled_buffer_size.width(),
                             1.f / scaled_buffer_size.height());
     }
-
-  gfx::Rect damage_rect;
-  gfx::Rect output_rect = gfx::Rect(contents_surface_size);
-  if (full_damage) {
-    damage_rect = output_rect;
-  } else {
-    // pending_damage_ is in Surface coordinates.
-    damage_rect = gfx::SkIRectToRect(pending_damage_.getBounds());
-    damage_rect.Intersect(output_rect);
-  }
-
-  const int kRenderPassId = 1;
-  std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create();
-  render_pass->SetNew(kRenderPassId, output_rect, damage_rect,
-                      gfx::Transform());
-
-  gfx::Rect quad_rect = output_rect;
-  cc::SharedQuadState* quad_state =
-      render_pass->CreateAndAppendSharedQuadState();
-  quad_state->quad_layer_rect = gfx::Rect(contents_surface_size);
-  quad_state->visible_quad_layer_rect = quad_rect;
-  quad_state->opacity = state_.alpha;
-
-  cc::CompositorFrame frame;
-  // If we commit while we don't have an active BeginFrame, we acknowledge a
-  // manual one.
-  if (current_begin_frame_ack_.sequence_number ==
-      cc::BeginFrameArgs::kInvalidFrameNumber) {
-    current_begin_frame_ack_ = cc::BeginFrameAck::CreateManualAckWithDamage();
-  } else {
-    current_begin_frame_ack_.has_damage = true;
-  }
-  frame.metadata.begin_frame_ack = current_begin_frame_ack_;
-  frame.metadata.device_scale_factor = device_scale_factor_;
-
-  if (current_resource_.id) {
     // Texture quad is only needed if buffer is not fully transparent.
     if (state_.alpha) {
       cc::TextureDrawQuad* texture_quad =
@@ -845,23 +696,46 @@ void Surface::UpdateSurface(bool full_damage) {
         opaque_rect = gfx::SkIRectToRect(state_.opaque_region.getBounds());
       }
 
-      texture_quad->SetNew(quad_state, quad_rect, opaque_rect, quad_rect,
-                           current_resource_.id, true, uv_top_left,
-                           uv_bottom_right, SK_ColorTRANSPARENT, vertex_opacity,
-                           false, false, state_.only_visible_on_secure_output);
+      texture_quad->SetNew(
+          quad_state, quad_rect, opaque_rect, quad_rect, current_resource_.id,
+          true /* premultiplied_alpha */, uv_top_left, uv_bottom_right,
+          SK_ColorTRANSPARENT /* background_color */, vertex_opacity,
+          false /* y_flipped */, false /* nearest_neighbor */,
+          state_.only_visible_on_secure_output);
       if (current_resource_.is_overlay_candidate)
         texture_quad->set_resource_size_in_pixels(current_resource_.size);
-      frame.resource_list.push_back(current_resource_);
+      frame->resource_list.push_back(current_resource_);
     }
   } else {
     cc::SolidColorDrawQuad* solid_quad =
         render_pass->CreateAndAppendDrawQuad<cc::SolidColorDrawQuad>();
-    solid_quad->SetNew(quad_state, quad_rect, quad_rect, SK_ColorBLACK, false);
+    solid_quad->SetNew(quad_state, quad_rect, quad_rect, SK_ColorBLACK,
+                       false /* force_anti_aliasing_off */);
   }
+}
+
+void Surface::UpdateContentSize() {
+  gfx::Size buffer_size = current_resource_.size;
+  gfx::SizeF scaled_buffer_size(
+      gfx::ScaleSize(gfx::SizeF(buffer_size), 1.0f / state_.buffer_scale));
+  if (!state_.viewport.IsEmpty()) {
+    content_size_ = state_.viewport;
+  } else if (!state_.crop.IsEmpty()) {
+    DLOG_IF(WARNING, !gfx::IsExpressibleAsInt(state_.crop.width()) ||
+                         !gfx::IsExpressibleAsInt(state_.crop.height()))
+        << "Crop rectangle size (" << state_.crop.size().ToString()
+        << ") most be expressible using integers when viewport is not set";
+    content_size_ = gfx::ToCeiledSize(state_.crop.size());
+  } else {
+    content_size_ = gfx::ToCeiledSize(scaled_buffer_size);
+  }
+  window_->SetBounds(gfx::Rect(window_->bounds().origin(), content_size_));
 
-  frame.render_pass_list.push_back(std::move(render_pass));
-  layer_tree_frame_sink_holder_->frame_sink()->SubmitCompositorFrame(
-      std::move(frame));
+  // Enable/disable sub-surface based on if it has contents.
+  if (has_contents())
+    window_->Show();
+  else
+    window_->Hide();
 }
 
 }  // namespace exo

components/exo/surface.h

@@ -15,13 +15,10 @@
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "cc/resources/transferable_resource.h"
-#include "cc/scheduler/begin_frame_source.h"
 #include "components/exo/layer_tree_frame_sink_holder.h"
 #include "third_party/skia/include/core/SkBlendMode.h"
 #include "third_party/skia/include/core/SkRegion.h"
 #include "ui/aura/window.h"
-#include "ui/aura/window_observer.h"
-#include "ui/compositor/compositor_vsync_manager.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/native_widget_types.h"
 
@@ -31,12 +28,17 @@ class TracedValue;
 }
 }
 
+namespace cc {
+class CompositorFrame;
+}
+
 namespace gfx {
 class Path;
 }
 
 namespace exo {
 class Buffer;
+class LayerTreeFrameSinkHolder;
 class Pointer;
 class SurfaceDelegate;
 class SurfaceObserver;
@@ -52,28 +54,18 @@ using CursorProvider = Pointer;
 
 // This class represents a rectangular area that is displayed on the screen.
 // It has a location, size and pixel contents.
-class Surface : public ui::ContextFactoryObserver,
-                public aura::WindowObserver,
-                public ui::PropertyHandler,
-                public ui::CompositorVSyncManager::Observer,
-                public cc::BeginFrameObserverBase {
+class Surface : public ui::PropertyHandler {
  public:
   using PropertyDeallocator = void (*)(int64_t value);
 
   Surface();
-  ~Surface() override;
+  ~Surface();
 
   // Type-checking downcast routine.
   static Surface* AsSurface(const aura::Window* window);
 
   aura::Window* window() { return window_.get(); }
 
-  viz::SurfaceId GetSurfaceId() const;
-
-  LayerTreeFrameSinkHolder* layer_tree_frame_sink_holder() {
-    return layer_tree_frame_sink_holder_.get();
-  }
-
   // Set a buffer as the content of this surface. A buffer can only be attached
   // to one surface at a time.
   void Attach(Buffer* buffer);
@@ -144,7 +136,17 @@ class Surface : public ui::ContextFactoryObserver,
   // This will synchronously commit all pending state of the surface and its
   // descendants by recursively calling CommitSurfaceHierarchy() for each
   // sub-surface with pending state.
-  void CommitSurfaceHierarchy();
+  enum FrameType {
+    FRAME_TYPE_COMMIT,
+    FRAME_TYPE_RECREATED_RESOURCES,
+  };
+  void CommitSurfaceHierarchy(
+      const gfx::Point& origin,
+      FrameType frame_type,
+      LayerTreeFrameSinkHolder* frame_sink_holder,
+      cc::CompositorFrame* frame,
+      std::list<FrameCallback>* frame_callbacks,
+      std::list<PresentationCallback>* presentation_callbacks);
 
   // Returns true if surface is in synchronized mode.
   bool IsSynchronized() const;
@@ -186,10 +188,6 @@ class Surface : public ui::ContextFactoryObserver,
   // Returns a trace value representing the state of the surface.
   std::unique_ptr<base::trace_event::TracedValue> AsTracedValue() const;
 
-  // Call this to indicate that the previous CompositorFrame is processed and
-  // the surface is being scheduled for a draw.
-  void DidReceiveCompositorFrameAck();
-
   // Called when the begin frame source has changed.
   void SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source);
 
@@ -202,26 +200,16 @@ class Surface : public ui::ContextFactoryObserver,
   // Enables 'stylus-only' mode for the associated window.
   void SetStylusOnly();
 
-  // Overridden from ui::ContextFactoryObserver:
-  void OnLostResources() override;
-
-  // Overridden from aura::WindowObserver:
-  void OnWindowAddedToRootWindow(aura::Window* window) override;
-  void OnWindowRemovingFromRootWindow(aura::Window* window,
-                                      aura::Window* new_root) override;
+  // Recreates resources for the surface and sub surfaces.
+  void RecreateResources(LayerTreeFrameSinkHolder* frame_sink_holder);
 
-  // Overridden from ui::CompositorVSyncManager::Observer:
-  void OnUpdateVSyncParameters(base::TimeTicks timebase,
-                               base::TimeDelta interval) override;
+  // Returns true if the surface's bounds should be filled opaquely.
+  bool FillsBoundsOpaquely() const;
 
   bool HasPendingDamageForTesting(const gfx::Rect& damage) const {
     return pending_damage_.contains(gfx::RectToSkIRect(damage));
   }
 
-  // Overridden from cc::BeginFrameObserverBase:
-  bool OnBeginFrameDerivedImpl(const cc::BeginFrameArgs& args) override;
-  void OnBeginFrameSourcePausedChanged(bool paused) override {}
-
  private:
   struct State {
     State();
@@ -269,14 +257,16 @@ class Surface : public ui::ContextFactoryObserver,
   // contents of the attached buffer (or id 0, if no buffer is attached).
   // UpdateSurface must be called afterwards to ensure the release callback
   // will be called.
-  void UpdateResource(bool client_usage);
+  void UpdateResource(LayerTreeFrameSinkHolder* frame_sink_holder,
+                      bool client_usage);
 
-  // Updates the current Surface with a new frame referring to the resource in
-  // current_resource_.
-  void UpdateSurface(bool full_damage);
+  // Puts the current surface into a draw quad, and appends the draw quads into
+  // the |frame|.
+  void AppendContentsToFrame(const gfx::Point& origin,
+                             FrameType frame_type,
+                             cc::CompositorFrame* frame);
 
-  // Adds/Removes begin frame observer based on state.
-  void UpdateNeedsBeginFrame();
+  void UpdateContentSize();
 
   // This returns true when the surface has some contents assigned to it.
   bool has_contents() const { return !!current_buffer_.buffer(); }
@@ -284,10 +274,8 @@ class Surface : public ui::ContextFactoryObserver,
   // This window has the layer which contains the Surface contents.
   std::unique_ptr<aura::Window> window_;
 
-  // This is true if it's possible that the layer properties (size, opacity,
-  // etc.) may have been modified since the last commit. Attaching a new
-  // buffer with the same size as the old shouldn't set this to true.
-  bool has_pending_layer_changes_ = true;
+  // This true, if sub_surfaces_ has changes (order, position, etc).
+  bool sub_surfaces_changed_ = false;
 
   // This is the size of the last committed contents.
   gfx::Size content_size_;
@@ -302,8 +290,6 @@ class Surface : public ui::ContextFactoryObserver,
   // The device scale factor sent in CompositorFrames.
   float device_scale_factor_ = 1.0f;
 
-  std::unique_ptr<LayerTreeFrameSinkHolder> layer_tree_frame_sink_holder_;
-
   // The damage region to schedule paint for when Commit() is called.
   SkRegion pending_damage_;
 
@@ -313,8 +299,6 @@ class Surface : public ui::ContextFactoryObserver,
   // |active_frame_callbacks_| when the effect of the Commit() is scheduled to
   // be drawn. They fire at the first begin frame notification after this.
   std::list<FrameCallback> pending_frame_callbacks_;
-  std::list<FrameCallback> frame_callbacks_;
-  std::list<FrameCallback> active_frame_callbacks_;
 
   // These lists contains the callbacks to notify the client when surface
   // contents have been presented. These callbacks move to
@@ -324,9 +308,6 @@ class Surface : public ui::ContextFactoryObserver,
   // after receiving VSync parameters update for the previous frame. They fire
   // at the next VSync parameters update after that.
   std::list<PresentationCallback> pending_presentation_callbacks_;
-  std::list<PresentationCallback> presentation_callbacks_;
-  std::list<PresentationCallback> swapping_presentation_callbacks_;
-  std::list<PresentationCallback> swapped_presentation_callbacks_;
 
   // This is the state that has yet to be committed.
   State pending_state_;
@@ -340,6 +321,7 @@ class Surface : public ui::ContextFactoryObserver,
   using SubSurfaceEntry = std::pair<Surface*, gfx::Point>;
   using SubSurfaceEntryList = std::list<SubSurfaceEntry>;
   SubSurfaceEntryList pending_sub_surfaces_;
+  SubSurfaceEntryList sub_surfaces_;
 
   // The buffer that is currently set as content of surface.
   BufferAttachment current_buffer_;
@@ -369,11 +351,6 @@ class Surface : public ui::ContextFactoryObserver,
   // Surface observer list. Surface does not own the observers.
   base::ObserverList<SurfaceObserver, true> observers_;
 
-  // The begin frame source being observed.
-  cc::BeginFrameSource* begin_frame_source_ = nullptr;
-  bool needs_begin_frame_ = false;
-  cc::BeginFrameAck current_begin_frame_ack_;
-
   DISALLOW_COPY_AND_ASSIGN(Surface);
 };
 

components/exo/surface_tree_host.cc

@@ -8,7 +8,11 @@
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "cc/output/compositor_frame.h"
+#include "cc/output/layer_tree_frame_sink.h"
+#include "components/exo/layer_tree_frame_sink_holder.h"
 #include "components/exo/surface.h"
+#include "ui/aura/env.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_delegate.h"
 #include "ui/aura/window_event_dispatcher.h"
@@ -73,15 +77,20 @@ SurfaceTreeHost::SurfaceTreeHost(const std::string& window_name,
   host_window_ = base::MakeUnique<aura::Window>(window_delegate);
   host_window_->SetType(aura::client::WINDOW_TYPE_CONTROL);
   host_window_->SetName(window_name);
-  host_window_->Init(ui::LAYER_NOT_DRAWN);
+  host_window_->Init(ui::LAYER_SOLID_COLOR);
   host_window_->set_owned_by_parent(false);
   host_window_->SetEventTargeter(base::MakeUnique<CustomWindowTargeter>(this));
+  layer_tree_frame_sink_holder_ = base::MakeUnique<LayerTreeFrameSinkHolder>(
+      this, host_window_->CreateLayerTreeFrameSink());
+  aura::Env::GetInstance()->context_factory()->AddObserver(this);
 }
 
 SurfaceTreeHost::~SurfaceTreeHost() {
-  if (root_surface_) {
-    root_surface_->window()->Hide();
-    root_surface_->SetSurfaceDelegate(nullptr);
+  aura::Env::GetInstance()->context_factory()->RemoveObserver(this);
+  SetRootSurface(nullptr);
+  if (host_window_->layer()->GetCompositor()) {
+    host_window_->layer()->GetCompositor()->vsync_manager()->RemoveObserver(
+        this);
   }
 }
 
@@ -96,6 +105,28 @@ void SurfaceTreeHost::SetRootSurface(Surface* root_surface) {
         gfx::Rect(host_window_->bounds().origin(), gfx::Size()));
     root_surface_->SetSurfaceDelegate(nullptr);
     root_surface_ = nullptr;
+
+    active_frame_callbacks_.splice(active_frame_callbacks_.end(),
+                                   frame_callbacks_);
+    // Call all frame callbacks with a null frame time to indicate that they
+    // have been cancelled.
+    while (!active_frame_callbacks_.empty()) {
+      active_frame_callbacks_.front().Run(base::TimeTicks());
+      active_frame_callbacks_.pop_front();
+    }
+
+    swapping_presentation_callbacks_.splice(
+        swapping_presentation_callbacks_.end(), presentation_callbacks_);
+    swapped_presentation_callbacks_.splice(
+        swapped_presentation_callbacks_.end(),
+        swapping_presentation_callbacks_);
+    // Call all presentation callbacks with a null presentation time to indicate
+    // that they have been cancelled.
+    while (!swapped_presentation_callbacks_.empty()) {
+      swapped_presentation_callbacks_.front().Run(base::TimeTicks(),
+                                                  base::TimeDelta());
+      swapped_presentation_callbacks_.pop_front();
+    }
   }
 
   if (root_surface) {
@@ -125,14 +156,43 @@ gfx::NativeCursor SurfaceTreeHost::GetCursor(const gfx::Point& point) const {
   return root_surface_ ? root_surface_->GetCursor() : ui::CursorType::kNull;
 }
 
+void SurfaceTreeHost::DidReceiveCompositorFrameAck() {
+  active_frame_callbacks_.splice(active_frame_callbacks_.end(),
+                                 frame_callbacks_);
+  swapping_presentation_callbacks_.splice(
+      swapping_presentation_callbacks_.end(), presentation_callbacks_);
+  UpdateNeedsBeginFrame();
+}
+
+void SurfaceTreeHost::SetBeginFrameSource(
+    cc::BeginFrameSource* begin_frame_source) {
+  if (needs_begin_frame_) {
+    DCHECK(begin_frame_source_);
+    begin_frame_source_->RemoveObserver(this);
+    needs_begin_frame_ = false;
+  }
+  begin_frame_source_ = begin_frame_source;
+  UpdateNeedsBeginFrame();
+}
+
+void SurfaceTreeHost::UpdateNeedsBeginFrame() {
+  if (!begin_frame_source_)
+    return;
+  bool needs_begin_frame = !active_frame_callbacks_.empty();
+  if (needs_begin_frame == needs_begin_frame_)
+    return;
+  needs_begin_frame_ = needs_begin_frame;
+  if (needs_begin_frame_)
+    begin_frame_source_->AddObserver(this);
+  else
+    begin_frame_source_->RemoveObserver(this);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // SurfaceDelegate overrides:
 
 void SurfaceTreeHost::OnSurfaceCommit() {
-  DCHECK(root_surface_);
-  root_surface_->CommitSurfaceHierarchy();
-  host_window_->SetBounds(gfx::Rect(host_window_->bounds().origin(),
-                                    root_surface_->content_size()));
+  SubmitCompositorFrame(Surface::FRAME_TYPE_COMMIT);
 }
 
 bool SurfaceTreeHost::IsSurfaceSynchronized() const {
@@ -141,4 +201,111 @@ bool SurfaceTreeHost::IsSurfaceSynchronized() const {
   return false;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// aura::WindowObserver overrides:
+
+void SurfaceTreeHost::OnWindowAddedToRootWindow(aura::Window* window) {
+  DCHECK_EQ(window, host_window());
+  window->layer()->GetCompositor()->vsync_manager()->AddObserver(this);
+}
+
+void SurfaceTreeHost::OnWindowRemovingFromRootWindow(aura::Window* window,
+                                                     aura::Window* new_root) {
+  DCHECK_EQ(window, host_window());
+  window->layer()->GetCompositor()->vsync_manager()->RemoveObserver(this);
+}
+
+void SurfaceTreeHost::OnWindowDestroying(aura::Window* window) {
+  DCHECK_EQ(window, host_window());
+  window->RemoveObserver(this);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// cc::BeginFrameObserverBase overrides:
+
+bool SurfaceTreeHost::OnBeginFrameDerivedImpl(const cc::BeginFrameArgs& args) {
+  current_begin_frame_ack_ =
+      cc::BeginFrameAck(args.source_id, args.sequence_number, false);
+  while (!active_frame_callbacks_.empty()) {
+    active_frame_callbacks_.front().Run(args.frame_time);
+    active_frame_callbacks_.pop_front();
+  }
+  return true;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// ui::CompositorVSyncManager::Observer overrides:
+
+void SurfaceTreeHost::OnUpdateVSyncParameters(base::TimeTicks timebase,
+                                              base::TimeDelta interval) {
+  // Use current time if platform doesn't provide an accurate timebase.
+  if (timebase.is_null())
+    timebase = base::TimeTicks::Now();
+  while (!swapped_presentation_callbacks_.empty()) {
+    swapped_presentation_callbacks_.front().Run(timebase, interval);
+    swapped_presentation_callbacks_.pop_front();
+  }
+  // VSync parameters updates are generated at the start of a new swap. Move
+  // the swapping presentation callbacks to swapped callbacks so they fire
+  // at the next VSync parameters update as that will contain the presentation
+  // time for the previous frame.
+  swapped_presentation_callbacks_.splice(swapped_presentation_callbacks_.end(),
+                                         swapping_presentation_callbacks_);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// ui::ContextFactoryObserver overrides:
+
+void SurfaceTreeHost::OnLostResources() {
+  if (!host_window_->GetSurfaceId().is_valid())
+    return;
+  root_surface_->RecreateResources(layer_tree_frame_sink_holder_.get());
+  SubmitCompositorFrame(Surface::FRAME_TYPE_RECREATED_RESOURCES);
+}
+
+void SurfaceTreeHost::SubmitCompositorFrame(Surface::FrameType frame_type) {
+  DCHECK(root_surface_);
+  cc::CompositorFrame frame;
+  // If we commit while we don't have an active BeginFrame, we acknowledge a
+  // manual one.
+  if (current_begin_frame_ack_.sequence_number ==
+      cc::BeginFrameArgs::kInvalidFrameNumber) {
+    current_begin_frame_ack_ = cc::BeginFrameAck::CreateManualAckWithDamage();
+  } else {
+    current_begin_frame_ack_.has_damage = true;
+  }
+
+  frame.metadata.begin_frame_ack = current_begin_frame_ack_;
+  frame.metadata.device_scale_factor =
+      host_window_->layer()->device_scale_factor();
+  const int kRenderPassId = 1;
+  std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create();
+  render_pass->SetNew(kRenderPassId, gfx::Rect(), gfx::Rect(),
+                      gfx::Transform());
+  frame.render_pass_list.push_back(std::move(render_pass));
+  root_surface_->CommitSurfaceHierarchy(
+      gfx::Point(), frame_type, layer_tree_frame_sink_holder_.get(), &frame,
+      &frame_callbacks_, &presentation_callbacks_);
+  frame.render_pass_list.back()->output_rect =
+      gfx::Rect(root_surface_->content_size());
+  layer_tree_frame_sink_holder_->frame_sink()->SubmitCompositorFrame(
+      std::move(frame));
+  host_window_->SetBounds(gfx::Rect(host_window_->bounds().origin(),
+                                    root_surface_->content_size()));
+  host_window_->layer()->SetFillsBoundsOpaquely(
+      root_surface_->FillsBoundsOpaquely());
+
+  if (current_begin_frame_ack_.sequence_number !=
+      cc::BeginFrameArgs::kInvalidFrameNumber) {
+    if (!current_begin_frame_ack_.has_damage) {
+      layer_tree_frame_sink_holder_->frame_sink()->DidNotProduceFrame(
+          current_begin_frame_ack_);
+    }
+    current_begin_frame_ack_.sequence_number =
+        cc::BeginFrameArgs::kInvalidFrameNumber;
+    if (begin_frame_source_)
+      begin_frame_source_->DidFinishFrame(this);
+  }
+}
+
 }  // namespace exo

components/exo/surface_tree_host.h

@@ -8,8 +8,12 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "cc/scheduler/begin_frame_source.h"
+#include "components/exo/layer_tree_frame_sink_holder.h"
 #include "components/exo/surface.h"
 #include "components/exo/surface_delegate.h"
+#include "ui/aura/window_observer.h"
+#include "ui/compositor/compositor_vsync_manager.h"
 #include "ui/gfx/geometry/rect.h"
 
 namespace aura {
@@ -17,15 +21,24 @@ class Window;
 class WindowDelegate;
 }  // namespace aura
 
+namespace cc {
+class BeginFrameSource;
+}  // namespace cc
+
 namespace gfx {
 class Path;
 }  // namespace gfx
 
 namespace exo {
+class LayerTreeFrameSinkHolder;
 
 // This class provides functionality for hosting a surface tree. The surface
 // tree is hosted in the |host_window_|.
-class SurfaceTreeHost : public SurfaceDelegate {
+class SurfaceTreeHost : public SurfaceDelegate,
+                        public aura::WindowObserver,
+                        public cc::BeginFrameObserverBase,
+                        public ui::CompositorVSyncManager::Observer,
+                        public ui::ContextFactoryObserver {
  public:
   SurfaceTreeHost(const std::string& window_name,
                   aura::WindowDelegate* window_delegate);
@@ -48,19 +61,77 @@ class SurfaceTreeHost : public SurfaceDelegate {
   // registered then CursorType::kNull is returned.
   gfx::NativeCursor GetCursor(const gfx::Point& point) const;
 
+  // Call this to indicate that the previous CompositorFrame is processed and
+  // the surface is being scheduled for a draw.
+  void DidReceiveCompositorFrameAck();
+
+  // Called when the begin frame source has changed.
+  void SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source);
+
+  // Adds/Removes begin frame observer based on state.
+  void UpdateNeedsBeginFrame();
+
   aura::Window* host_window() { return host_window_.get(); }
   const aura::Window* host_window() const { return host_window_.get(); }
 
   Surface* root_surface() { return root_surface_; }
   const Surface* root_surface() const { return root_surface_; }
 
+  LayerTreeFrameSinkHolder* layer_tree_frame_sink_holder() {
+    return layer_tree_frame_sink_holder_.get();
+  }
+
   // Overridden from SurfaceDelegate:
   void OnSurfaceCommit() override;
   bool IsSurfaceSynchronized() const override;
 
+  // Overridden from aura::WindowObserver:
+  void OnWindowAddedToRootWindow(aura::Window* window) override;
+  void OnWindowRemovingFromRootWindow(aura::Window* window,
+                                      aura::Window* new_root) override;
+  void OnWindowDestroying(aura::Window* window) override;
+
+  // Overridden from cc::BeginFrameObserverBase:
+  bool OnBeginFrameDerivedImpl(const cc::BeginFrameArgs& args) override;
+  void OnBeginFrameSourcePausedChanged(bool paused) override {}
+
+  // Overridden from ui::CompositorVSyncManager::Observer:
+  void OnUpdateVSyncParameters(base::TimeTicks timebase,
+                               base::TimeDelta interval) override;
+
+  // Overridden from ui::ContextFactoryObserver:
+  void OnLostResources() override;
+
  private:
+  void SubmitCompositorFrame(Surface::FrameType frame_type);
+
   Surface* root_surface_ = nullptr;
   std::unique_ptr<aura::Window> host_window_;
+  std::unique_ptr<LayerTreeFrameSinkHolder> layer_tree_frame_sink_holder_;
+
+  // The begin frame source being observed.
+  cc::BeginFrameSource* begin_frame_source_ = nullptr;
+  bool needs_begin_frame_ = false;
+  cc::BeginFrameAck current_begin_frame_ack_;
+
+  // These lists contain the callbacks to notify the client when it is a good
+  // time to start producing a new frame. These callbacks move to
+  // |frame_callbacks_| when Commit() is called. Later they are moved to
+  // |active_frame_callbacks_| when the effect of the Commit() is scheduled to
+  // be drawn. They fire at the first begin frame notification after this.
+  std::list<Surface::FrameCallback> frame_callbacks_;
+  std::list<Surface::FrameCallback> active_frame_callbacks_;
+
+  // These lists contains the callbacks to notify the client when surface
+  // contents have been presented. These callbacks move to
+  // |presentation_callbacks_| when Commit() is called. Later they are moved to
+  // |swapping_presentation_callbacks_| when the effect of the Commit() is
+  // scheduled to be drawn and then moved to |swapped_presentation_callbacks_|
+  // after receiving VSync parameters update for the previous frame. They fire
+  // at the next VSync parameters update after that.
+  std::list<Surface::PresentationCallback> presentation_callbacks_;
+  std::list<Surface::PresentationCallback> swapping_presentation_callbacks_;
+  std::list<Surface::PresentationCallback> swapped_presentation_callbacks_;
 
   DISALLOW_COPY_AND_ASSIGN(SurfaceTreeHost);
 };

components/exo/surface_unittest.cc

@@ -9,11 +9,13 @@
 #include "cc/test/begin_frame_args_test.h"
 #include "cc/test/fake_external_begin_frame_source.h"
 #include "components/exo/buffer.h"
+#include "components/exo/shell_surface.h"
 #include "components/exo/surface.h"
 #include "components/exo/test/exo_test_base.h"
 #include "components/exo/test/exo_test_helper.h"
 #include "components/viz/service/frame_sinks/frame_sink_manager.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/khronos/GLES2/gl2.h"
 #include "ui/aura/env.h"
 #include "ui/compositor/layer_tree_owner.h"
 #include "ui/gfx/gpu_memory_buffer.h"
@@ -99,8 +101,8 @@ TEST_F(SurfaceTest, RequestFrameCallback) {
   EXPECT_TRUE(frame_time.is_null());
 }
 
-const cc::CompositorFrame& GetFrameFromSurface(Surface* surface) {
-  viz::SurfaceId surface_id = surface->GetSurfaceId();
+const cc::CompositorFrame& GetFrameFromSurface(ShellSurface* shell_surface) {
+  viz::SurfaceId surface_id = shell_surface->host_window()->GetSurfaceId();
   cc::SurfaceManager* surface_manager = aura::Env::GetInstance()
                                             ->context_factory_private()
                                             ->GetFrameSinkManager()
@@ -112,9 +114,10 @@ const cc::CompositorFrame& GetFrameFromSurface(Surface* surface) {
 
 TEST_F(SurfaceTest, SetOpaqueRegion) {
   gfx::Size buffer_size(1, 1);
-  std::unique_ptr<Buffer> buffer(
-      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
-  std::unique_ptr<Surface> surface(new Surface);
+  auto buffer = base::MakeUnique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+  auto surface = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(surface.get());
 
   // Attaching a buffer with alpha channel.
   surface->Attach(buffer.get());
@@ -126,7 +129,7 @@ TEST_F(SurfaceTest, SetOpaqueRegion) {
   RunAllPendingInMessageLoop();
 
   {
-    const cc::CompositorFrame& frame = GetFrameFromSurface(surface.get());
+    const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
     ASSERT_EQ(1u, frame.render_pass_list.size());
     ASSERT_EQ(1u, frame.render_pass_list.back()->quad_list.size());
     EXPECT_FALSE(frame.render_pass_list.back()
@@ -140,7 +143,7 @@ TEST_F(SurfaceTest, SetOpaqueRegion) {
   RunAllPendingInMessageLoop();
 
   {
-    const cc::CompositorFrame& frame = GetFrameFromSurface(surface.get());
+    const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
     ASSERT_EQ(1u, frame.render_pass_list.size());
     ASSERT_EQ(1u, frame.render_pass_list.back()->quad_list.size());
     EXPECT_TRUE(frame.render_pass_list.back()
@@ -159,7 +162,7 @@ TEST_F(SurfaceTest, SetOpaqueRegion) {
   RunAllPendingInMessageLoop();
 
   {
-    const cc::CompositorFrame& frame = GetFrameFromSurface(surface.get());
+    const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
     ASSERT_EQ(1u, frame.render_pass_list.size());
     ASSERT_EQ(1u, frame.render_pass_list.back()->quad_list.size());
     EXPECT_FALSE(frame.render_pass_list.back()
@@ -180,9 +183,10 @@ TEST_F(SurfaceTest, SetInputRegion) {
 
 TEST_F(SurfaceTest, SetBufferScale) {
   gfx::Size buffer_size(512, 512);
-  std::unique_ptr<Buffer> buffer(
-      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
-  std::unique_ptr<Surface> surface(new Surface);
+  auto buffer = base::MakeUnique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+  auto surface = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(surface.get());
 
   // This will update the bounds of the surface and take the buffer scale into
   // account.
@@ -200,9 +204,10 @@ TEST_F(SurfaceTest, SetBufferScale) {
 
 TEST_F(SurfaceTest, MirrorLayers) {
   gfx::Size buffer_size(512, 512);
-  std::unique_ptr<Buffer> buffer(
-      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
-  std::unique_ptr<Surface> surface(new Surface);
+  auto buffer = base::MakeUnique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+  auto surface = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(surface.get());
 
   surface->Attach(buffer.get());
   surface->Commit();
@@ -210,19 +215,20 @@ TEST_F(SurfaceTest, MirrorLayers) {
   EXPECT_EQ(buffer_size, surface->window()->bounds().size());
   EXPECT_EQ(buffer_size, surface->window()->layer()->bounds().size());
   std::unique_ptr<ui::LayerTreeOwner> old_layer_owner =
-      ::wm::MirrorLayers(surface->window(), false /* sync_bounds */);
+      ::wm::MirrorLayers(shell_surface->host_window(), false /* sync_bounds */);
   EXPECT_EQ(buffer_size, surface->window()->bounds().size());
   EXPECT_EQ(buffer_size, surface->window()->layer()->bounds().size());
   EXPECT_EQ(buffer_size, old_layer_owner->root()->bounds().size());
-  EXPECT_TRUE(surface->window()->layer()->has_external_content());
+  EXPECT_TRUE(shell_surface->host_window()->layer()->has_external_content());
   EXPECT_TRUE(old_layer_owner->root()->has_external_content());
 }
 
 TEST_F(SurfaceTest, SetViewport) {
   gfx::Size buffer_size(1, 1);
-  std::unique_ptr<Buffer> buffer(
-      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
-  std::unique_ptr<Surface> surface(new Surface);
+  auto buffer = base::MakeUnique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+  auto surface = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(surface.get());
 
   // This will update the bounds of the surface and take the viewport into
   // account.
@@ -244,9 +250,10 @@ TEST_F(SurfaceTest, SetViewport) {
 
 TEST_F(SurfaceTest, SetCrop) {
   gfx::Size buffer_size(16, 16);
-  std::unique_ptr<Buffer> buffer(
-      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
-  std::unique_ptr<Surface> surface(new Surface);
+  auto buffer = base::MakeUnique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+  auto surface = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(surface.get());
 
   surface->Attach(buffer.get());
   gfx::Size crop_size(12, 12);
@@ -259,16 +266,17 @@ TEST_F(SurfaceTest, SetCrop) {
 
 TEST_F(SurfaceTest, SetBlendMode) {
   gfx::Size buffer_size(1, 1);
-  std::unique_ptr<Buffer> buffer(
-      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
-  std::unique_ptr<Surface> surface(new Surface);
+  auto buffer = base::MakeUnique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+  auto surface = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(surface.get());
 
   surface->Attach(buffer.get());
   surface->SetBlendMode(SkBlendMode::kSrc);
   surface->Commit();
   RunAllPendingInMessageLoop();
 
-  const cc::CompositorFrame& frame = GetFrameFromSurface(surface.get());
+  const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
   ASSERT_EQ(1u, frame.render_pass_list.size());
   ASSERT_EQ(1u, frame.render_pass_list.back()->quad_list.size());
   EXPECT_FALSE(frame.render_pass_list.back()
@@ -278,15 +286,17 @@ TEST_F(SurfaceTest, SetBlendMode) {
 
 TEST_F(SurfaceTest, OverlayCandidate) {
   gfx::Size buffer_size(1, 1);
-  std::unique_ptr<Buffer> buffer(new Buffer(
-      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size), 0, 0, true, true));
-  std::unique_ptr<Surface> surface(new Surface);
+  auto buffer = base::MakeUnique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size), GL_TEXTURE_2D, 0,
+      true, true);
+  auto surface = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(surface.get());
 
   surface->Attach(buffer.get());
   surface->Commit();
   RunAllPendingInMessageLoop();
 
-  const cc::CompositorFrame& frame = GetFrameFromSurface(surface.get());
+  const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
   ASSERT_EQ(1u, frame.render_pass_list.size());
   ASSERT_EQ(1u, frame.render_pass_list.back()->quad_list.size());
   cc::DrawQuad* draw_quad = frame.render_pass_list.back()->quad_list.back();
@@ -299,9 +309,11 @@ TEST_F(SurfaceTest, OverlayCandidate) {
 
 TEST_F(SurfaceTest, SetAlpha) {
   gfx::Size buffer_size(1, 1);
-  std::unique_ptr<Buffer> buffer(
-      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
-  std::unique_ptr<Surface> surface(new Surface);
+  auto buffer = base::MakeUnique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size), GL_TEXTURE_2D, 0,
+      true, true);
+  auto surface = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(surface.get());
 
   surface->Attach(buffer.get());
   surface->SetAlpha(0.5f);
@@ -318,10 +330,12 @@ TEST_F(SurfaceTest, Commit) {
 TEST_F(SurfaceTest, SendsBeginFrameAcks) {
   cc::FakeExternalBeginFrameSource source(0.f, false);
   gfx::Size buffer_size(1, 1);
-  std::unique_ptr<Buffer> buffer(
-      new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size)));
-  std::unique_ptr<Surface> surface(new Surface);
-  surface->SetBeginFrameSource(&source);
+  auto buffer = base::MakeUnique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size), GL_TEXTURE_2D, 0,
+      true, true);
+  auto surface = base::MakeUnique<Surface>();
+  auto shell_surface = base::MakeUnique<ShellSurface>(surface.get());
+  shell_surface->SetBeginFrameSource(&source);
   surface->Attach(buffer.get());
 
   // Request a frame callback so that Surface now needs BeginFrames.
@@ -333,7 +347,7 @@ TEST_F(SurfaceTest, SendsBeginFrameAcks) {
 
   // Surface should add itself as observer during
   // DidReceiveCompositorFrameAck().
-  surface->DidReceiveCompositorFrameAck();
+  shell_surface->DidReceiveCompositorFrameAck();
   EXPECT_EQ(1u, source.num_observers());
 
   cc::BeginFrameArgs args(source.CreateBeginFrameArgs(BEGINFRAME_FROM_HERE));
@@ -344,7 +358,7 @@ TEST_F(SurfaceTest, SendsBeginFrameAcks) {
   surface->Commit();  // Acknowledges the BeginFrame via CompositorFrame.
   RunAllPendingInMessageLoop();
 
-  const cc::CompositorFrame& frame = GetFrameFromSurface(surface.get());
+  const cc::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
   cc::BeginFrameAck expected_ack(args.source_id, args.sequence_number, true);
   EXPECT_EQ(expected_ack, frame.metadata.begin_frame_ack);