Do not hide shadow underlay for max/fullscreen state even if the client disables the shadow.

components/exo/shell_surface.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/exo/shell_surface.h"
 
 #include <algorithm>
 
 #include "ash/frame/custom_frame_view_ash.h"
 #include "ash/public/cpp/shell_window_ids.h"
@@ skipping 609 matching lines @@
     DLOG(WARNING) << "Surface geometry must be non-empty";
     return;
   }
 
   pending_geometry_ = geometry;
 }
 
 void ShellSurface::SetRectangularShadowEnabled(bool enabled) {
   TRACE_EVENT1("exo", "ShellSurface::SetRectangularShadowEnabled", "enabled",
                enabled);
+  if (shadow_underlay_in_surface_)
+    ResetShadowWindows();

[reveman, 2017/04/25 18:04:40]

Should these calls be deferred until commit? What if the client is waiting a
significant amount of time before commit? Will it cause artifacts to do it here?

[oshima, 2017/04/26 16:36:11]

Good point. Done.

+
   shadow_underlay_in_surface_ = false;
   shadow_enabled_ = enabled;
 }
 
 void ShellSurface::SetRectangularShadow_DEPRECATED(
     const gfx::Rect& content_bounds) {
   TRACE_EVENT1("exo", "ShellSurface::SetRectangularShadow_DEPRECATED",
                "content_bounds", content_bounds.ToString());
+  if (shadow_underlay_in_surface_)
+    ResetShadowWindows();

[reveman, 2017/04/25 18:04:40]

ditto

   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());
+  if (!shadow_underlay_in_surface_)
+    ResetShadowWindows();

[reveman, 2017/04/25 18:04:40]

ditto

   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;
 }
@@ skipping 212 matching lines @@
   if (bounds_mode_ != BoundsMode::CLIENT)
     return WidgetDelegate::GetSavedWindowPlacement(widget, bounds, show_state);
   return false;
 }
 
 void ShellSurface::WindowClosing() {
   if (resizer_)
     EndDrag(true /* revert */);
   SetEnabled(false);
   widget_ = nullptr;
-  shadow_overlay_ = nullptr;
-  shadow_underlay_ = nullptr;
+  ResetShadowWindows();

[reveman, 2017/04/25 18:04:40]

is this needed? prior we did this to avoid leaving pointers around that point to
instances that have been destroyed but now that we own them..

[oshima, 2017/04/26 16:36:11]

This is just for the safety because they'll never be and can't be reused, but
I'm fine to remove if you prefer. LMK.

[reveman, 2017/04/26 18:00:56]

Let's remove it if not needed as then there's also no need to add a
ResetShadowWindows function

[oshima, 2017/04/26 21:45:01]

Done.

 }
 
 views::Widget* ShellSurface::GetWidget() {
   return widget_;
 }
 
 const views::Widget* ShellSurface::GetWidget() const {
   return widget_;
 }
 
@@ skipping 671 matching lines @@
 
   surface_->window()->SetBounds(
       gfx::Rect(GetSurfaceOrigin() + client_view_bounds.OffsetFromOrigin(),
                 surface_->window()->layer()->size()));
 }
 
 void ShellSurface::UpdateShadow() {
   if (!widget_ || !surface_)
     return;
   aura::Window* window = widget_->GetNativeWindow();
-  if (!shadow_enabled_) {
+
+  bool underlay_capture_events =
+      WMHelper::GetInstance()->IsSpokenFeedbackEnabled() && widget_->IsActive();
+  bool black_background_enabled =
+      ((widget_->IsFullscreen() || widget_->IsMaximized()) ||
+       underlay_capture_events) &&
+      ash::wm::GetWindowState(window)->allow_set_bounds_direct() &&
+      window->layer()->GetTargetTransform().IsIdentity();
+
+  if (!shadow_enabled_ && !black_background_enabled) {
     wm::SetShadowElevation(window, wm::ShadowElevation::NONE);
     if (shadow_underlay_)
       shadow_underlay_->Hide();
   } else {
     wm::SetShadowElevation(window, wm::ShadowElevation::MEDIUM);
     gfx::Rect shadow_content_bounds =
         gfx::ScaleToEnclosedRect(shadow_content_bounds_, 1.f / scale_);
 
     // Convert from screen to display coordinates.
     if (!shadow_content_bounds.IsEmpty()) {
@@ skipping 26 matching lines @@
         shadow_content_bounds.set_origin(origin);
       }
     }
 
     gfx::Point shadow_origin = shadow_content_bounds.origin();
     shadow_origin -= window->bounds().OffsetFromOrigin();
     gfx::Rect shadow_bounds(shadow_origin, shadow_content_bounds.size());
 
     // Always create and show the underlay, even in maximized/fullscreen.
     if (!shadow_underlay_) {
-      shadow_underlay_ = new aura::Window(nullptr);
+      shadow_underlay_ = base::MakeUnique<aura::Window>(nullptr);
+      shadow_underlay_->set_owned_by_parent(false);
       shadow_underlay_event_handler_ =
           base::MakeUnique<ShadowUnderlayEventHandler>();
       shadow_underlay_->SetTargetHandler(shadow_underlay_event_handler_.get());
-      DCHECK(shadow_underlay_->owned_by_parent());
+      DCHECK(!shadow_underlay_->owned_by_parent());
       // Ensure the background area inside the shadow is solid black.
       // Clients that provide translucent contents should not be using
       // rectangular shadows as this method requires opaque contents to
       // cast a shadow that represent it correctly.
       shadow_underlay_->Init(ui::LAYER_SOLID_COLOR);
       shadow_underlay_->layer()->SetColor(SK_ColorBLACK);
       DCHECK(shadow_underlay_->layer()->fills_bounds_opaquely());
       if (shadow_underlay_in_surface_) {
-        surface_->window()->AddChild(shadow_underlay_);
-        surface_->window()->StackChildAtBottom(shadow_underlay_);
+        surface_->window()->AddChild(shadow_underlay());
+        surface_->window()->StackChildAtBottom(shadow_underlay());
       } else {
-        window->AddChild(shadow_underlay_);
-        window->StackChildAtBottom(shadow_underlay_);
+        window->AddChild(shadow_underlay());
+        window->StackChildAtBottom(shadow_underlay());
       }
     }
 
-    bool underlay_capture_events =
-        WMHelper::GetInstance()->IsSpokenFeedbackEnabled() &&
-        widget_->IsActive();
-
     float shadow_underlay_opacity = shadow_background_opacity_;
 
     // Put the black background layer behind the window if
     // 1) the window is in immersive fullscreen, maximized or is active with
     //    spoken feedback enabled.
     // 2) the window can control the bounds of the window in fullscreen (
     //    thus the background can be visible).
     // 3) the window has no transform (the transformed background may
     //    not cover the entire background, e.g. overview mode).
-    if ((widget_->IsFullscreen() || widget_->IsMaximized() ||
-         underlay_capture_events) &&
-        ash::wm::GetWindowState(window)->allow_set_bounds_direct() &&
-        window->layer()->GetTargetTransform().IsIdentity()) {
+    if (black_background_enabled) {
       if (shadow_underlay_in_surface_) {
         shadow_underlay_bounds = gfx::Rect(surface_->window()->bounds().size());
       } else {
         gfx::Point origin;
         origin -= window->bounds().origin().OffsetFromOrigin();
         shadow_bounds.set_origin(origin);
         shadow_bounds.set_size(window->parent()->bounds().size());
       }
       shadow_underlay_opacity = 1.0f;
     }
 
     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.
     if (frame_enabled_) {
       shadow_underlay_bounds.Intersect(
           widget_->non_client_view()->frame_view()->GetBoundsForClientView());
     }
 
     shadow_underlay_->SetBounds(shadow_underlay_bounds);
 
-    shadow_underlay_->Show();
+    if (!shadow_underlay_->IsVisible())

[oshima, 2017/04/25 17:04:40]

Not sure why I missed the original CL, but this is additional change to avoid
DCHECK failure. 

+      shadow_underlay_->Show();
 
     // TODO(oshima): Setting to the same value should be no-op.
     // crbug.com/642223.
     if (shadow_underlay_opacity !=
         shadow_underlay_->layer()->GetTargetOpacity()) {
       shadow_underlay_->layer()->SetOpacity(shadow_underlay_opacity);
     }
 
     wm::Shadow* shadow = wm::ShadowController::GetShadowForWindow(window);
     // Maximized/Fullscreen window does not create a shadow.
     if (!shadow)
       return;
 
     if (!shadow_overlay_) {
-      shadow_overlay_ = new aura::Window(nullptr);
-      DCHECK(shadow_overlay_->owned_by_parent());
+      shadow_overlay_ = base::MakeUnique<aura::Window>(nullptr);
+      shadow_overlay_->set_owned_by_parent(false);
+      DCHECK(!shadow_overlay_->owned_by_parent());
       shadow_overlay_->set_ignore_events(true);
       shadow_overlay_->Init(ui::LAYER_NOT_DRAWN);
       shadow_overlay_->layer()->Add(shadow->layer());
-      window->AddChild(shadow_overlay_);
+      window->AddChild(shadow_overlay());
 
       if (shadow_underlay_in_surface_) {
-        window->StackChildBelow(shadow_overlay_, surface_->window());
+        window->StackChildBelow(shadow_overlay(), surface_->window());
       } else {
-        window->StackChildAbove(shadow_overlay_, shadow_underlay_);
+        window->StackChildAbove(shadow_overlay(), shadow_underlay());
       }
       shadow_overlay_->Show();
     }
     shadow_overlay_->SetBounds(shadow_bounds);
     shadow->SetContentBounds(gfx::Rect(shadow_bounds.size()));
     // Surfaces that can't be activated are usually menus and tooltips. Use a
     // small style shadow for them.
     if (!activatable_)
       shadow->SetElevation(wm::ShadowElevation::SMALL);
     // We don't have rounded corners unless frame is enabled.
     if (!frame_enabled_)
       shadow->SetRoundedCornerRadius(0);
   }
 }
 
 gfx::Point ShellSurface::GetMouseLocation() const {
   aura::Window* const root_window = widget_->GetNativeWindow()->GetRootWindow();
   gfx::Point location =
       root_window->GetHost()->dispatcher()->GetLastMouseLocationInRoot();
   aura::Window::ConvertPointToTarget(
       root_window, widget_->GetNativeWindow()->parent(), &location);
   return location;
 }
 
+void ShellSurface::ResetShadowWindows() {
+  shadow_overlay_.reset();
+  shadow_underlay_.reset();
+}
+
 }  // namespace exo