[exo] Pending surface states are cached for synced wl_subsurface

If the surface is a synchronized subsurface, wl_surface.commit on
a sub-surface will accumulate the committed state in a cache, but
the state will not be applied until a parent surface commit.

Reference: https://github.com/wayland-project/wayland/blob/master/protocol/wayland.xml#L2834

Bug: 1123921
Change-Id: I32396bb087db1e06262deca2611615d9f89c930e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2382057Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: Kramer Ge <fangzhoug@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821318}

components/exo/surface.cc

@@ -259,15 +259,20 @@ Surface::~Surface() {
 
   // Call all frame callbacks with a null frame time to indicate that they
   // have been cancelled.
-  frame_callbacks_.splice(frame_callbacks_.end(), pending_frame_callbacks_);
-  for (const auto& frame_callback : frame_callbacks_)
+  state_.frame_callbacks.splice(state_.frame_callbacks.end(),
+                                cached_state_.frame_callbacks);
+  state_.frame_callbacks.splice(state_.frame_callbacks.end(),
+                                pending_state_.frame_callbacks);
+  for (const auto& frame_callback : state_.frame_callbacks)
     frame_callback.Run(base::TimeTicks());
 
   // Call all presentation callbacks with a null presentation time to indicate
   // that they have been cancelled.
-  presentation_callbacks_.splice(presentation_callbacks_.end(),
-                                 pending_presentation_callbacks_);
-  for (const auto& presentation_callback : presentation_callbacks_)
+  state_.presentation_callbacks.splice(state_.presentation_callbacks.end(),
+                                       cached_state_.presentation_callbacks);
+  state_.presentation_callbacks.splice(state_.presentation_callbacks.end(),
+                                       pending_state_.presentation_callbacks);
+  for (const auto& presentation_callback : state_.presentation_callbacks)
     presentation_callback.Run(gfx::PresentationFeedback());
 
   WMHelper::GetInstance()->ResetDragDropDelegate(window_.get());
@@ -287,72 +292,73 @@ void Surface::Attach(Buffer* buffer, gfx::Vector2d offset) {
                buffer ? buffer->gfx_buffer() : nullptr, "app_id",
                GetApplicationId(window_.get()));
   has_pending_contents_ = true;
-  pending_buffer_.Reset(buffer ? buffer->AsWeakPtr() : base::WeakPtr<Buffer>());
-  pending_state_.offset = offset;
+  pending_state_.buffer.Reset(buffer ? buffer->AsWeakPtr()
+                                     : base::WeakPtr<Buffer>());
+  pending_state_.basic_state.offset = offset;
 }
 
 gfx::Vector2d Surface::GetBufferOffset() {
-  return state_.offset;
+  return state_.basic_state.offset;
 }
 
 bool Surface::HasPendingAttachedBuffer() const {
-  return pending_buffer_.buffer() != nullptr;
+  return pending_state_.buffer.buffer() != nullptr;
 }
 
 void Surface::Damage(const gfx::Rect& damage) {
   TRACE_EVENT1("exo", "Surface::Damage", "damage", damage.ToString());
 
-  pending_damage_.Union(damage);
+  pending_state_.damage.Union(damage);
 }
 
 void Surface::RequestFrameCallback(const FrameCallback& callback) {
   TRACE_EVENT0("exo", "Surface::RequestFrameCallback");
 
-  pending_frame_callbacks_.push_back(callback);
+  pending_state_.frame_callbacks.push_back(callback);
 }
 
 void Surface::RequestPresentationCallback(
     const PresentationCallback& callback) {
   TRACE_EVENT0("exo", "Surface::RequestPresentationCallback");
 
-  pending_presentation_callbacks_.push_back(callback);
+  pending_state_.presentation_callbacks.push_back(callback);
 }
 
 void Surface::SetOpaqueRegion(const cc::Region& region) {
   TRACE_EVENT1("exo", "Surface::SetOpaqueRegion", "region", region.ToString());
 
-  pending_state_.opaque_region = region;
+  pending_state_.basic_state.opaque_region = region;
 }
 
 void Surface::SetInputRegion(const cc::Region& region) {
   TRACE_EVENT1("exo", "Surface::SetInputRegion", "region", region.ToString());
 
-  pending_state_.input_region = region;
+  pending_state_.basic_state.input_region = region;
 }
 
 void Surface::ResetInputRegion() {
   TRACE_EVENT0("exo", "Surface::ResetInputRegion");
 
-  pending_state_.input_region = base::nullopt;
+  pending_state_.basic_state.input_region = base::nullopt;
 }
 
 void Surface::SetInputOutset(int outset) {
   TRACE_EVENT1("exo", "Surface::SetInputOutset", "outset", outset);
 
-  pending_state_.input_outset = outset;
+  pending_state_.basic_state.input_outset = outset;
 }
 
 void Surface::SetBufferScale(float scale) {
   TRACE_EVENT1("exo", "Surface::SetBufferScale", "scale", scale);
 
-  pending_state_.buffer_scale = scale;
+  pending_state_.basic_state.buffer_scale = scale;
 }
 
 void Surface::SetBufferTransform(Transform transform) {
   TRACE_EVENT1("exo", "Surface::SetBufferTransform", "transform",
                static_cast<int>(transform));
 
-  pending_state_.buffer_transform = transform;
+  pending_state_.basic_state.buffer_transform = transform;
 }
 
 void Surface::AddSubSurface(Surface* sub_surface) {
@@ -480,33 +486,34 @@ void Surface::OnSubSurfaceCommit() {
 void Surface::SetViewport(const gfx::Size& viewport) {
   TRACE_EVENT1("exo", "Surface::SetViewport", "viewport", viewport.ToString());
 
-  pending_state_.viewport = viewport;
+  pending_state_.basic_state.viewport = viewport;
 }
 
 void Surface::SetCrop(const gfx::RectF& crop) {
   TRACE_EVENT1("exo", "Surface::SetCrop", "crop", crop.ToString());
 
-  pending_state_.crop = crop;
+  pending_state_.basic_state.crop = crop;
 }
 
 void Surface::SetOnlyVisibleOnSecureOutput(bool only_visible_on_secure_output) {
   TRACE_EVENT1("exo", "Surface::SetOnlyVisibleOnSecureOutput",
                "only_visible_on_secure_output", only_visible_on_secure_output);
 
-  pending_state_.only_visible_on_secure_output = only_visible_on_secure_output;
+  pending_state_.basic_state.only_visible_on_secure_output =
+      only_visible_on_secure_output;
 }
 
 void Surface::SetBlendMode(SkBlendMode blend_mode) {
   TRACE_EVENT1("exo", "Surface::SetBlendMode", "blend_mode",
                static_cast<int>(blend_mode));
 
-  pending_state_.blend_mode = blend_mode;
+  pending_state_.basic_state.blend_mode = blend_mode;
 }
 
 void Surface::SetAlpha(float alpha) {
   TRACE_EVENT1("exo", "Surface::SetAlpha", "alpha", alpha);
 
-  pending_state_.alpha = alpha;
+  pending_state_.basic_state.alpha = alpha;
 }
 
 void Surface::SetFrame(SurfaceFrameType type) {
@@ -550,7 +557,7 @@ void Surface::SetColorSpace(gfx::ColorSpace color_space) {
   TRACE_EVENT1("exo", "Surface::SetColorSpace", "color_space",
                color_space.ToString());
 
-  pending_state_.color_space = color_space;
+  pending_state_.basic_state.color_space = color_space;
 }
 
 void Surface::SetParent(Surface* parent, const gfx::Point& position) {
@@ -593,22 +600,45 @@ void Surface::SetAcquireFence(std::unique_ptr<gfx::GpuFence> gpu_fence) {
   TRACE_EVENT1("exo", "Surface::SetAcquireFence", "fence_fd",
                gpu_fence ? gpu_fence->GetGpuFenceHandle().owned_fd.get() : -1);
 
-  pending_acquire_fence_ = std::move(gpu_fence);
+  pending_state_.acquire_fence = std::move(gpu_fence);
 }
 
 bool Surface::HasPendingAcquireFence() const {
-  return !!pending_acquire_fence_;
+  return !!pending_state_.acquire_fence;
 }
 
 void Surface::Commit() {
   TRACE_EVENT1("exo", "Surface::Commit", "buffer_id",
-               pending_buffer_.buffer() ? pending_buffer_.buffer()->gfx_buffer()
-                                        : nullptr);
+               pending_state_.buffer.buffer()
+                   ? pending_state_.buffer.buffer()->gfx_buffer()
+                   : nullptr);
 
   for (auto& observer : observers_)
     observer.OnCommit(this);
 
   needs_commit_surface_ = true;
+
+  // Transfer pending state to cached state.
+  cached_state_.basic_state = pending_state_.basic_state;
+  pending_state_.basic_state.only_visible_on_secure_output = false;
+  has_cached_contents_ |= has_pending_contents_;
+  has_pending_contents_ = false;
+  cached_state_.buffer = std::move(pending_state_.buffer);
+  cached_state_.acquire_fence = std::move(pending_state_.acquire_fence);
+  cached_state_.frame_callbacks.splice(cached_state_.frame_callbacks.end(),
+                                       pending_state_.frame_callbacks);
+  cached_state_.damage.Union(pending_state_.damage);
+  pending_state_.damage.Clear();
+
+  // Existing presentation callbacks in the cached state when a new pending
+  // state is merged in should end up delivered as "discarded".
+  for (const auto& presentation_callback : cached_state_.presentation_callbacks)
+    presentation_callback.Run(gfx::PresentationFeedback());
+  cached_state_.presentation_callbacks.clear();
+  cached_state_.presentation_callbacks.splice(
+      cached_state_.presentation_callbacks.end(),
+      pending_state_.presentation_callbacks);
+
   if (delegate_)
     delegate_->OnSurfaceCommit();
   else
@@ -634,45 +664,53 @@ void Surface::CommitSurfaceHierarchy(bool synchronized) {
     // https://crbug.com/779704
     bool needs_full_damage =
         sub_surfaces_changed_ ||
-        pending_state_.opaque_region != state_.opaque_region ||
-        pending_state_.buffer_scale != state_.buffer_scale ||
-        pending_state_.buffer_transform != state_.buffer_transform ||
-        pending_state_.viewport != state_.viewport ||
-        pending_state_.crop != state_.crop ||
-        pending_state_.only_visible_on_secure_output !=
-            state_.only_visible_on_secure_output ||
-        pending_state_.blend_mode != state_.blend_mode ||
-        pending_state_.alpha != state_.alpha ||
-        pending_state_.color_space != state_.color_space ||
-        pending_state_.is_tracking_occlusion != state_.is_tracking_occlusion;
+        cached_state_.basic_state.opaque_region !=
+            state_.basic_state.opaque_region ||
+        cached_state_.basic_state.buffer_scale !=
+            state_.basic_state.buffer_scale ||
+        cached_state_.basic_state.buffer_transform !=
+            state_.basic_state.buffer_transform ||
+        cached_state_.basic_state.viewport != state_.basic_state.viewport ||
+        cached_state_.basic_state.crop != state_.basic_state.crop ||
+        cached_state_.basic_state.only_visible_on_secure_output !=
+            state_.basic_state.only_visible_on_secure_output ||
+        cached_state_.basic_state.blend_mode != state_.basic_state.blend_mode ||
+        cached_state_.basic_state.alpha != state_.basic_state.alpha ||
+        cached_state_.basic_state.color_space !=
+            state_.basic_state.color_space ||
+        cached_state_.basic_state.is_tracking_occlusion !=
+            state_.basic_state.is_tracking_occlusion;
 
     bool needs_update_buffer_transform =
-        pending_state_.buffer_scale != state_.buffer_scale ||
-        pending_state_.buffer_transform != state_.buffer_transform;
+        cached_state_.basic_state.buffer_scale !=
+            state_.basic_state.buffer_scale ||
+        cached_state_.basic_state.buffer_transform !=
+            state_.basic_state.buffer_transform;
 
 #if defined(OS_CHROMEOS)
     bool needs_output_protection =
-        pending_state_.only_visible_on_secure_output !=
-        state_.only_visible_on_secure_output;
+        cached_state_.basic_state.only_visible_on_secure_output !=
+        state_.basic_state.only_visible_on_secure_output;
 #endif  // defined(OS_CHROMEOS)
 
-    bool pending_invert_y = false;
+    bool cached_invert_y = false;
 
     // If the current state is fully transparent, the last submitted frame will
     // not include the TextureDrawQuad for the resource, so the resource might
     // have been released and needs to be updated again.
-    if (!state_.alpha && pending_state_.alpha)
+    if (!state_.basic_state.alpha && cached_state_.basic_state.alpha)
       needs_update_resource_ = true;
 
-    state_ = pending_state_;
-    pending_state_.only_visible_on_secure_output = false;
+    state_.basic_state = cached_state_.basic_state;
+    cached_state_.basic_state.only_visible_on_secure_output = false;
 
     window_->SetEventTargetingPolicy(
-        (state_.input_region.has_value() && state_.input_region->IsEmpty())
+        (state_.basic_state.input_region.has_value() &&
+         state_.basic_state.input_region->IsEmpty())
             ? aura::EventTargetingPolicy::kDescendantsOnly
             : aura::EventTargetingPolicy::kTargetAndDescendants);
 
-    if (state_.is_tracking_occlusion) {
+    if (state_.basic_state.is_tracking_occlusion) {
       // TODO(edcourtney): Currently, it doesn't seem to be possible to stop
       // tracking the occlusion state once started, but it would be nice to stop
       // if the tracked occlusion region becomes empty.
@@ -686,51 +724,56 @@ void Surface::CommitSurfaceHierarchy(bool synchronized) {
             std::make_unique<ash::OutputProtectionDelegate>(window_.get());
       }
 
-      uint32_t protection_mask = state_.only_visible_on_secure_output
-                                     ? display::CONTENT_PROTECTION_METHOD_HDCP
-                                     : display::CONTENT_PROTECTION_METHOD_NONE;
+      uint32_t protection_mask =
+          state_.basic_state.only_visible_on_secure_output
+              ? display::CONTENT_PROTECTION_METHOD_HDCP
+              : display::CONTENT_PROTECTION_METHOD_NONE;
 
       output_protection_->SetProtection(protection_mask, base::DoNothing());
     }
 #endif  // defined(OS_CHROMEOS)
 
     // We update contents if Attach() has been called since last commit.
-    if (has_pending_contents_) {
-      has_pending_contents_ = false;
+    if (has_cached_contents_) {
+      has_cached_contents_ = false;
 
       bool current_invert_y =
-          current_buffer_.buffer() && current_buffer_.buffer()->y_invert();
-      pending_invert_y =
-          pending_buffer_.buffer() && pending_buffer_.buffer()->y_invert();
-      if (current_invert_y != pending_invert_y)
+          state_.buffer.buffer() && state_.buffer.buffer()->y_invert();
+      cached_invert_y = cached_state_.buffer.buffer() &&
+                        cached_state_.buffer.buffer()->y_invert();
+      if (current_invert_y != cached_invert_y)
         needs_update_buffer_transform = true;
 
-      current_buffer_ = std::move(pending_buffer_);
-      acquire_fence_ = std::move(pending_acquire_fence_);
-      if (state_.alpha)
+      state_.buffer = std::move(cached_state_.buffer);
+      state_.acquire_fence = std::move(cached_state_.acquire_fence);
+      if (state_.basic_state.alpha)
         needs_update_resource_ = true;
     }
     // Either we didn't have a pending acquire fence, or we had one along with
-    // a new buffer, and it was already moved to acquire_fence_. Note that
+    // a new buffer, and it was already moved to state_.acquire_fence. Note that
     // it is a commit-time client error to commit a fence without a buffer.
-    DCHECK(!pending_acquire_fence_);
+    DCHECK(!cached_state_.acquire_fence);
 
     if (needs_update_buffer_transform)
-      UpdateBufferTransform(pending_invert_y);
+      UpdateBufferTransform(cached_invert_y);
 
-    // 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 |state_.frame_callbacks|.
+    state_.frame_callbacks.splice(state_.frame_callbacks.end(),
+                                  cached_state_.frame_callbacks);
 
     // Move pending presentation callbacks to the end of
-    // |presentation_callbacks_|.
-    presentation_callbacks_.splice(presentation_callbacks_.end(),
-                                   pending_presentation_callbacks_);
+    // |state_.presentation_callbacks|.
+    state_.presentation_callbacks.splice(state_.presentation_callbacks.end(),
+                                         cached_state_.presentation_callbacks);
 
     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.
+    // Changes to sub_surface stack is immediately applied to pending, which
+    // will be copied to active directly when parent surface is committed,
+    // skipping the cached state.
     if (sub_surfaces_changed_) {
       sub_surfaces_.clear();
       aura::Window* stacking_target = nullptr;
@@ -753,24 +796,24 @@ void Surface::CommitSurfaceHierarchy(bool synchronized) {
 
     gfx::Rect output_rect(content_size_);
     if (needs_full_damage) {
-      damage_ = output_rect;
+      state_.damage = output_rect;
     } else {
-      // pending_damage_ is in Surface coordinates.
-      damage_.Swap(&pending_damage_);
-      damage_.Intersect(output_rect);
+      // cached_state_.damage is in Surface coordinates.
+      state_.damage.Swap(&cached_state_.damage);
+      state_.damage.Intersect(output_rect);
     }
-    pending_damage_.Clear();
+    cached_state_.damage.Clear();
   }
 
   surface_hierarchy_content_bounds_ = gfx::Rect(content_size_);
-  if (state_.input_region) {
-    hit_test_region_ = *state_.input_region;
+  if (state_.basic_state.input_region) {
+    hit_test_region_ = *state_.basic_state.input_region;
     hit_test_region_.Intersect(surface_hierarchy_content_bounds_);
   } else {
     hit_test_region_ = surface_hierarchy_content_bounds_;
   }
 
-  int outset = state_.input_outset;
+  int outset = state_.basic_state.input_outset;
   if (outset > 0) {
     gfx::Rect input_rect = surface_hierarchy_content_bounds_;
     input_rect.Inset(-outset, -outset);
@@ -793,10 +836,10 @@ void Surface::AppendSurfaceHierarchyCallbacks(
     std::list<FrameCallback>* frame_callbacks,
     std::list<PresentationCallback>* presentation_callbacks) {
   // Move frame callbacks to the end of |frame_callbacks|.
-  frame_callbacks->splice(frame_callbacks->end(), frame_callbacks_);
+  frame_callbacks->splice(frame_callbacks->end(), state_.frame_callbacks);
   // Move presentation callbacks to the end of |presentation_callbacks|.
   presentation_callbacks->splice(presentation_callbacks->end(),
-                                 presentation_callbacks_);
+                                 state_.presentation_callbacks);
 
   for (const auto& sub_surface_entry : base::Reversed(sub_surfaces_)) {
     auto* sub_surface = sub_surface_entry.first;
@@ -895,16 +938,16 @@ void Surface::SurfaceHierarchyResourcesLost() {
 
 bool Surface::FillsBoundsOpaquely() const {
   return !current_resource_has_alpha_ ||
-         state_.blend_mode == SkBlendMode::kSrc ||
-         state_.opaque_region.Contains(gfx::Rect(content_size_));
+         state_.basic_state.blend_mode == SkBlendMode::kSrc ||
+         state_.basic_state.opaque_region.Contains(gfx::Rect(content_size_));
 }
 
 void Surface::SetOcclusionTracking(bool tracking) {
-  pending_state_.is_tracking_occlusion = tracking;
+  pending_state_.basic_state.is_tracking_occlusion = tracking;
 }
 
 bool Surface::IsTrackingOcclusion() {
-  return state_.is_tracking_occlusion;
+  return state_.basic_state.is_tracking_occlusion;
 }
 
 void Surface::SetSurfaceHierarchyContentBoundsForTest(
@@ -936,6 +979,10 @@ Surface::BufferAttachment::~BufferAttachment() {
     buffer_->OnDetach();
 }
 
+Surface::ExtendedState::ExtendedState() = default;
+
+Surface::ExtendedState::~ExtendedState() = default;
+
 Surface::BufferAttachment& Surface::BufferAttachment::operator=(
     BufferAttachment&& other) {
   if (buffer_)
@@ -973,26 +1020,27 @@ void Surface::BufferAttachment::Reset(base::WeakPtr<Buffer> buffer) {
 void Surface::UpdateResource(FrameSinkResourceManager* resource_manager) {
   DCHECK(needs_update_resource_);
   needs_update_resource_ = false;
-  if (current_buffer_.buffer()) {
-    if (current_buffer_.buffer()->ProduceTransferableResource(
-            resource_manager, std::move(acquire_fence_),
-            state_.only_visible_on_secure_output, &current_resource_)) {
+  if (state_.buffer.buffer()) {
+    if (state_.buffer.buffer()->ProduceTransferableResource(
+            resource_manager, std::move(state_.acquire_fence),
+            state_.basic_state.only_visible_on_secure_output,
+            &current_resource_)) {
       current_resource_has_alpha_ =
-          FormatHasAlpha(current_buffer_.buffer()->GetFormat());
+          FormatHasAlpha(state_.buffer.buffer()->GetFormat());
       // Planar buffers are sampled as RGB. Technically, the driver is supposed
       // to preserve the colorspace, so we could still pass the primaries and
       // transfer function.  However, we don't actually pass the colorspace
       // to the driver, and it's unclear what drivers would actually do if we
       // did. So in effect, the colorspace is undefined.
       if (NumberOfPlanesForLinearBufferFormat(
-              current_buffer_.buffer()->GetFormat()) > 1) {
-        current_resource_.color_space = state_.color_space;
+              state_.buffer.buffer()->GetFormat()) > 1) {
+        current_resource_.color_space = state_.basic_state.color_space;
       }
     } else {
       current_resource_.id = 0;
       // Use the buffer's size, so the AppendContentsToFrame() will append
       // a SolidColorDrawQuad with the buffer's size.
-      current_resource_.size = current_buffer_.size();
+      current_resource_.size = state_.buffer.size();
       current_resource_has_alpha_ = false;
     }
   } else {
@@ -1004,7 +1052,7 @@ void Surface::UpdateResource(FrameSinkResourceManager* resource_manager) {
 
 void Surface::UpdateBufferTransform(bool y_invert) {
   SkMatrix buffer_matrix;
-  switch (state_.buffer_transform) {
+  switch (state_.basic_state.buffer_transform) {
     case Transform::NORMAL:
       buffer_matrix.setIdentity();
       break;
@@ -1020,9 +1068,9 @@ void Surface::UpdateBufferTransform(bool y_invert) {
   }
   if (y_invert)
     buffer_matrix.preScale(1, -1, 0.5f, 0.5f);
-  if (state_.buffer_scale != 0)
-    buffer_matrix.postScale(1.0f / state_.buffer_scale,
-                            1.0f / state_.buffer_scale);
+  if (state_.basic_state.buffer_scale != 0)
+    buffer_matrix.postScale(1.0f / state_.basic_state.buffer_scale,
+                            1.0f / state_.basic_state.buffer_scale);
   buffer_transform_ = gfx::Transform(buffer_matrix);
 }
 
@@ -1036,7 +1084,7 @@ void Surface::AppendContentsToFrame(const gfx::Point& origin,
 
   // Surface bounds are in DIPs, but |damage_rect| and |output_rect| are in
   // pixels, so we need to scale by the |device_scale_factor|.
-  gfx::Rect damage_rect = damage_.bounds();
+  gfx::Rect damage_rect = state_.damage.bounds();
   if (!damage_rect.IsEmpty()) {
     // Outset damage by 1 DIP to as damage is in surface coordinate space and
     // client might not be aware of |device_scale_factor| and the
@@ -1057,7 +1105,7 @@ void Surface::AppendContentsToFrame(const gfx::Point& origin,
       render_pass->damage_rect.Union(gfx::ToEnclosedRect(scaled_damage));
     }
   }
-  damage_.Clear();
+  state_.damage.Clear();
 
   gfx::PointF scale(content_size_.width(), content_size_.height());
 
@@ -1069,19 +1117,19 @@ void Surface::AppendContentsToFrame(const gfx::Point& origin,
     quad_rect = gfx::Rect(embedded_surface_size_);
     scale = gfx::PointF(1.0f, 1.0f);
 
-    if (!state_.crop.IsEmpty()) {
+    if (!state_.basic_state.crop.IsEmpty()) {
       // In order to crop an AxB rect to CxD we need to scale by A/C, B/D.
       // We achieve clipping by scaling it up and then drawing only in the
       // output rectangle.
-      scale.Scale(content_size_.width() / state_.crop.width(),
-                  content_size_.height() / state_.crop.height());
+      scale.Scale(content_size_.width() / state_.basic_state.crop.width(),
+                  content_size_.height() / state_.basic_state.crop.height());
 
-      auto offset = state_.crop.origin().OffsetFromOrigin();
+      auto offset = state_.basic_state.crop.origin().OffsetFromOrigin();
       translate =
           gfx::Vector2dF(-offset.x() * scale.x(), -offset.y() * scale.y());
     }
   } else {
-    scale.Scale(state_.buffer_scale);
+    scale.Scale(state_.basic_state.buffer_scale);
   }
 
   // Compute the total transformation from post-transform buffer coordinates to
@@ -1099,29 +1147,31 @@ void Surface::AppendContentsToFrame(const gfx::Point& origin,
   quad_to_target_transform.ConcatTransform(
       gfx::Transform(viewport_to_target_matrix));
 
-  bool are_contents_opaque = !current_resource_has_alpha_ ||
-                             state_.blend_mode == SkBlendMode::kSrc ||
-                             state_.opaque_region.Contains(output_rect);
+  bool are_contents_opaque =
+      !current_resource_has_alpha_ ||
+      state_.basic_state.blend_mode == SkBlendMode::kSrc ||
+      state_.basic_state.opaque_region.Contains(output_rect);
 
   viz::SharedQuadState* quad_state =
       render_pass->CreateAndAppendSharedQuadState();
-  quad_state->SetAll(
-      quad_to_target_transform, quad_rect /*quad_layer_rect=*/,
-      quad_rect /*visible_quad_layer_rect=*/,
-      gfx::RRectF() /*rounded_corner_bounds=*/, gfx::Rect() /*clip_rect=*/,
-      false /*is_clipped=*/, are_contents_opaque, state_.alpha /*opacity=*/,
-      SkBlendMode::kSrcOver /*blend_mode=*/, 0 /*sorting_context_id=*/);
+  quad_state->SetAll(quad_to_target_transform, quad_rect /*quad_layer_rect=*/,
+                     quad_rect /*visible_quad_layer_rect=*/,
+                     gfx::RRectF() /*rounded_corner_bounds=*/,
+                     gfx::Rect() /*clip_rect=*/, false /*is_clipped=*/,
+                     are_contents_opaque, state_.basic_state.alpha /*opacity=*/,
+                     SkBlendMode::kSrcOver /*blend_mode=*/,
+                     0 /*sorting_context_id=*/);
   quad_state->no_damage = damage_rect.IsEmpty();
 
   if (current_resource_.id) {
     gfx::RectF uv_crop(gfx::SizeF(1, 1));
-    if (!state_.crop.IsEmpty()) {
+    if (!state_.basic_state.crop.IsEmpty()) {
       // The crop rectangle is a post-transformation rectangle. To get the UV
       // coordinates, we need to convert it to normalized buffer coordinates and
       // pass them through the inverse of the buffer transformation.
-      uv_crop = gfx::RectF(state_.crop);
-      gfx::Size transformed_buffer_size(
-          ToTransformedSize(current_resource_.size, state_.buffer_transform));
+      uv_crop = gfx::RectF(state_.basic_state.crop);
+      gfx::Size transformed_buffer_size(ToTransformedSize(
+          current_resource_.size, state_.basic_state.buffer_transform));
       if (!transformed_buffer_size.IsEmpty())
         uv_crop.Scale(1.f / transformed_buffer_size.width(),
                       1.f / transformed_buffer_size.height());
@@ -1147,7 +1197,7 @@ void Surface::AppendContentsToFrame(const gfx::Point& origin,
       }
       if (latest_embedded_surface_id_.is_valid() &&
           !embedded_surface_size_.IsEmpty()) {
-        if (!state_.crop.IsEmpty()) {
+        if (!state_.basic_state.crop.IsEmpty()) {
           quad_state->is_clipped = true;
           quad_state->clip_rect = output_rect;
         }
@@ -1162,7 +1212,7 @@ void Surface::AppendContentsToFrame(const gfx::Point& origin,
       // A resource was still produced for this so we still need to release it
       // later.
       frame->resource_list.push_back(current_resource_);
-    } else if (state_.alpha) {
+    } else if (state_.basic_state.alpha) {
       // Texture quad is only needed if buffer is not fully transparent.
       viz::TextureDrawQuad* texture_quad =
           render_pass->CreateAndAppendDrawQuad<viz::TextureDrawQuad>();
@@ -1173,7 +1223,7 @@ void Surface::AppendContentsToFrame(const gfx::Point& origin,
           /* premultiplied_alpha=*/true, uv_crop.origin(),
           uv_crop.bottom_right(), background_color, vertex_opacity,
           /* y_flipped=*/false, /* nearest_neighbor=*/false,
-          state_.only_visible_on_secure_output,
+          state_.basic_state.only_visible_on_secure_output,
           gfx::ProtectedVideoType::kClear);
       if (current_resource_.is_overlay_candidate)
         texture_quad->set_resource_size_in_pixels(current_resource_.size);
@@ -1189,20 +1239,21 @@ void Surface::AppendContentsToFrame(const gfx::Point& origin,
 
 void Surface::UpdateContentSize() {
   gfx::Size content_size;
-  if (!state_.viewport.IsEmpty()) {
-    content_size = state_.viewport;
-  } else if (!state_.crop.IsEmpty()) {
-    DLOG_IF(WARNING,
-            !base::IsValueInRangeForNumericType<int>(state_.crop.width()) ||
-                !base::IsValueInRangeForNumericType<int>(state_.crop.height()))
-        << "Crop rectangle size (" << state_.crop.size().ToString()
+  if (!state_.basic_state.viewport.IsEmpty()) {
+    content_size = state_.basic_state.viewport;
+  } else if (!state_.basic_state.crop.IsEmpty()) {
+    DLOG_IF(WARNING, !base::IsValueInRangeForNumericType<int>(
+                         state_.basic_state.crop.width()) ||
+                         !base::IsValueInRangeForNumericType<int>(
+                             state_.basic_state.crop.height()))
+        << "Crop rectangle size (" << state_.basic_state.crop.size().ToString()
         << ") most be expressible using integers when viewport is not set";
-    content_size = gfx::ToCeiledSize(state_.crop.size());
+    content_size = gfx::ToCeiledSize(state_.basic_state.crop.size());
   } else {
-    content_size = gfx::ToCeiledSize(
-        gfx::ScaleSize(gfx::SizeF(ToTransformedSize(current_buffer_.size(),
-                                                    state_.buffer_transform)),
-                       1.0f / state_.buffer_scale));
+    content_size = gfx::ToCeiledSize(gfx::ScaleSize(
+        gfx::SizeF(ToTransformedSize(state_.buffer.size(),
+                                     state_.basic_state.buffer_transform)),
+        1.0f / state_.basic_state.buffer_scale));
   }
 
   // Enable/disable sub-surface based on if it has contents.
@@ -1221,7 +1272,7 @@ void Surface::UpdateContentSize() {
 }
 
 void Surface::OnWindowOcclusionChanged() {
-  if (!state_.is_tracking_occlusion)
+  if (!state_.basic_state.is_tracking_occlusion)
     return;
 
   for (SurfaceObserver& observer : observers_)

components/exo/surface.h

@@ -287,7 +287,7 @@ class Surface final : public ui::PropertyHandler {
   bool FillsBoundsOpaquely() const;
 
   bool HasPendingDamageForTesting(const gfx::Rect& damage) const {
-    return pending_damage_.Contains(damage);
+    return pending_state_.damage.Contains(damage);
   }
 
   // Set occlusion tracking region for surface.
@@ -346,6 +346,26 @@ class Surface final : public ui::PropertyHandler {
     DISALLOW_COPY_AND_ASSIGN(BufferAttachment);
   };
 
+  struct ExtendedState {
+    ExtendedState();
+    ~ExtendedState();
+
+    State basic_state;
+
+    // The buffer that will become the content of surface.
+    BufferAttachment buffer;
+    // The damage region to schedule paint for.
+    cc::Region damage;
+    // These lists contain the callbacks to notify the client when it is a good
+    // time to start producing a new frame.
+    std::list<FrameCallback> frame_callbacks;
+    // These lists contain the callbacks to notify the client when surface
+    // contents have been presented.
+    std::list<PresentationCallback> presentation_callbacks;
+    // The acquire gpu fence to associate with the surface buffer.
+    std::unique_ptr<gfx::GpuFence> acquire_fence;
+  };
+
   friend class subtle::PropertyHelper;
 
   // Updates current_resource_ with a new resource id corresponding to the
@@ -367,7 +387,7 @@ class Surface final : public ui::PropertyHandler {
   void UpdateContentSize();
 
   // This returns true when the surface has some contents assigned to it.
-  bool has_contents() const { return !current_buffer_.size().IsEmpty(); }
+  bool has_contents() const { return !state_.buffer.size().IsEmpty(); }
 
   // This window has the layer which contains the Surface contents.
   std::unique_ptr<aura::Window> window_;
@@ -381,43 +401,19 @@ class Surface final : public ui::PropertyHandler {
   // This is the bounds of the last committed surface hierarchy contents.
   gfx::Rect surface_hierarchy_content_bounds_;
 
-  // This is true when Attach() has been called and new contents should take
-  // effect next time Commit() is called.
+  // This is true when Attach() has been called and new contents should be
+  // cached next time Commit() is called.
   bool has_pending_contents_ = false;
+  // This is true when new contents are cached and should take effect next time
+  // synchronized CommitSurfaceHierarchy() is called.
+  bool has_cached_contents_ = false;
 
-  // The buffer that will become the content of surface when Commit() is called.
-  BufferAttachment pending_buffer_;
-
-  // The damage region to schedule paint for when Commit() is called.
-  cc::Region pending_damage_;
-
-  // The damage region which will be used by
-  // AppendSurfaceHierarchyContentsToFrame() to generate frame.
-  cc::Region damage_;
-
-  // These lists contains 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<FrameCallback> pending_frame_callbacks_;
-  std::list<FrameCallback> 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<PresentationCallback> pending_presentation_callbacks_;
-  std::list<PresentationCallback> presentation_callbacks_;
-
+  // This is the state that has yet to be cached.
+  ExtendedState pending_state_;
   // This is the state that has yet to be committed.
-  State pending_state_;
-
+  ExtendedState cached_state_;
   // This is the state that has been committed.
-  State state_;
+  ExtendedState state_;
 
   // Cumulative input region of surface and its sub-surfaces.
   cc::Region hit_test_region_;
@@ -430,21 +426,12 @@ class Surface final : public ui::PropertyHandler {
   SubSurfaceEntryList pending_sub_surfaces_;
   SubSurfaceEntryList sub_surfaces_;
 
-  // The buffer that is currently set as content of surface.
-  BufferAttachment current_buffer_;
-
   // The last resource that was sent to a surface.
   viz::TransferableResource current_resource_;
 
   // Whether the last resource that was sent to a surface has an alpha channel.
   bool current_resource_has_alpha_ = false;
 
-  // The acquire gpu fence to associate with the surface buffer when Commit()
-  // is called.
-  std::unique_ptr<gfx::GpuFence> pending_acquire_fence_;
-  // The acquire gpu fence that is currently associated with the surface buffer.
-  std::unique_ptr<gfx::GpuFence> acquire_fence_;
-
   // This is true if a call to Commit() as been made but
   // CommitSurfaceHierarchy() has not yet been called.
   bool needs_commit_surface_ = false;

components/exo/surface_unittest.cc

@@ -260,6 +260,128 @@ TEST_P(SurfaceTest, SubsurfaceDamageAggregation) {
   }
 }
 
+TEST_P(SurfaceTest, SubsurfaceDamageSynchronizedCommitBehavior) {
+  gfx::Size buffer_size(256, 512);
+  auto buffer = std::make_unique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+  auto surface = std::make_unique<Surface>();
+  auto shell_surface = std::make_unique<ShellSurface>(surface.get());
+  surface->Attach(buffer.get());
+  gfx::Size child_buffer_size(64, 128);
+  auto child_buffer = std::make_unique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(child_buffer_size));
+  auto child_surface = std::make_unique<Surface>();
+  auto sub_surface =
+      std::make_unique<SubSurface>(child_surface.get(), surface.get());
+  // Set commit behavior to synchronized.
+  sub_surface->SetCommitBehavior(true);
+  child_surface->Attach(child_buffer.get());
+  child_surface->Commit();
+  surface->Commit();
+  base::RunLoop().RunUntilIdle();
+
+  {
+    // Initial frame has full damage.
+    const viz::CompositorFrame& frame =
+        GetFrameFromSurface(shell_surface.get());
+    const gfx::Rect scaled_damage = gfx::ToNearestRect(gfx::ScaleRect(
+        gfx::RectF(gfx::Rect(buffer_size)), device_scale_factor()));
+    EXPECT_EQ(scaled_damage, frame.render_pass_list.back()->damage_rect);
+  }
+
+  const gfx::RectF subsurface_damage(32, 32, 16, 16);
+  const gfx::RectF subsurface_damage2(0, 0, 16, 16);
+  int margin = ceil(device_scale_factor());
+
+  child_surface->Damage(gfx::ToNearestRect(subsurface_damage));
+  EXPECT_TRUE(child_surface->HasPendingDamageForTesting(
+      gfx::ToNearestRect(subsurface_damage)));
+  // Subsurface damage is cached.
+  child_surface->Commit();
+  EXPECT_FALSE(child_surface->HasPendingDamageForTesting(
+      gfx::ToNearestRect(subsurface_damage)));
+  base::RunLoop().RunUntilIdle();
+
+  {
+    // Subsurface damage should not be propagated at all.
+    const viz::CompositorFrame& frame =
+        GetFrameFromSurface(shell_surface.get());
+    const gfx::Rect scaled_damage = gfx::ToNearestRect(gfx::ScaleRect(
+        gfx::RectF(gfx::Rect(buffer_size)), device_scale_factor()));
+    EXPECT_EQ(scaled_damage, frame.render_pass_list.back()->damage_rect);
+  }
+
+  // Damage but do not commit.
+  child_surface->Damage(gfx::ToNearestRect(subsurface_damage2));
+  EXPECT_TRUE(child_surface->HasPendingDamageForTesting(
+      gfx::ToNearestRect(subsurface_damage2)));
+  // Apply subsurface damage from cached state, not pending state.
+  surface->Commit();
+  base::RunLoop().RunUntilIdle();
+
+  {
+    // Subsurface damage in cached state should be propagated.
+    const viz::CompositorFrame& frame =
+        GetFrameFromSurface(shell_surface.get());
+    const gfx::Rect scaled_damage = gfx::ToNearestRect(
+        gfx::ScaleRect(subsurface_damage, device_scale_factor()));
+    EXPECT_TRUE(scaled_damage.ApproximatelyEqual(
+        frame.render_pass_list.back()->damage_rect, margin));
+  }
+}
+
+TEST_P(SurfaceTest, SubsurfaceDamageDesynchronizedCommitBehavior) {
+  gfx::Size buffer_size(256, 512);
+  auto buffer = std::make_unique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+  auto surface = std::make_unique<Surface>();
+  auto shell_surface = std::make_unique<ShellSurface>(surface.get());
+  surface->Attach(buffer.get());
+  gfx::Size child_buffer_size(64, 128);
+  auto child_buffer = std::make_unique<Buffer>(
+      exo_test_helper()->CreateGpuMemoryBuffer(child_buffer_size));
+  auto child_surface = std::make_unique<Surface>();
+  auto sub_surface =
+      std::make_unique<SubSurface>(child_surface.get(), surface.get());
+  // Set commit behavior to desynchronized.
+  sub_surface->SetCommitBehavior(false);
+  child_surface->Attach(child_buffer.get());
+  child_surface->Commit();
+  surface->Commit();
+  base::RunLoop().RunUntilIdle();
+
+  {
+    // Initial frame has full damage.
+    const viz::CompositorFrame& frame =
+        GetFrameFromSurface(shell_surface.get());
+    const gfx::Rect scaled_damage = gfx::ToNearestRect(gfx::ScaleRect(
+        gfx::RectF(gfx::Rect(buffer_size)), device_scale_factor()));
+    EXPECT_EQ(scaled_damage, frame.render_pass_list.back()->damage_rect);
+  }
+
+  const gfx::RectF subsurface_damage(32, 32, 16, 16);
+  int margin = ceil(device_scale_factor());
+
+  child_surface->Damage(gfx::ToNearestRect(subsurface_damage));
+  EXPECT_TRUE(child_surface->HasPendingDamageForTesting(
+      gfx::ToNearestRect(subsurface_damage)));
+  // Subsurface damage is applied.
+  child_surface->Commit();
+  EXPECT_FALSE(child_surface->HasPendingDamageForTesting(
+      gfx::ToNearestRect(subsurface_damage)));
+  base::RunLoop().RunUntilIdle();
+
+  {
+    // Subsurface damage should be propagated.
+    const viz::CompositorFrame& frame =
+        GetFrameFromSurface(shell_surface.get());
+    const gfx::Rect scaled_damage = gfx::ToNearestRect(
+        gfx::ScaleRect(subsurface_damage, device_scale_factor()));
+    EXPECT_TRUE(scaled_damage.ApproximatelyEqual(
+        frame.render_pass_list.back()->damage_rect, margin));
+  }
+}
+
 void SetFrameTime(base::TimeTicks* result, base::TimeTicks frame_time) {
   *result = frame_time;
 }