Use pixel coordinates for shadow underlay bounds

components/exo/shell_surface.cc

Index: components/exo/shell_surface.cc
diff --git a/components/exo/shell_surface.cc b/components/exo/shell_surface.cc
index 82f2f5db68fdedaf96c12cd14dc1f7d468b5c088..cbd7025396c999fd9b0d2da6e12b3042f6d83d25 100644
--- a/components/exo/shell_surface.cc
+++ b/components/exo/shell_surface.cc
@@ -592,24 +592,34 @@ void ShellSurface::SetGeometry(const gfx::Rect& geometry) {
   pending_geometry_ = geometry;
 }
 
-void ShellSurface::SetRectangularShadow(bool enabled) {
-  TRACE_EVENT1("exo", "ShellSurface::SetRectangularShadow", "enabled", enabled);
-
+void ShellSurface::SetEnableRectangularShadow(bool enabled) {
+  TRACE_EVENT1("exo", "ShellSurface::SetEnableRectangularShadow", "enabled",
+               enabled);
+  shadow_underlay_in_surface_ = false;
   shadow_enabled_ = enabled;
 }
 
-void ShellSurface::SetRectangularShadowContentBounds(
+void ShellSurface::SetRectangularShadow_DEPRECATED(
     const gfx::Rect& content_bounds) {
-  TRACE_EVENT1("exo", "ShellSurface::SetRectangularShadowContentBounds",
+  TRACE_EVENT1("exo", "ShellSurface::SetRectangularShadow_DEPRECATED",
                "content_bounds", content_bounds.ToString());
+  shadow_underlay_in_surface_ = false;
+  shadow_content_bounds_ = content_bounds;
+  shadow_enabled_ = !content_bounds.IsEmpty();
+}
 
+void ShellSurface::SetRectangularSurfaceShadow(
+    const gfx::Rect& content_bounds) {
+  TRACE_EVENT1("exo", "ShellSurface::SetRectangularSurfaceShadow",
+               "content_bounds", content_bounds.ToString());
+  shadow_underlay_in_surface_ = true;
   shadow_content_bounds_ = content_bounds;
+  shadow_enabled_ = !content_bounds.IsEmpty();
 }
 
 void ShellSurface::SetRectangularShadowBackgroundOpacity(float opacity) {
   TRACE_EVENT1("exo", "ShellSurface::SetRectangularShadowBackgroundOpacity",
                "opacity", opacity);
-
   shadow_background_opacity_ = opacity;
 }
 
@@ -1420,14 +1430,25 @@ void ShellSurface::UpdateShadow() {
   aura::Window* window = widget_->GetNativeWindow();
   if (!shadow_enabled_) {
     wm::SetShadowElevation(window, wm::ShadowElevation::NONE);
-    if (shadow_underlay_)
+    if (shadow_underlay_) {
       shadow_underlay_->Hide();
+      delete shadow_underlay_;

[reveman, 2017/02/06 22:38:03]

I think this would be best to do as a separate patch if you don't mind.

[oshima, 2017/02/07 02:21:45]

This was to cover the test. Done.

+      shadow_underlay_ = nullptr;
+    }
   } else {
     wm::SetShadowElevation(window, wm::ShadowElevation::MEDIUM);
-
-    gfx::Rect shadow_content_bounds = shadow_content_bounds_;
-    if (shadow_content_bounds.IsEmpty())
-      shadow_content_bounds = window->bounds();
+    gfx::Rect shadow_content_bounds =
+        gfx::ScaleToEnclosedRect(shadow_content_bounds_, 1.f / scale_);
+    gfx::Rect shadow_underlay_bounds = shadow_content_bounds_;
+    if (shadow_underlay_bounds.IsEmpty())
+      shadow_underlay_bounds = gfx::Rect(surface_->window()->bounds().size());
+
+    if (!shadow_underlay_in_surface_) {
+      shadow_content_bounds = shadow_content_bounds_;
+      if (shadow_content_bounds.IsEmpty()) {
+        shadow_content_bounds = window->bounds();
+      }
+    }
 
     // TODO(oshima): Adjust the coordinates from client screen to
     // chromeos screen when multi displays are supported.
@@ -1450,8 +1471,16 @@ void ShellSurface::UpdateShadow() {
       shadow_underlay_->Init(ui::LAYER_SOLID_COLOR);
       shadow_underlay_->layer()->SetColor(SK_ColorBLACK);
       DCHECK(shadow_underlay_->layer()->fills_bounds_opaquely());
-      window->AddChild(shadow_underlay_);
-      window->StackChildAtBottom(shadow_underlay_);
+      if (!shadow_underlay_in_surface_) {
+        window->AddChild(shadow_underlay_);
+        window->StackChildAtBottom(shadow_underlay_);
+      } else {
+        surface_->window()->AddChild(shadow_underlay_);
+      }
+    }
+
+    if (shadow_underlay_in_surface_) {

[reveman, 2017/02/06 22:38:03]

why do we need this?

[oshima, 2017/02/07 02:21:45]

This is to make sure the underlay is at the bottom of the surface layers. I made
more strict.

I did this because the tree structure under surface_ can change, but
I can remove if it's guaranteed that it will stay at the bottom
once it's moved.

[reveman, 2017/02/07 03:16:53]

I don't think we're doing anything that requires us to restack the underlay and
if we do then I'm not sure UpdateShadow is called to fix it anyhow so best to
remove unless we know a case where it's needed.

[oshima, 2017/02/07 03:21:40]

Ok, move the stacking code into to above if block.

+      surface_->window()->StackChildAtBottom(shadow_underlay_);
     }
 
     bool underlay_capture_events =
@@ -1469,14 +1498,19 @@ void ShellSurface::UpdateShadow() {
     if ((widget_->IsFullscreen() || underlay_capture_events) &&
         ash::wm::GetWindowState(window)->allow_set_bounds_in_maximized() &&
         window->layer()->GetTargetTransform().IsIdentity()) {
-      gfx::Point origin;
-      origin -= window->bounds().origin().OffsetFromOrigin();
-      shadow_bounds.set_origin(origin);
-      shadow_bounds.set_size(window->parent()->bounds().size());
+      if (!shadow_underlay_in_surface_) {
+        gfx::Point origin;
+        origin -= window->bounds().origin().OffsetFromOrigin();
+        shadow_bounds.set_origin(origin);
+        shadow_bounds.set_size(window->parent()->bounds().size());
+      } else {
+        shadow_underlay_bounds = gfx::Rect(surface_->window()->bounds().size());
+      }
       shadow_underlay_opacity = 1.0f;
     }
 
-    gfx::Rect shadow_underlay_bounds = shadow_bounds;
+    if (!shadow_underlay_in_surface_)
+      shadow_underlay_bounds = shadow_bounds;
 
     // Constrain the underlay bounds to the client area in case shell surface
     // frame is enabled.
@@ -1508,7 +1542,12 @@ void ShellSurface::UpdateShadow() {
       shadow_overlay_->Init(ui::LAYER_NOT_DRAWN);
       shadow_overlay_->layer()->Add(shadow->layer());
       window->AddChild(shadow_overlay_);
-      window->StackChildAbove(shadow_overlay_, shadow_underlay_);
+
+      if (!shadow_underlay_in_surface_) {

[reveman, 2017/02/06 22:38:03]

nit: remove "!" and flip the contents of each clause

[oshima, 2017/02/07 02:21:45]

Done.

+        window->StackChildAbove(shadow_overlay_, shadow_underlay_);
+      } else {
+        window->StackChildBelow(shadow_overlay_, surface_->window());
+      }
       shadow_overlay_->Show();
     }
     shadow_overlay_->SetBounds(shadow_bounds);