Make Shadow work ui::RecreateLayers()

ui::RecreateLayers() works only with LayerOwners. This means that
Shadow which creates its own layers won't be recreated and the old
layers won't be returned in the old layer tree returned by
ui::RecreateLayers().

This fixes the shadow disappearing for the window-move-to-desk
animation.

BUG=977434
TEST=Manually, shadow should show while animating, no change in behavior.

Change-Id: Iabd12036821a35b56804b5ac774714598fb39e95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1711046
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681158}

ash/public/cpp/view_shadow.cc

@@ -18,6 +18,7 @@ ViewShadow::ViewShadow(views::View* view, int elevation)
   view_->AddLayerBeneathView(shadow_->layer());
   shadow_->SetContentBounds(view_->layer()->bounds());
   view_->AddObserver(this);
+  shadow_->AddObserver(this);
 }
 
 ViewShadow::~ViewShadow() {
@@ -32,11 +33,19 @@ void ViewShadow::SetRoundedCornerRadius(int corner_radius) {
   shadow_->SetRoundedCornerRadius(corner_radius);
 }
 
+void ViewShadow::OnLayerRecreated(ui::Layer* old_layer) {
+  if (!view_)
+    return;
+  view_->RemoveLayerBeneathViewKeepInLayerTree(old_layer);
+  view_->AddLayerBeneathView(shadow_->layer());
+}
+
 void ViewShadow::OnLayerTargetBoundsChanged(views::View* view) {
   shadow_->SetContentBounds(view->layer()->bounds());
 }
 
 void ViewShadow::OnViewIsDeleting(views::View* view) {
+  shadow_->RemoveObserver(this);
   shadow_.reset();
   view_->RemoveObserver(this);
   view_ = nullptr;

ash/public/cpp/view_shadow.h

@@ -9,6 +9,7 @@
 
 #include "ash/public/cpp/ash_public_export.h"
 #include "base/macros.h"
+#include "ui/compositor/layer_owner.h"
 #include "ui/views/view_observer.h"
 
 namespace ui {
@@ -19,7 +20,8 @@ namespace ash {
 
 // Manages the shadow for a view. This forces |view| to paint to layer if it's
 // not.
-class ASH_PUBLIC_EXPORT ViewShadow : public views::ViewObserver {
+class ASH_PUBLIC_EXPORT ViewShadow : public views::ViewObserver,
+                                     public ui::LayerOwner::Observer {
  public:
   ViewShadow(views::View* view, int elevation);
   ~ViewShadow() override;
@@ -27,6 +29,9 @@ class ASH_PUBLIC_EXPORT ViewShadow : public views::ViewObserver {
   // Update the corner radius of the view along with the shadow.
   void SetRoundedCornerRadius(int corner_radius);
 
+  // ui::LayerOwner::Observer:
+  void OnLayerRecreated(ui::Layer* old_layer) override;
+
   ui::Shadow* shadow() { return shadow_.get(); }
 
  private:

ui/compositor/layer_owner.cc

@@ -16,6 +16,14 @@ LayerOwner::LayerOwner(std::unique_ptr<Layer> layer) {
 
 LayerOwner::~LayerOwner() = default;
 
+void LayerOwner::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void LayerOwner::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 void LayerOwner::SetLayer(std::unique_ptr<Layer> layer) {
   DCHECK(!OwnsLayer());
   layer_owner_ = std::move(layer);
@@ -29,6 +37,16 @@ std::unique_ptr<Layer> LayerOwner::AcquireLayer() {
   return std::move(layer_owner_);
 }
 
+std::unique_ptr<Layer> LayerOwner::ReleaseLayer() {
+  layer_ = nullptr;
+  return AcquireLayer();
+}
+
+void LayerOwner::Reset(std::unique_ptr<Layer> layer) {
+  ReleaseLayer();
+  SetLayer(std::move(layer));
+}
+
 std::unique_ptr<Layer> LayerOwner::RecreateLayer() {
   std::unique_ptr<ui::Layer> old_layer(AcquireLayer());
   if (!old_layer)
@@ -63,6 +81,9 @@ std::unique_ptr<Layer> LayerOwner::RecreateLayer() {
   // state to the new layer.
   layer_->set_delegate(old_delegate);
 
+  for (auto& observer : observers_)
+    observer.OnLayerRecreated(old_layer.get());
+
   return old_layer;
 }
 

ui/compositor/layer_owner.h

@@ -9,6 +9,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "base/observer_list.h"
 #include "ui/compositor/compositor_export.h"
 #include "ui/compositor/layer.h"
 
@@ -16,9 +17,24 @@ namespace ui {
 
 class COMPOSITOR_EXPORT LayerOwner {
  public:
+  class Observer {
+   public:
+    // Called when the |layer()| of this LayerOwner has been recreated (i.e.
+    // RecreateLayer() was called). |old_layer| should not be retained after
+    // this as it may be destroyed soon.
+    // The new layer can be retrieved from LayerOwner::layer().
+    virtual void OnLayerRecreated(ui::Layer* old_layer) = 0;
+
+   protected:
+    virtual ~Observer() = default;
+  };
+
   explicit LayerOwner(std::unique_ptr<Layer> layer = nullptr);
   virtual ~LayerOwner();
 
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
   // Releases the owning reference to its layer, and returns it.
   // This is used when you need to animate the presentation of the owner just
   // prior to destroying it. The Owner can be destroyed soon after calling this
@@ -27,6 +43,13 @@ class COMPOSITOR_EXPORT LayerOwner {
   // end of ~LayerOwner().
   std::unique_ptr<Layer> AcquireLayer();
 
+  // Similar to AcquireLayer(), but layer() will be set to nullptr immediately.
+  std::unique_ptr<Layer> ReleaseLayer();
+
+  // Releases the ownership of the current layer, and takes ownership of
+  // |layer|.
+  void Reset(std::unique_ptr<Layer> layer);
+
   // Asks the owner to recreate the layer, returning the old Layer. NULL is
   // returned if there is no existing layer, or recreate is not supported.
   //
@@ -52,6 +75,8 @@ class COMPOSITOR_EXPORT LayerOwner {
   std::unique_ptr<Layer> layer_owner_;
   Layer* layer_ = nullptr;
 
+  base::ObserverList<Observer>::Unchecked observers_;
+
   DISALLOW_COPY_AND_ASSIGN(LayerOwner);
 };
 

ui/compositor_extra/shadow.cc

@@ -26,8 +26,8 @@ Shadow::~Shadow() {}
 void Shadow::Init(int elevation) {
   DCHECK_GE(elevation, 0);
   desired_elevation_ = elevation;
-  layer_.reset(new ui::Layer(ui::LAYER_NOT_DRAWN));
-  layer_->set_name("Shadow Parent Container");
+  SetLayer(std::make_unique<ui::Layer>(ui::LAYER_NOT_DRAWN));
+  layer()->set_name("Shadow Parent Container");
   RecreateShadowLayer();
 }
 
@@ -52,26 +52,26 @@ void Shadow::SetElevation(int elevation) {
   StopObservingImplicitAnimations();
 
   // The old shadow layer is the new fading out layer.
-  DCHECK(shadow_layer_);
-  fading_layer_ = std::move(shadow_layer_);
+  DCHECK(shadow_layer());
+  fading_layer_owner_.Reset(shadow_layer_owner_.ReleaseLayer());
   RecreateShadowLayer();
-  shadow_layer_->SetOpacity(0.f);
+  shadow_layer()->SetOpacity(0.f);
 
   {
     // Observe the fade out animation so we can clean up the layer when done.
-    ui::ScopedLayerAnimationSettings settings(fading_layer_->GetAnimator());
+    ui::ScopedLayerAnimationSettings settings(fading_layer()->GetAnimator());
     settings.AddObserver(this);
     settings.SetTransitionDuration(
         base::TimeDelta::FromMilliseconds(kShadowAnimationDurationMs));
-    fading_layer_->SetOpacity(0.f);
+    fading_layer()->SetOpacity(0.f);
   }
 
   {
     // We don't care to observe this one.
-    ui::ScopedLayerAnimationSettings settings(shadow_layer_->GetAnimator());
+    ui::ScopedLayerAnimationSettings settings(shadow_layer()->GetAnimator());
     settings.SetTransitionDuration(
         base::TimeDelta::FromMilliseconds(kShadowAnimationDurationMs));
-    shadow_layer_->SetOpacity(1.f);
+    shadow_layer()->SetOpacity(1.f);
   }
 }
 
@@ -85,18 +85,23 @@ void Shadow::SetRoundedCornerRadius(int rounded_corner_radius) {
 }
 
 void Shadow::OnImplicitAnimationsCompleted() {
-  fading_layer_.reset();
-  // The size needed for layer() may be smaller now that |fading_layer_| is
+  std::unique_ptr<ui::Layer> to_be_deleted = fading_layer_owner_.ReleaseLayer();
+  // The size needed for layer() may be smaller now that |fading_layer()| is
   // removed.
   UpdateLayerBounds();
 }
 
 void Shadow::RecreateShadowLayer() {
-  shadow_layer_.reset(new ui::Layer(ui::LAYER_NINE_PATCH));
-  shadow_layer_->set_name("Shadow");
-  shadow_layer_->SetVisible(true);
-  shadow_layer_->SetFillsBoundsOpaquely(false);
-  layer()->Add(shadow_layer_.get());
+  if (shadow_layer_owner_.OwnsLayer()) {
+    shadow_layer_owner_.RecreateLayer();
+  } else {
+    shadow_layer_owner_.Reset(
+        std::make_unique<ui::Layer>(ui::LAYER_NINE_PATCH));
+    shadow_layer()->set_name("Shadow");
+    shadow_layer()->SetVisible(true);
+    shadow_layer()->SetFillsBoundsOpaquely(false);
+    layer()->Add(shadow_layer());
+  }
 
   UpdateLayerBounds();
 }
@@ -117,17 +122,17 @@ void Shadow::UpdateLayerBounds() {
       gfx::ShadowDetails::Get(size_adjusted_elevation, rounded_corner_radius_);
   gfx::Insets blur_region = gfx::ShadowValue::GetBlurRegion(details.values) +
                             gfx::Insets(rounded_corner_radius_);
-  // Update |shadow_layer_| if details changed and it has been updated in
+  // Update |shadow_layer()| if details changed and it has been updated in
   // the past (|details_| is set), or elevation is non-zero.
   if ((&details != details_) && (details_ || size_adjusted_elevation)) {
-    shadow_layer_->UpdateNinePatchLayerImage(details.ninebox_image);
+    shadow_layer()->UpdateNinePatchLayerImage(details.ninebox_image);
     // The ninebox grid is defined in terms of the image size. The shadow blurs
     // in both inward and outward directions from the edge of the contents, so
     // the aperture goes further inside the image than the shadow margins (which
     // represent exterior blur).
     gfx::Rect aperture(details.ninebox_image.size());
     aperture.Inset(blur_region);
-    shadow_layer_->UpdateNinePatchLayerAperture(aperture);
+    shadow_layer()->UpdateNinePatchLayerAperture(aperture);
     details_ = &details;
   }
 
@@ -140,7 +145,7 @@ void Shadow::UpdateLayerBounds() {
 
   // When there's an old shadow fading out, the bounds of layer() have to be
   // big enough to encompass both shadows.
-  if (fading_layer_) {
+  if (fading_layer()) {
     const gfx::Rect old_layer_bounds = layer()->bounds();
     gfx::Rect combined_layer_bounds = old_layer_bounds;
     combined_layer_bounds.Union(new_layer_bounds);
@@ -149,10 +154,10 @@ void Shadow::UpdateLayerBounds() {
     // If this is reached via SetContentBounds, we might hypothetically need
     // to change the size of the fading layer, but the fade is so fast it's
     // not really an issue.
-    gfx::Rect fading_layer_bounds(fading_layer_->bounds());
+    gfx::Rect fading_layer_bounds(fading_layer()->bounds());
     fading_layer_bounds.Offset(old_layer_bounds.origin() -
                                combined_layer_bounds.origin());
-    fading_layer_->SetBounds(fading_layer_bounds);
+    fading_layer()->SetBounds(fading_layer_bounds);
 
     shadow_layer_bounds.Offset(new_layer_bounds.origin() -
                                combined_layer_bounds.origin());
@@ -160,16 +165,16 @@ void Shadow::UpdateLayerBounds() {
     layer()->SetBounds(new_layer_bounds);
   }
 
-  shadow_layer_->SetBounds(shadow_layer_bounds);
+  shadow_layer()->SetBounds(shadow_layer_bounds);
 
   // Occlude the region inside the bounding box. Occlusion uses shadow layer
   // space. See nine_patch_layer.h for more context on what's going on here.
   gfx::Rect occlusion_bounds(shadow_layer_bounds.size());
   occlusion_bounds.Inset(-margins + gfx::Insets(rounded_corner_radius_));
-  shadow_layer_->UpdateNinePatchOcclusion(occlusion_bounds);
+  shadow_layer()->UpdateNinePatchOcclusion(occlusion_bounds);
 
   // The border is the same inset as the aperture.
-  shadow_layer_->UpdateNinePatchLayerBorder(
+  shadow_layer()->UpdateNinePatchLayerBorder(
       gfx::Rect(blur_region.left(), blur_region.top(), blur_region.width(),
                 blur_region.height()));
 }

ui/compositor_extra/shadow.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "ui/compositor/layer_animation_observer.h"
+#include "ui/compositor/layer_owner.h"
 #include "ui/gfx/geometry/rect.h"
 
 namespace gfx {
@@ -19,7 +20,7 @@ namespace ui {
 class Layer;
 
 // Simple class that draws a drop shadow around content at given bounds.
-class Shadow : public ui::ImplicitAnimationObserver {
+class Shadow : public ui::ImplicitAnimationObserver, public ui::LayerOwner {
  public:
   Shadow();
   ~Shadow() override;
@@ -29,20 +30,18 @@ class Shadow : public ui::ImplicitAnimationObserver {
   // for the shadow style.
   void Init(int elevation);
 
-  // Returns |layer_.get()|. This is exposed so it can be added to the same
-  // layer as the content and stacked below it.  SetContentBounds() should be
-  // used to adjust the shadow's size and position (rather than applying
-  // transformations to this layer).
-  ui::Layer* layer() const { return layer_.get(); }
-
   // Exposed to allow setting animation parameters for bounds and opacity
   // animations.
-  ui::Layer* shadow_layer() const { return shadow_layer_.get(); }
+  ui::Layer* shadow_layer() { return shadow_layer_owner_.layer(); }
+
+  ui::Layer* fading_layer() { return fading_layer_owner_.layer(); }
 
   const gfx::Rect& content_bounds() const { return content_bounds_; }
   int desired_elevation() const { return desired_elevation_; }
 
   // Moves and resizes the shadow layer to frame |content_bounds|.
+  // This should be used to adjust the shadow's size and position (rather than
+  // applying transformations to the `layer()` of this Shadow).
   void SetContentBounds(const gfx::Rect& content_bounds);
 
   // Sets the shadow's appearance, animating opacity as necessary.
@@ -75,23 +74,19 @@ class Shadow : public ui::ImplicitAnimationObserver {
   // we need to exclude them from the occlusion area.
   int rounded_corner_radius_ = 2;
 
-  // The details of the shadow image that's currently set on |shadow_layer_|.
+  // The details of the shadow image that's currently set on |shadow_layer()|.
   // This will be null until a positive elevation has been set. Once set, it
   // will always point to a global ShadowDetails instance that is guaranteed
   // to outlive the Shadow instance. See ui/gfx/shadow_util.h for how these
   // ShadowDetails instances are created.
   const gfx::ShadowDetails* details_ = nullptr;
 
-  // The parent layer of the shadow layer. It serves as a container accessible
-  // from the outside to control the visibility of the shadow.
-  std::unique_ptr<ui::Layer> layer_;
-
-  // The actual shadow layer corresponding to a cc::NinePatchLayer.
-  std::unique_ptr<ui::Layer> shadow_layer_;
+  // The owner of the actual shadow layer corresponding to a cc::NinePatchLayer.
+  ui::LayerOwner shadow_layer_owner_;
 
   // When the elevation changes, the old shadow cross-fades with the new one.
-  // When non-null, this is an old |shadow_layer_| that's being animated out.
-  std::unique_ptr<ui::Layer> fading_layer_;
+  // When non-null, this owns an old |shadow_layer()| that's being animated out.
+  ui::LayerOwner fading_layer_owner_;
 
   // Bounds of the content that the shadow encloses.
   gfx::Rect content_bounds_;

ui/views/view.cc

@@ -552,7 +552,9 @@ void View::AddLayerBeneathView(ui::Layer* new_layer) {
   // correctly. If not, this will happen on layer creation.
   if (layer()) {
     ui::Layer* parent_layer = layer()->parent();
-    if (parent_layer)
+    // Note that |new_layer| may have already been added to the parent, for
+    // example when the layer of a LayerOwner is recreated.
+    if (parent_layer && parent_layer != new_layer->parent())
       parent_layer->Add(new_layer);
     new_layer->SetBounds(gfx::Rect(new_layer->size()) +
                          layer()->bounds().OffsetFromOrigin());
@@ -566,18 +568,23 @@ void View::AddLayerBeneathView(ui::Layer* new_layer) {
 }
 
 void View::RemoveLayerBeneathView(ui::Layer* old_layer) {
+  RemoveLayerBeneathViewKeepInLayerTree(old_layer);
+
+  // Note that |old_layer| may have already been removed from its parent.
+  ui::Layer* parent_layer = layer()->parent();
+  if (parent_layer && parent_layer == old_layer->parent())
+    parent_layer->Remove(old_layer);
+
+  CreateOrDestroyLayer();
+}
+
+void View::RemoveLayerBeneathViewKeepInLayerTree(ui::Layer* old_layer) {
   auto layer_pos =
       std::find(layers_beneath_.begin(), layers_beneath_.end(), old_layer);
   DCHECK(layer_pos != layers_beneath_.end())
       << "Attempted to remove a layer that was never added.";
   layers_beneath_.erase(layer_pos);
   old_layer->RemoveObserver(this);
-
-  ui::Layer* parent_layer = layer()->parent();
-  if (parent_layer)
-    parent_layer->Remove(old_layer);
-
-  CreateOrDestroyLayer();
 }
 
 std::vector<ui::Layer*> View::GetLayersInOrder() {

ui/views/view.h

@@ -631,6 +631,15 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,
   virtual void AddLayerBeneathView(ui::Layer* new_layer);
   virtual void RemoveLayerBeneathView(ui::Layer* old_layer);
 
+  // This is like RemoveLayerBeneathView() but doesn't remove |old_layer| from
+  // its parent. This is useful for when a layer beneth this view is owned by a
+  // ui::LayerOwner which just recreated it (by calling RecreateLayer()). In
+  // this case, this function can be called to remove it from |layers_beneath_|,
+  // and to stop observing it, but it remains in the layer tree since the
+  // expectation of ui::LayerOwner::RecreateLayer() is that the old layer
+  // remains under the same parent, and stacked above the newly cloned layer.
+  void RemoveLayerBeneathViewKeepInLayerTree(ui::Layer* old_layer);
+
   // Gets the layers associated with this view that should be immediate children
   // of the parent layer. They are returned in bottom-to-top order. This
   // includes |this->layer()| and any layers added with |AddLayerBeneathView()|.