exo: Add support for subcompositor interface.

components/exo/surface.cc

Index: components/exo/surface.cc
diff --git a/components/exo/surface.cc b/components/exo/surface.cc
index 6e21edc3b10fe86422022d86413811c2a64b2d0f..1fc1ca64d842c3ad18a560913e1783de9baeecab 100644
--- a/components/exo/surface.cc
+++ b/components/exo/surface.cc
@@ -11,6 +11,7 @@
 #include "cc/resources/single_release_callback.h"
 #include "components/exo/buffer.h"
 #include "components/exo/surface_delegate.h"
+#include "components/exo/surface_observer.h"
 #include "ui/compositor/layer.h"
 #include "ui/gfx/buffer_format_util.h"
 #include "ui/gfx/gpu_memory_buffer.h"
@@ -20,14 +21,19 @@ namespace exo {
 ////////////////////////////////////////////////////////////////////////////////
 // Surface, public:
 
-Surface::Surface() : compositor_(nullptr), delegate_(nullptr) {
+Surface::Surface()
+    : needs_commit_surface_hierarchy_(false),
+      has_contents_(false),
+      compositor_(nullptr),
+      delegate_(nullptr) {
   SetLayer(new ui::Layer(ui::LAYER_SOLID_COLOR));
   set_owned_by_client();
+  SetVisible(false);
+  SetEnabled(false);
 }
 
 Surface::~Surface() {
-  if (delegate_)
-    delegate_->OnSurfaceDestroying();
+  FOR_EACH_OBSERVER(SurfaceObserver, observers_, OnSurfaceDestroying(this));
 
   layer()->SetShowSolidColorContent();
 
@@ -69,11 +75,121 @@ void Surface::SetOpaqueRegion(const SkRegion& region) {
   pending_opaque_region_ = region;
 }
 
+void Surface::AddSubSurface(Surface* sub_surface) {
+  TRACE_EVENT1("exo", "Surface::AddSubSurface", "sub_surface",
+               sub_surface->AsTracedValue());
+
+  DCHECK(!sub_surface->parent());
+  DCHECK(!sub_surface->visible());
+  DCHECK(!sub_surface->enabled());
+  DCHECK(sub_surface->bounds().origin() == gfx::Point());
+  AddChildView(sub_surface);
+
+  pending_sub_surfaces_.push_back(std::make_pair(sub_surface, gfx::Point()));

[piman, 2015/11/19 21:59:10]

nit: DCHECK that the sub_surface is not already in the list? I think it would
cause issues otherwise. I think this is ensured by the caller because you can
only create one SubSurface per Surface, right?

[reveman, 2015/11/20 05:19:48]

Done.
Correct and verified by unit tests in latest patch.

+}
+
+void Surface::RemoveSubSurface(Surface* sub_surface) {
+  TRACE_EVENT1("exo", "Surface::AddSubSurface", "sub_surface",
+               sub_surface->AsTracedValue());
+
+  RemoveChildView(sub_surface);
+
+  SubSurfaceEntryList::iterator it =
+      std::find_if(pending_sub_surfaces_.begin(), pending_sub_surfaces_.end(),
+                   [sub_surface](const SubSurfaceEntry& entry) {
+                     return entry.first == sub_surface;
+                   });

[piman, 2015/11/19 21:59:10]

nit: worth a helper function SubSurfaceEntryList::iterator
FindSubSurfaceEntry(Surface* subsurface);

[reveman, 2015/11/20 05:19:49]

Done.

+  DCHECK(it != pending_sub_surfaces_.end());
+  pending_sub_surfaces_.erase(it);
+}
+
+void Surface::SetSubSurfacePosition(Surface* sub_surface,
+                                    const gfx::Point& position) {
+  TRACE_EVENT2("exo", "Surface::SetSubSurfacePosition", "sub_surface",
+               sub_surface->AsTracedValue(), "position", position.ToString());
+
+  SubSurfaceEntryList::iterator it =
+      std::find_if(pending_sub_surfaces_.begin(), pending_sub_surfaces_.end(),
+                   [sub_surface](const SubSurfaceEntry& entry) {
+                     return entry.first == sub_surface;
+                   });
+  DCHECK(it != pending_sub_surfaces_.end());
+  it->second = position;
+}
+
+void Surface::PlaceSubSurfaceAbove(Surface* sub_surface, Surface* reference) {
+  TRACE_EVENT2("exo", "Surface::PlaceSubSurfaceAbove", "sub_surface",
+               sub_surface->AsTracedValue(), "reference",
+               reference->AsTracedValue());

[piman, 2015/11/19 21:59:10]

nit: can you add a DCHECK_NE(sub_surface, reference);? This is ensured by the
caller, but it would be good to repeat here, otherwise the splice is undefined I
think.

[reveman, 2015/11/20 05:19:48]

I added proper checking here in latest patch instead of relying on the caller to
do this. Same below.

+
+  SubSurfaceEntryList::iterator position_it;
+  if (reference == this) {
+    position_it = pending_sub_surfaces_.begin();
+  } else {
+    position_it =
+        std::find_if(pending_sub_surfaces_.begin(), pending_sub_surfaces_.end(),
+                     [reference](const SubSurfaceEntry& entry) {
+                       return entry.first == reference;
+                     });
+    if (position_it == pending_sub_surfaces_.end()) {
+      DLOG(WARNING) << "Client tried to place sub-surface above a reference "
+                       "surface that is neither a parent nor s sibling";

[piman, 2015/11/19 21:59:10]

nit: typo 's sibling'->'a sibling'

[reveman, 2015/11/20 05:19:49]

Done.

+      return;
+    }
+
+    // Advance iterator to have |position_it| point to the sibling surface
+    // above |reference|.
+    std::advance(position_it, 1);

[piman, 2015/11/19 21:59:10]

nit: ++position_it ?

[reveman, 2015/11/20 05:19:48]

Done.

+  }
+
+  SubSurfaceEntryList::iterator it =
+      std::find_if(pending_sub_surfaces_.begin(), pending_sub_surfaces_.end(),
+                   [sub_surface](const SubSurfaceEntry& entry) {
+                     return entry.first == sub_surface;
+                   });
+  DCHECK(it != pending_sub_surfaces_.end());
+  pending_sub_surfaces_.splice(position_it, pending_sub_surfaces_, it);
+}
+
+void Surface::PlaceSubSurfaceBelow(Surface* sub_surface, Surface* sibling) {
+  TRACE_EVENT2("exo", "Surface::PlaceSubSurfaceBelow", "sub_surface",
+               sub_surface->AsTracedValue(), "sibling",
+               sibling->AsTracedValue());
+
+  SubSurfaceEntryList::iterator sibling_it =
+      std::find_if(pending_sub_surfaces_.begin(), pending_sub_surfaces_.end(),
+                   [sibling](const SubSurfaceEntry& entry) {
+                     return entry.first == sibling;
+                   });
+  if (sibling_it == pending_sub_surfaces_.end()) {
+    DLOG(WARNING) << "Client tried to place sub-surface below a surface that "
+                     "is not a sibling";
+    return;
+  }
+
+  SubSurfaceEntryList::iterator it =
+      std::find_if(pending_sub_surfaces_.begin(), pending_sub_surfaces_.end(),
+                   [sub_surface](const SubSurfaceEntry& entry) {
+                     return entry.first == sub_surface;
+                   });
+  DCHECK(it != pending_sub_surfaces_.end());
+  pending_sub_surfaces_.splice(sibling_it, pending_sub_surfaces_, it);
+}
+
 void Surface::Commit() {
   TRACE_EVENT0("exo", "Surface::Commit");
 
+  needs_commit_surface_hierarchy_ = true;
+
   if (delegate_)
     delegate_->OnSurfaceCommit();
+  else
+    CommitSurfaceHierarchy();
+}
+
+void Surface::CommitSurfaceHierarchy() {
+  DCHECK(needs_commit_surface_hierarchy_);
+  needs_commit_surface_hierarchy_ = false;
 
   cc::TextureMailbox texture_mailbox;
   scoped_ptr<cc::SingleReleaseCallback> texture_mailbox_release_callback;
@@ -81,9 +197,11 @@ void Surface::Commit() {
     texture_mailbox_release_callback =
         pending_buffer_->AcquireTextureMailbox(&texture_mailbox);
     pending_buffer_.reset();
+    has_contents_ = true;
   } else {
     // Show solid color content if there is no pending buffer.
     layer()->SetShowSolidColorContent();
+    has_contents_ = false;
   }
 
   if (texture_mailbox_release_callback) {
@@ -113,6 +231,36 @@ void Surface::Commit() {
     // Move pending frame callbacks to the end of |frame_callbacks_|.
     frame_callbacks_.splice(frame_callbacks_.end(), pending_frame_callbacks_);
   }
+
+  // Synchronize view hierarchy. This will position and update the stacking
+  // order of all sub-surfaces after committing all pending state of sub-surface
+  // descendants.
+  int index = 0;
+  for (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.
+    sub_surface->SetVisible(sub_surface->has_contents());
+    sub_surface->SetEnabled(sub_surface->has_contents());
+
+    // Move sub-surface to its new position in the stack.
+    DCHECK_LT(index, child_count());
+    ReorderChildView(sub_surface, index);
+
+    // Update sub-surface position relative to surface origin.
+    sub_surface->SetPosition(sub_surface_entry.second);
+
+    ++index;
+  }
+}
+
+bool Surface::IsSynchronized() const {
+  return delegate_ ? delegate_->IsSurfaceSynchronized() : false;
 }
 
 void Surface::SetSurfaceDelegate(SurfaceDelegate* delegate) {
@@ -120,6 +268,22 @@ void Surface::SetSurfaceDelegate(SurfaceDelegate* delegate) {
   delegate_ = delegate;
 }
 
+bool Surface::HasSurfaceDelegate() const {
+  return !!delegate_;
+}
+
+void Surface::AddSurfaceObserver(SurfaceObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void Surface::RemoveSurfaceObserver(SurfaceObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+bool Surface::HasSurfaceObserver(const SurfaceObserver* observer) const {
+  return observers_.HasObserver(observer);
+}
+
 scoped_refptr<base::trace_event::TracedValue> Surface::AsTracedValue() const {
   scoped_refptr<base::trace_event::TracedValue> value =
       new base::trace_event::TracedValue;