[ash-md] Hides frame headers in overview mode not using a mask

ash/common/wm/overview/scoped_transform_overview_window.cc

Index: ash/common/wm/overview/scoped_transform_overview_window.cc
diff --git a/ash/common/wm/overview/scoped_transform_overview_window.cc b/ash/common/wm/overview/scoped_transform_overview_window.cc
index ca1974343fc8cc04797ae5dae6cc593286b98d7c..59299b0f1a941fcb5288f15ed89375c974a8fe8d 100644
--- a/ash/common/wm/overview/scoped_transform_overview_window.cc
+++ b/ash/common/wm/overview/scoped_transform_overview_window.cc
@@ -180,7 +180,7 @@ TransientDescendantIteratorRange GetTransientTreeIterator(WmWindow* window) {
 class ScopedTransformOverviewWindow::OverviewContentMask
     : public ui::LayerDelegate {
  public:
-  OverviewContentMask(int inset, int radius);
+  OverviewContentMask(float radius);

[tdanderson, 2016/07/09 21:23:21]

Make this constructor explicit

[varkha, 2016/07/09 22:00:17]

Done.

   ~OverviewContentMask() override;
 
   ui::Layer* layer() { return &layer_; }
@@ -193,16 +193,14 @@ class ScopedTransformOverviewWindow::OverviewContentMask
 
  private:
   ui::Layer layer_;
-  int inset_;
-  int radius_;
+  float radius_;
 
   DISALLOW_COPY_AND_ASSIGN(OverviewContentMask);
 };
 
 ScopedTransformOverviewWindow::OverviewContentMask::OverviewContentMask(
-    int inset,
-    int radius)
-    : layer_(ui::LAYER_TEXTURED), inset_(inset), radius_(radius) {
+    float radius)
+    : layer_(ui::LAYER_TEXTURED), radius_(radius) {
   layer_.set_delegate(this);
 }
 
@@ -214,7 +212,6 @@ void ScopedTransformOverviewWindow::OverviewContentMask::OnPaintLayer(
     const ui::PaintContext& context) {
   ui::PaintRecorder recorder(context, layer()->size());
   gfx::Rect bounds(layer()->bounds().size());
-  bounds.Inset(0, inset_, 0, 0);
 
   // Tile a window into an area, rounding the bottom corners.
   const SkRect rect = gfx::RectToSkRect(bounds);
@@ -250,6 +247,7 @@ base::Closure ScopedTransformOverviewWindow::OverviewContentMask::
 
 ScopedTransformOverviewWindow::ScopedTransformOverviewWindow(WmWindow* window)
     : window_(window),
+      alpha_shape_set_(false),
       minimized_(window->GetShowState() == ui::SHOW_STATE_MINIMIZED),
       ignored_by_shelf_(window->GetWindowState()->ignored_by_shelf()),
       overview_started_(false),
@@ -260,11 +258,6 @@ ScopedTransformOverviewWindow::ScopedTransformOverviewWindow(WmWindow* window)
 ScopedTransformOverviewWindow::~ScopedTransformOverviewWindow() {}
 
 void ScopedTransformOverviewWindow::RestoreWindow() {
-  if (ash::MaterialDesignController::IsOverviewMaterial()) {
-    window()->GetLayer()->SetMaskLayer(nullptr);
-    mask_.reset();
-  }
-
   ScopedAnimationSettings animation_settings_list;
   BeginScopedAnimation(OverviewAnimationType::OVERVIEW_ANIMATION_RESTORE_WINDOW,
                        &animation_settings_list);
@@ -288,6 +281,21 @@ void ScopedTransformOverviewWindow::RestoreWindow() {
   }
   window_->GetWindowState()->set_ignored_by_shelf(ignored_by_shelf_);
   SetOpacity(original_opacity_);
+
+  if (ash::MaterialDesignController::IsOverviewMaterial()) {
+    ui::Layer* layer = window()->GetLayer();
+    layer->SetMaskLayer(nullptr);
+    mask_.reset();
+
+    if (original_alpha_shape_) {
+      layer->SetAlphaShape(
+          base::WrapUnique(new SkRegion(*original_alpha_shape_.get())));
+    } else {
+      layer->SetAlphaShape(nullptr);
+    }
+    window()->SetMasksToBounds(false);
+    alpha_shape_set_ = false;
+  }
 }
 
 void ScopedTransformOverviewWindow::BeginScopedAnimation(
@@ -394,14 +402,30 @@ gfx::Transform ScopedTransformOverviewWindow::GetTransformForRect(
 void ScopedTransformOverviewWindow::SetTransform(
     WmWindow* root_window,
     const gfx::Transform& transform,
-    int radius) {
+    float radius) {
   DCHECK(overview_started_);
 
-  if (ash::MaterialDesignController::IsOverviewMaterial() && !mask_) {
-    mask_.reset(new OverviewContentMask(
-        window()->GetIntProperty(WmWindowProperty::TOP_VIEW_INSET), radius));
-    mask_->layer()->SetBounds(GetTargetBoundsInScreen());
+  if (ash::MaterialDesignController::IsOverviewMaterial() &&
+      &transform != &original_transform_) {

[tdanderson, 2016/07/09 21:23:21]

Hmm, shouldn't all of  SetTransform() be a no-op if this is true? i.e.,

void ScopedTransformOverviewWindow::SetTransform(...) {
  if (&transform == &original_transform)
    return;

  ...
}

That, or perhaps I don't understand why you're including this check.

[varkha, 2016/07/09 22:00:17]

No. This method is used to both set the transform on a a window that scales it
into overview and also to revert back to original transform. But when reverting
there is no point to refresh the header mask since it is about to be removed
anyway. See a call site above in RestoreWindow().

+    gfx::Rect bounds(GetTargetBoundsInScreen().size());
+    mask_.reset(new OverviewContentMask(radius));
+    mask_->layer()->SetBounds(bounds);

[tdanderson, 2016/07/09 21:23:21]

I don't understand why you're setting bounds with an origin of (0,0) (from line
410) rather than the true bounds.

[varkha, 2016/07/09 22:00:17]

This is for a mask layer - my understanding is that this is using coordinates
relative to the window()->GetLayer().

     window()->GetLayer()->SetMaskLayer(mask_->layer());
+
+    if (!alpha_shape_set_) {

[tdanderson, 2016/07/09 21:23:21]

I don't think you need |alpha_shape_set_| at all. Assuming that calling
window()->SetMasksToBounds(true) is a no-op if SMTB(true) has already been
called on window(), then it seems that lines 415-421 could be replaced with:

SkRegion* alpha_shape = window()->GetLayer()->alpha_shape();
if (!original_alpha_shape_ && alpha_shape)
  original_alpha_shape_.reset(new SKRegion(*alpha_shape));
window()->SetMasksToBounds(true);

Furthermore, line 297 seems unnecessary anyway since (iiuc) RestoreWindow() is
called right before |this| is destroyed.

[varkha, 2016/07/09 22:00:17]

Done.

+      if (window()->GetLayer()->alpha_shape())

[tdanderson, 2016/07/09 21:23:21]

{} since call to reset() spans two lines

[varkha, 2016/07/09 22:00:17]

Done.

+        original_alpha_shape_.reset(
+            new SkRegion(*window()->GetLayer()->alpha_shape()));
+      window()->SetMasksToBounds(true);
+      alpha_shape_set_ = true;
+    }
+    bounds.Inset(0, window()->GetIntProperty(WmWindowProperty::TOP_VIEW_INSET),
+                 0, 0);
+    SkRegion* region = new SkRegion;
+    region->setRect(RectToSkIRect(bounds));
+    if (original_alpha_shape_)
+      region->op(*original_alpha_shape_, SkRegion::kIntersect_Op);
+    window()->GetLayer()->SetAlphaShape(base::WrapUnique(region));

[varkha, 2016/07/09 21:07:32]

Self review: this could be done only for windows that need to mask a header
(i.e. non-zero TOP_VIEW_INSET) rather than for all windows.

[varkha, 2016/07/09 22:00:17]

Done.

   }
 
   gfx::Point target_origin(GetTargetBoundsInScreen().origin());