Use ui::LayerOwner for holding space tray icon previews

Instead of HoldingSpaceTrayIconPreview owning the image layer directly,
delegate the layer ownership to a ui::LayerOwner. Unlike
HoldingSpaceTrayIconPreview, LayerOwner has logic to recreate the layer
when required. For example, when the layer tree was copied during
transition to tablet mode, the HoldingSpaceTrayIconPreview layers were
just moved to the new layer, causing the layer copy shown during the
tablet mode transition not to have item previews.

BUG=1166013

Change-Id: I17322043167e3275d6c9297617c90de834fb912e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2629029
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#843724}

ash/system/holding_space/holding_space_tray_icon_preview.cc

@@ -177,7 +177,7 @@ void HoldingSpaceTrayIconPreview::AnimateIn(base::TimeDelta additional_delay) {
   gfx::Transform mid_transform(transform_);
   mid_transform.Translate(0, preview_size.height() * 0.25f);
 
-  ui::ScopedLayerAnimationSettings scoped_settings(layer_->GetAnimator());
+  ui::ScopedLayerAnimationSettings scoped_settings(layer()->GetAnimator());
   scoped_settings.SetPreemptionStrategy(
       ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
 
@@ -199,7 +199,7 @@ void HoldingSpaceTrayIconPreview::AnimateIn(base::TimeDelta additional_delay) {
   rebound->set_tween_type(gfx::Tween::FAST_OUT_SLOW_IN_3);
   sequence->AddElement(std::move(rebound));
 
-  layer_->GetAnimator()->StartAnimation(sequence.release());
+  layer()->GetAnimator()->StartAnimation(sequence.release());
 }
 
 void HoldingSpaceTrayIconPreview::AnimateOut(
@@ -208,17 +208,17 @@ void HoldingSpaceTrayIconPreview::AnimateOut(
   pending_index_.reset();
   index_.reset();
 
-  if (!layer_) {
+  if (!layer()) {
     std::move(animate_out_closure_).Run();
     return;
   }
 
-  ui::ScopedLayerAnimationSettings animation_settings(layer_->GetAnimator());
+  ui::ScopedLayerAnimationSettings animation_settings(layer()->GetAnimator());
   SetUpAnimation(&animation_settings);
   animation_settings.AddObserver(this);
 
-  layer_->SetOpacity(0.f);
-  layer_->SetVisible(false);
+  layer()->SetOpacity(0.f);
+  layer()->SetVisible(false);
 }
 
 void HoldingSpaceTrayIconPreview::AnimateShift(base::TimeDelta delay) {
@@ -229,7 +229,7 @@ void HoldingSpaceTrayIconPreview::AnimateShift(base::TimeDelta delay) {
   pending_index_.reset();
 
   bool created_layer = false;
-  if (!layer_ && NeedsLayer()) {
+  if (!layer() && NeedsLayer()) {
     CreateLayer(transform_);
     created_layer = true;
   }
@@ -243,15 +243,15 @@ void HoldingSpaceTrayIconPreview::AnimateShift(base::TimeDelta delay) {
   AdjustForShelfAlignmentAndTextDirection(&translation);
   transform_.Translate(translation);
 
-  if (!layer_)
+  if (!layer())
     return;
 
-  // If the `layer_` has just been created because it is shifting into the
+  // If the `layer()` has just been created because it is shifting into the
   // viewport, animate in its opacity.
   if (created_layer)
-    layer_->SetOpacity(0.f);
+    layer()->SetOpacity(0.f);
 
-  ui::ScopedLayerAnimationSettings scoped_settings(layer_->GetAnimator());
+  ui::ScopedLayerAnimationSettings scoped_settings(layer()->GetAnimator());
   scoped_settings.AddObserver(this);
   scoped_settings.SetPreemptionStrategy(
       ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
@@ -275,7 +275,7 @@ void HoldingSpaceTrayIconPreview::AnimateShift(base::TimeDelta delay) {
   shift->set_tween_type(gfx::Tween::FAST_OUT_SLOW_IN);
   transform_sequence->AddElement(std::move(shift));
 
-  layer_->GetAnimator()->StartTogether(
+  layer()->GetAnimator()->StartTogether(
       {opacity_sequence.release(), transform_sequence.release()});
 }
 
@@ -306,8 +306,8 @@ void HoldingSpaceTrayIconPreview::OnShelfAlignmentChanged(
   // updated. First stop the current animation to immediately advance to target
   // end values.
   const auto weak_ptr = weak_factory_.GetWeakPtr();
-  if (layer_ && layer_->GetAnimator()->is_animating())
-    layer_->GetAnimator()->StopAnimating();
+  if (layer() && layer()->GetAnimator()->is_animating())
+    layer()->GetAnimator()->StopAnimating();
 
   // This instance may have been deleted as a result of stopping the current
   // animation if it was in the process of animating out.
@@ -332,9 +332,9 @@ void HoldingSpaceTrayIconPreview::OnShelfAlignmentChanged(
   swapped_transform.Translate(translation.y(), translation.x());
   transform_ = swapped_transform;
 
-  if (layer_) {
+  if (layer()) {
     UpdateLayerBounds();
-    layer_->SetTransform(transform_);
+    layer()->SetTransform(transform_);
   }
 }
 
@@ -351,8 +351,8 @@ void HoldingSpaceTrayIconPreview::OnShelfConfigChanged() {
   // be updated. First stop the current animation to immediately advance to
   // target end values.
   const auto weak_ptr = weak_factory_.GetWeakPtr();
-  if (layer_ && layer_->GetAnimator()->is_animating())
-    layer_->GetAnimator()->StopAnimating();
+  if (layer() && layer()->GetAnimator()->is_animating())
+    layer()->GetAnimator()->StopAnimating();
 
   // This instance may have been deleted as a result of stopping the current
   // animation if it was in the process of animating out.
@@ -366,9 +366,9 @@ void HoldingSpaceTrayIconPreview::OnShelfConfigChanged() {
   transform_.MakeIdentity();
   transform_.Translate(translation);
 
-  if (layer_) {
+  if (layer()) {
     UpdateLayerBounds();
-    layer_->SetTransform(transform_);
+    layer()->SetTransform(transform_);
   }
 
   // Invalidate `contents_image_` so it is resized.
@@ -412,10 +412,8 @@ void HoldingSpaceTrayIconPreview::OnDeviceScaleFactorChanged(
 }
 
 void HoldingSpaceTrayIconPreview::OnImplicitAnimationsCompleted() {
-  if (!NeedsLayer()) {
-    container_->layer()->Remove(layer_.get());
-    layer_.reset();
-  }
+  if (!NeedsLayer())
+    DestroyLayer();
 
   // NOTE: Running `animate_out_closure_` may delete `this`.
   if (animate_out_closure_)
@@ -424,7 +422,7 @@ void HoldingSpaceTrayIconPreview::OnImplicitAnimationsCompleted() {
 
 void HoldingSpaceTrayIconPreview::OnViewBoundsChanged(views::View* view) {
   DCHECK_EQ(container_, view);
-  if (layer_)
+  if (layer())
     UpdateLayerBounds();
 }
 
@@ -443,14 +441,21 @@ void HoldingSpaceTrayIconPreview::OnHoldingSpaceItemImageChanged() {
 
 void HoldingSpaceTrayIconPreview::CreateLayer(
     const gfx::Transform& initial_transform) {
-  DCHECK(!layer_);
-  layer_ = std::make_unique<ui::Layer>(ui::LAYER_TEXTURED);
-  layer_->SetFillsBoundsOpaquely(false);
-  layer_->SetTransform(initial_transform);
-  layer_->set_delegate(this);
+  DCHECK(!layer());
+  DCHECK(!layer_owner_.OwnsLayer());
+  auto new_layer = std::make_unique<ui::Layer>(ui::LAYER_TEXTURED);
+  new_layer->SetFillsBoundsOpaquely(false);
+  new_layer->SetTransform(initial_transform);
+  new_layer->set_delegate(this);
+  layer_owner_.Reset(std::move(new_layer));
+
   UpdateLayerBounds();
+  container_->layer()->Add(layer());
+}
 
-  container_->layer()->Add(layer_.get());
+void HoldingSpaceTrayIconPreview::DestroyLayer() {
+  if (layer())
+    layer_owner_.ReleaseLayer();
 }
 
 bool HoldingSpaceTrayIconPreview::NeedsLayer() const {
@@ -458,8 +463,8 @@ bool HoldingSpaceTrayIconPreview::NeedsLayer() const {
 }
 
 void HoldingSpaceTrayIconPreview::InvalidateLayer() {
-  if (layer_)
-    layer_->SchedulePaint(gfx::Rect(layer_->size()));
+  if (layer())
+    layer()->SchedulePaint(gfx::Rect(layer()->size()));
 }
 
 void HoldingSpaceTrayIconPreview::AdjustForShelfAlignmentAndTextDirection(
@@ -477,11 +482,11 @@ void HoldingSpaceTrayIconPreview::AdjustForShelfAlignmentAndTextDirection(
 }
 
 void HoldingSpaceTrayIconPreview::UpdateLayerBounds() {
-  DCHECK(layer_);
-  // With a horizontal shelf in RTL, `layer_` is aligned with its parent layer's
-  // right bound and translated with a negative offset. In all other cases,
-  // `layer_` is aligned with its parent layer's left/top bound and translated
-  // with a positive offset.
+  DCHECK(layer());
+  // With a horizontal shelf in RTL, `layer()` is aligned with its parent
+  // layer's right bound and translated with a negative offset. In all other
+  // cases, `layer()` is aligned with its parent layer's left/top bound and
+  // translated with a positive offset.
   const gfx::Size size = GetPreviewSize();
   gfx::Point origin;
   if (shelf_->IsHorizontalAlignment()) {
@@ -491,8 +496,8 @@ void HoldingSpaceTrayIconPreview::UpdateLayerBounds() {
     origin.Offset(0, (container_bounds.height() - size.height()) / 2);
   }
   gfx::Rect bounds(origin, size);
-  if (bounds != layer_->bounds())
-    layer_->SetBounds(bounds);
+  if (bounds != layer()->bounds())
+    layer()->SetBounds(bounds);
 }
 
 }  // namespace ash

ash/system/holding_space/holding_space_tray_icon_preview.h

@@ -75,7 +75,7 @@ class ASH_EXPORT HoldingSpaceTrayIconPreview
   // Returns the holding space `item_` visually represented by this preview.
   const HoldingSpaceItem* item() const { return item_; }
 
-  ui::Layer* layer() { return layer_.get(); }
+  ui::Layer* layer() { return layer_owner_.layer(); }
 
   const base::Optional<size_t>& index() const { return index_; }
 
@@ -99,22 +99,26 @@ class ASH_EXPORT HoldingSpaceTrayIconPreview
   // representation gets updated.
   void OnHoldingSpaceItemImageChanged();
 
-  // Creates the `layer_` for this preview. Note that `layer_` may be created
-  // multiple times throughout this preview's lifetime as `layer_` will only
-  // exist while in the viewport for the holding space tray `container_`.
-  // |initial_transform| - The transform that should be set on the layer.
+  // Creates a layer for this preview. The layer will be owned by
+  // `layer_owner_`. Note that a layer may be created multiple times throughout
+  // this preview's lifetime as the preview will only have a layer while in the
+  // viewport for the holding space tray `container_`. |initial_transform| - The
+  // transform that should be set on the layer.
   void CreateLayer(const gfx::Transform& initial_transform);
 
+  // Destroys the layer for this preview, if it was previously created.
+  void DestroyLayer();
+
   // Returns whether this preview needs a layer for its current `transform_`.
-  // Since we only maintain `layer_` while it appears in the viewport for the
-  // holding space tray `container_`, this is used to gate creation/deletion of
-  // `layer_`.
+  // Since 'layer_owner_' has a layer only while the preview appears in the
+  // viewport for the holding space tray `container_`, this is used to gate
+  // creation/deletion of the preview layer.
   bool NeedsLayer() const;
 
-  // Schedules repaint of `layer_`, no-oping if it doesn't exist.
+  // Schedules repaint of `layer()`, no-oping if it doesn't exist.
   void InvalidateLayer();
 
-  // Updates the bounds of `layer_`.
+  // Updates the bounds of `layer()`.
   void UpdateLayerBounds();
 
   // Adjusts the specified `vector_2df` for shelf alignment and text direction.
@@ -138,20 +142,20 @@ class ASH_EXPORT HoldingSpaceTrayIconPreview
 
   // A cached representation of the associated holding space `item_`'s image
   // which has been cropped, resized, and clipped to a circle to be painted at
-  // `layer_`'s contents bounds.
+  // `layer()`'s contents bounds.
   gfx::ImageSkia contents_image_;
 
-  // This is a proxy for `layer_`'s transform and represents the target
-  // position of this preview. Because `layer_` only exists while in
+  // This is a proxy for `layer()`'s transform and represents the target
+  // position of this preview. Because `layer()` only exists while in
   // `container_`'s viewport, we need to manage transform ourselves and continue
-  // to update it even when `layer_` doesn't exist.
+  // to update it even when `layer()` doesn't exist.
   gfx::Transform transform_;
 
   // The layer serving as the visual representation of the associated holding
   // space `item_` in the holding space icon in the shelf. This only exists
   // while in the `container_`s viewport as determined by the current
   // `transform_`.
-  std::unique_ptr<ui::Layer> layer_;
+  ui::LayerOwner layer_owner_;
 
   // Closure to invoke on completion of `AnimateOut()`. It is expected that this
   // preview may be deleted during invocation.
@@ -169,9 +173,9 @@ class ASH_EXPORT HoldingSpaceTrayIconPreview
   // `contents_image_`.
   base::CallbackListSubscription image_subscription_;
 
-  // The `layer_` for this preview is parented by `container_`'s layer. It is
+  // The `layer()` for this preview is parented by `container_`'s layer. It is
   // necessary to observe and react to bounds changes in `container_` to keep
-  // `layer_`'s bounds in sync.
+  // `layer()`'s bounds in sync.
   base::ScopedObservation<views::View, views::ViewObserver> container_observer_{
       this};