Add an aura property for window corner radius. Propagate to mus.

Fix a bug in Mash where a root window would be given two shadows:
one each process. The one in the browser process would be clipped
to the root window bounds, so is only visible when the window has
rounded corners.

The corner radius is currently only used for window server shadows.
(E.g. it's not used in aura::Window::HitTest()).

Bug: 811859, 801583
Change-Id: I260b0ff71e01181952d0ef4dfbfebd8ae95133b5
Reviewed-on: https://chromium-review.googlesource.com/925943
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539754}

chrome/browser/ui/views/omnibox/rounded_omnibox_results_frame.cc

@@ -94,7 +94,9 @@ RoundedOmniboxResultsFrame::~RoundedOmniboxResultsFrame() = default;
 void RoundedOmniboxResultsFrame::OnBeforeWidgetInit(
     views::Widget::InitParams* params) {
   params->shadow_type = views::Widget::InitParams::SHADOW_TYPE_DROP;
+  params->corner_radius = GetLayoutConstant(LOCATION_BAR_BUBBLE_CORNER_RADIUS);
   params->shadow_elevation = kElevation;
+  params->name = "RoundedOmniboxResultsFrameWindow";
 }
 
 // static
@@ -128,25 +130,5 @@ void RoundedOmniboxResultsFrame::AddedToWidget() {
 #if defined(USE_AURA)
   GetWidget()->GetNativeWindow()->SetEventTargeter(
       std::make_unique<ResultsTargeter>(content_insets_.top()));
-
-  // This works well on ChromeOS. On other platforms, some tricks may be needed
-  // to ensure the shadow is not clipped to the parent Widget bounds, since it
-  // can peek outside the browser bounds. It is possible to just put the shadow
-  // layer into the Widget's layer hierarchy, but that needs further logic to
-  // ensure mouse events pass through it to the browser below.
-  auto* shadow_layer =
-      wm::ShadowController::GetShadowForWindow(GetWidget()->GetNativeWindow());
-#if defined(OS_WIN)
-  // To get a shadow from the ShadowController on Windows, the Widget can not be
-  // top-level (it must be a child within another DesktopWindowTreeHost).
-  // ChromeViewsDelegate decides this: it chooses top-level because software
-  // compositing is force-enabled for the Widget on windows, and because
-  // top-level Widgets are preferred under Aero (compositing window manager).
-  if (!shadow_layer)
-    return;
-#endif
-  DCHECK(shadow_layer);
-  shadow_layer->SetRoundedCornerRadius(
-      GetLayoutConstant(LOCATION_BAR_BUBBLE_CORNER_RADIUS));
 #endif
 }

components/exo/client_controlled_shell_surface_unittest.cc

@@ -250,11 +250,8 @@ TEST_F(ClientControlledShellSurfaceTest, SurfaceShadow) {
 
   aura::Window* window = shell_surface->GetWidget()->GetNativeWindow();
 
-  // 1) Initial state, no shadow.
-  wm::Shadow* shadow = wm::ShadowController::GetShadowForWindow(window);
-  ASSERT_TRUE(shadow);
-  EXPECT_FALSE(shadow->layer()->visible());
-
+  // 1) Initial state, no shadow (SurfaceFrameType is NONE);
+  EXPECT_FALSE(wm::ShadowController::GetShadowForWindow(window));
   std::unique_ptr<Display> display(new Display);
 
   // 2) Just creating a sub surface won't create a shadow.
@@ -266,11 +263,14 @@ TEST_F(ClientControlledShellSurfaceTest, SurfaceShadow) {
       display->CreateSubSurface(child.get(), surface.get()));
   surface->Commit();
 
-  EXPECT_FALSE(shadow->layer()->visible());
+  EXPECT_FALSE(wm::ShadowController::GetShadowForWindow(window));
 
   // 3) Create a shadow.
+  surface->SetFrame(SurfaceFrameType::NORMAL);
   shell_surface->SetShadowBounds(gfx::Rect(10, 10, 100, 100));
   surface->Commit();
+  wm::Shadow* shadow = wm::ShadowController::GetShadowForWindow(window);
+  ASSERT_TRUE(shadow);
   EXPECT_TRUE(shadow->layer()->visible());
 
   gfx::Rect before = shadow->layer()->bounds();
@@ -321,6 +321,7 @@ TEST_F(ClientControlledShellSurfaceTest, ShadowWithStateChange) {
   const gfx::Rect original_bounds(gfx::Point(10, 10), content_size);
   shell_surface->SetGeometry(original_bounds);
   surface->Attach(buffer.get());
+  surface->SetFrame(SurfaceFrameType::NORMAL);
   surface->Commit();
 
   // Placing a shadow at screen origin will make the shadow's origin (-10, -10).
@@ -379,6 +380,7 @@ TEST_F(ClientControlledShellSurfaceTest, ShadowWithTransform) {
   const gfx::Rect original_bounds(gfx::Point(10, 10), content_size);
   shell_surface->SetGeometry(original_bounds);
   surface->Attach(buffer.get());
+  surface->SetFrame(SurfaceFrameType::NORMAL);
   surface->Commit();
 
   aura::Window* window = shell_surface->GetWidget()->GetNativeWindow();
@@ -408,6 +410,7 @@ TEST_F(ClientControlledShellSurfaceTest, ShadowStartMaximized) {
       exo_test_helper()->CreateClientControlledShellSurface(surface.get());
   shell_surface->SetMaximized();
   surface->Attach(buffer.get());
+  surface->SetFrame(SurfaceFrameType::NORMAL);
   surface->Commit();
 
   views::Widget* widget = shell_surface->GetWidget();
@@ -416,7 +419,7 @@ TEST_F(ClientControlledShellSurfaceTest, ShadowStartMaximized) {
   // There is no shadow when started in maximized state.
   EXPECT_FALSE(wm::ShadowController::GetShadowForWindow(window));
 
-  // Sending a shadow bounds in maximized state won't create a shaodw.
+  // Sending a shadow bounds in maximized state won't create a shadow.
   shell_surface->SetShadowBounds(gfx::Rect(10, 10, 100, 100));
   surface->Commit();
   EXPECT_FALSE(wm::ShadowController::GetShadowForWindow(window));

components/exo/shell_surface_base.cc

@@ -904,7 +904,8 @@ void ShellSurfaceBase::OnWindowBoundsChanged(aura::Window* window,
     UpdateSurfaceBounds();
 
     // The shadow size may be updated to match the widget. Change it back
-    // to the shadow content size.
+    // to the shadow content size. Note that this relies on wm::ShadowController
+    // being notified of the change before |this|.
     UpdateShadow();
 
     Configure();

services/ui/public/interfaces/window_manager.mojom

@@ -153,6 +153,10 @@ interface WindowManager {
   // Type: ShowState.
   const string kShowState_Property = "prop:show-state";
 
+  // The window corner radius in DIPs. Maps to
+  // aura::client::kWindowCornerRadiusKey. Type: int.
+  const string kWindowCornerRadius_Property = "prop:window-corner-radius";
+
   // The window icon; typically 16x16 for titlebars. Type: SkBitmap
   const string kWindowIcon_Property = "prop:window-icon";
 

testing/buildbot/filters/mash.browser_tests.filter

@@ -259,9 +259,8 @@
 -PowerPolicyLoginScreenBrowserTest.SetDevicePolicy
 -EnterpriseEnrollmentTest.TestActiveDirectoryEnrollment_Success
 
-# Access aura window shadow layer. http://crbug.com/811859
--OmniboxPopupContentsViewTest.PopupAlignment/Rounded
--RoundedOmniboxPopupContentsViewTest.*
+# Needs EventGenerator to work across window tree hosts. crbug.com/814675
+-RoundedOmniboxPopupContentsViewTest.ClickOmnibox*
 
 # HostedAppMenu needs porting to BrowserNonClientFrameViewMus crbug.com/813666
 -HostedAppPWAOnlyTest.AppInfoOpensPageInfo*

ui/aura/client/aura_constants.cc

@@ -73,6 +73,7 @@ DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(base::string16, kTitleKey, nullptr);
 DEFINE_UI_CLASS_PROPERTY_KEY(int, kTopViewInset, 0);
 DEFINE_UI_CLASS_PROPERTY_KEY(SkColor, kTopViewColor, SK_ColorTRANSPARENT);
 DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(gfx::ImageSkia, kWindowIconKey, nullptr);
+DEFINE_UI_CLASS_PROPERTY_KEY(int, kWindowCornerRadiusKey, -1);
 DEFINE_UI_CLASS_PROPERTY_KEY(ui::mojom::WindowType,
                            kWindowTypeKey,
                            ui::mojom::WindowType::UNKNOWN);

ui/aura/client/aura_constants.h

@@ -152,6 +152,10 @@ AURA_EXPORT extern const WindowProperty<gfx::ImageSkia*>* const kWindowIconKey;
 // Indicates the type of embedding within the given window.
 AURA_EXPORT extern const WindowProperty<WindowEmbedType>* const kEmbedType;
 
+// The corner radius of a window in DIPs. Currently only used for shadows.
+// Default is -1, meaning "unspecified". 0 Ensures corners are square.
+AURA_EXPORT extern const WindowProperty<int>* const kWindowCornerRadiusKey;
+
 AURA_EXPORT extern const WindowProperty<ui::mojom::WindowType>* const
     kWindowTypeKey;
 

ui/aura/mus/property_converter.cc

@@ -49,6 +49,10 @@ bool ValidateShowState(int64_t value) {
          value == int64_t(ui::mojom::ShowState::FULLSCREEN);
 }
 
+bool ValidateWindowCornerRadius(int64_t value) {
+  return value >= -1;
+}
+
 }  // namespace
 
 PropertyConverter::PrimitiveProperty::PrimitiveProperty() {}
@@ -96,6 +100,10 @@ PropertyConverter::PropertyConverter() {
                          ui::mojom::WindowManager::kName_Property);
   RegisterString16Property(client::kTitleKey,
                            ui::mojom::WindowManager::kWindowTitle_Property);
+  RegisterPrimitiveProperty(
+      client::kWindowCornerRadiusKey,
+      ui::mojom::WindowManager::kWindowCornerRadius_Property,
+      base::BindRepeating(&ValidateWindowCornerRadius));
 }
 
 PropertyConverter::~PropertyConverter() {}

ui/aura/mus/window_tree_client_unittest.cc

@@ -537,6 +537,9 @@ TEST_F(WindowTreeClientWmTest, OnWindowHierarchyChangedWithProperties) {
   data->properties[ui::mojom::WindowManager::kWindowType_InitProperty] =
       mojo::ConvertTo<std::vector<uint8_t>>(
           static_cast<int32_t>(ui::mojom::WindowType::BUBBLE));
+  constexpr int kWindowCornerRadiusValue = 6;
+  data->properties[ui::mojom::WindowManager::kWindowCornerRadius_Property] =
+      ConvertToPropertyTransportValue(kWindowCornerRadiusValue);
   data->parent_id = server_id(root_window());
   data->window_id = child_window_id;
   data->bounds = gfx::Rect(1, 2, 3, 4);
@@ -551,6 +554,8 @@ TEST_F(WindowTreeClientWmTest, OnWindowHierarchyChangedWithProperties) {
   Window* child = root_window()->children()[0];
   EXPECT_FALSE(child->TargetVisibility());
   EXPECT_EQ(server_test_property1_value, child->GetProperty(kTestPropertyKey1));
+  EXPECT_EQ(kWindowCornerRadiusValue,
+            child->GetProperty(client::kWindowCornerRadiusKey));
   EXPECT_EQ(child->type(), client::WINDOW_TYPE_POPUP);
   EXPECT_EQ(ui::mojom::WindowType::BUBBLE,
             child->GetProperty(client::kWindowTypeKey));

ui/views/mus/mus_client.cc

@@ -195,6 +195,12 @@ MusClient::ConfigurePropertiesFromParams(
   properties[WindowManager::kRemoveStandardFrame_InitProperty] =
       mojo::ConvertTo<TransportType>(init_params.remove_standard_frame);
 
+  if (init_params.corner_radius) {
+    properties[WindowManager::kWindowCornerRadius_Property] =
+        mojo::ConvertTo<TransportType>(
+            static_cast<PrimitiveType>(*init_params.corner_radius));
+  }
+
   if (!Widget::RequiresNonClientView(init_params.type))
     return properties;
 

ui/views/test/views_test_base.h

@@ -41,7 +41,7 @@ class ViewsTestBase : public PlatformTest {
 
   // Creates a widget of |type| with any platform specific data for use in
   // cross-platform tests.
-  Widget::InitParams CreateParams(Widget::InitParams::Type type);
+  virtual Widget::InitParams CreateParams(Widget::InitParams::Type type);
 
   bool HasCompositingManager() const;
 

ui/views/test/widget_test.h

@@ -100,6 +100,11 @@ class WidgetTest : public ViewsTestBase {
   // Return true if |window| is transparent according to the native platform.
   static bool IsNativeWindowTransparent(gfx::NativeWindow window);
 
+  // Returns whether |widget| has a Window shadow managed in this process. That
+  // is, a shadow that is drawn outside of the Widget bounds, and managed by the
+  // WindowManager.
+  static bool WidgetHasInProcessShadow(Widget* widget);
+
   // Returns the set of all Widgets that currently have a NativeWindow.
   static Widget::Widgets GetAllWidgets();
 

ui/views/test/widget_test_aura.cc

@@ -13,6 +13,7 @@
 #include "ui/aura/window_tree_host.h"
 #include "ui/views/mus/mus_client.h"
 #include "ui/views/widget/widget.h"
+#include "ui/wm/core/shadow_controller.h"
 
 #if defined(USE_X11)
 #include "ui/gfx/x/x11.h"        // nogncheck
@@ -162,6 +163,21 @@ bool WidgetTest::IsNativeWindowTransparent(gfx::NativeWindow window) {
   return window->transparent();
 }
 
+// static
+bool WidgetTest::WidgetHasInProcessShadow(Widget* widget) {
+  aura::Window* window = widget->GetNativeWindow();
+  if (wm::ShadowController::GetShadowForWindow(window))
+    return true;
+
+  // If the Widget's native window is the content window for a
+  // DesktopWindowTreeHost, then giving the root window a shadow also has the
+  // effect of drawing a shadow around the window.
+  if (window->parent() == window->GetRootWindow())
+    return wm::ShadowController::GetShadowForWindow(window->GetRootWindow());
+
+  return false;
+}
+
 // static
 Widget::Widgets WidgetTest::GetAllWidgets() {
   Widget::Widgets all_widgets;

ui/views/test/widget_test_mac.mm

@@ -89,6 +89,11 @@ bool WidgetTest::IsNativeWindowTransparent(gfx::NativeWindow window) {
   return ![window isOpaque];
 }
 
+// static
+bool WidgetTest::WidgetHasInProcessShadow(Widget* widget) {
+  return false;
+}
+
 // static
 Widget::Widgets WidgetTest::GetAllWidgets() {
   Widget::Widgets all_widgets;

ui/views/widget/native_widget_aura.cc

@@ -157,15 +157,23 @@ void NativeWidgetAura::InitNativeWidget(const Widget::InitParams& params) {
   // MusClient has assertions that ui::mojom::WindowType matches
   // views::Widget::InitParams::Type.
   aura::SetWindowType(window_, static_cast<ui::mojom::WindowType>(params.type));
+  if (params.corner_radius) {
+    window_->SetProperty(aura::client::kWindowCornerRadiusKey,
+                         *params.corner_radius);
+  }
   window_->SetProperty(aura::client::kShowStateKey, params.show_state);
   if (params.type == Widget::InitParams::TYPE_BUBBLE)
     wm::SetHideOnDeactivate(window_, true);
   window_->SetTransparent(
       params.opacity == Widget::InitParams::TRANSLUCENT_WINDOW);
+
+  // Check for SHADOW_TYPE_NONE before aura::Window::Init() to ensure observers
+  // do not add useless shadow layers by deriving one from the window type.
+  SetShadowElevationFromInitParams(window_, params);
+
   window_->Init(params.layer_type);
   // Set name after layer init so it propagates to layer.
   window_->SetName(params.name);
-  SetShadowElevationFromInitParams(window_, params);
   if (params.type == Widget::InitParams::TYPE_CONTROL)
     window_->Show();
 

ui/views/widget/widget.h

@@ -240,6 +240,8 @@ class VIEWS_EXPORT Widget : public internal::NativeWidgetDelegate,
     // A hint about the size of the shadow if the type is SHADOW_TYPE_DROP. May
     // be ignored on some platforms. No value indicates no preference.
     base::Optional<int> shadow_elevation;
+    // The window corner radius. May be ignored on some platforms.
+    base::Optional<int> corner_radius;
     // Specifies that the system default caption and icon should not be
     // rendered, and that the client area should be equivalent to the window
     // area. Only used on some platforms (Windows and Linux).

ui/views/widget/widget_unittest.cc

@@ -47,6 +47,12 @@
 #include "base/mac/mac_util.h"
 #endif
 
+#if defined(OS_CHROMEOS)
+#include "ui/wm/core/base_focus_rules.h"
+#include "ui/wm/core/focus_controller.h"
+#include "ui/wm/core/shadow_controller.h"
+#endif
+
 namespace views {
 namespace test {
 
@@ -3731,6 +3737,142 @@ TEST_F(WidgetTest, MouseWheelEvent) {
   EXPECT_EQ(1, event_count_view->GetEventCount(ui::ET_MOUSEWHEEL));
 }
 
+class WidgetShadowTest : public WidgetTest {
+ public:
+  WidgetShadowTest() { InitControllers(); }
+
+  // WidgetTest:
+  Widget::InitParams CreateParams(Widget::InitParams::Type type) override {
+    Widget::InitParams params =
+        WidgetTest::CreateParams(override_type_.value_or(type));
+    params.shadow_type = Widget::InitParams::SHADOW_TYPE_DROP;
+    params.shadow_elevation = 10;
+    params.name = name_;
+    params.child = force_child_;
+    return params;
+  }
+
+ protected:
+  base::Optional<Widget::InitParams::Type> override_type_;
+  std::string name_;
+  bool force_child_ = false;
+
+ private:
+#if !defined(OS_CHROMEOS)
+  void InitControllers() {}
+#else
+  class TestFocusRules : public wm::BaseFocusRules {
+   public:
+    TestFocusRules() = default;
+
+    bool SupportsChildActivation(aura::Window* window) const override {
+      return true;
+    }
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(TestFocusRules);
+  };
+
+  void InitControllers() {
+    // Add bits usually managed by the ash::Shell. Under Mus,
+    // DesktopNativeWidgetAura provides these in-process instead.
+    if (IsMus())
+      return;
+
+    focus_controller_ =
+        std::make_unique<wm::FocusController>(new TestFocusRules);
+    shadow_controller_ =
+        std::make_unique<wm::ShadowController>(focus_controller_.get());
+  }
+
+  std::unique_ptr<wm::FocusController> focus_controller_;
+  std::unique_ptr<wm::ShadowController> shadow_controller_;
+#endif  // OS_CHROMEOS
+
+  DISALLOW_COPY_AND_ASSIGN(WidgetShadowTest);
+};
+
+// Disabled on Mac: All drop shadows are managed out of process for now.
+#if defined(OS_MACOSX) && !defined(USE_AURA)
+#define MAYBE_ShadowsInRootWindow DISABLED_ShadowsInRootWindow
+#else
+#define MAYBE_ShadowsInRootWindow ShadowsInRootWindow
+#endif
+
+// Test that shadows are not added to root windows when created or upon
+// activation. Test that shadows are added to non-root windows even if not
+// activated.
+TEST_F(WidgetShadowTest, MAYBE_ShadowsInRootWindow) {
+  // A desktop window clips to its bounds, so it shouldn't have a shadow.
+  bool top_level_window_should_have_shadow = false;
+
+#if defined(OS_CHROMEOS)
+  // In Mus, the shadow should be in the WindowServer process only. In non-mus
+  // CreateNativeDesktopWidget() creates a non-root window, so it should have
+  // a shadow.
+  top_level_window_should_have_shadow = !IsMus();
+#endif
+
+  // To start, just create a Widget. This constructs the first ShadowController
+  // which will start observing the environment for additional aura::Window
+  // initialization. The very first ShadowController in DesktopNativeWidgetAura
+  // is created after the call to aura::Window::Init(), so the ShadowController
+  // Impl class won't ever see this first Window being initialized.
+  name_ = "other_top_level";
+  Widget* other_top_level = CreateNativeDesktopWidget();
+
+  name_ = "top_level";
+  Widget* top_level = CreateNativeDesktopWidget();
+  top_level->SetBounds(gfx::Rect(100, 100, 320, 200));
+
+  EXPECT_FALSE(WidgetHasInProcessShadow(top_level));
+  EXPECT_FALSE(top_level->IsVisible());
+  top_level->ShowInactive();
+  EXPECT_EQ(top_level_window_should_have_shadow,
+            WidgetHasInProcessShadow(top_level));
+  top_level->Show();
+  EXPECT_EQ(top_level_window_should_have_shadow,
+            WidgetHasInProcessShadow(top_level));
+
+  name_ = "control";
+  Widget* control = CreateChildNativeWidgetWithParent(top_level);
+  control->SetBounds(gfx::Rect(20, 20, 160, 100));
+
+  // Widgets of TYPE_CONTROL become visible during Init, so start with a shadow.
+  EXPECT_TRUE(WidgetHasInProcessShadow(control));
+  control->ShowInactive();
+  EXPECT_TRUE(WidgetHasInProcessShadow(control));
+  control->Show();
+  EXPECT_TRUE(WidgetHasInProcessShadow(control));
+
+  name_ = "child";
+  override_type_ = Widget::InitParams::TYPE_POPUP;
+  force_child_ = true;
+  Widget* child = CreateChildNativeWidgetWithParent(top_level);
+  child->SetBounds(gfx::Rect(20, 20, 160, 100));
+
+  // Now false: the Widget hasn't been shown yet.
+  EXPECT_FALSE(WidgetHasInProcessShadow(child));
+  child->ShowInactive();
+  EXPECT_TRUE(WidgetHasInProcessShadow(child));
+  child->Show();
+  EXPECT_TRUE(WidgetHasInProcessShadow(child));
+
+  other_top_level->Show();
+
+  // Re-activate the top level window. This handles a hypothetical case where
+  // a shadow is added via the ActivationChangeObserver rather than by the
+  // aura::WindowObserver. Activation changes only modify an existing shadow
+  // (if there is one), but should never install a Shadow, even if the Window
+  // properties otherwise say it should have one.
+  top_level->Show();
+  EXPECT_EQ(top_level_window_should_have_shadow,
+            WidgetHasInProcessShadow(top_level));
+
+  top_level->Close();
+  other_top_level->Close();
+}
+
 #if defined(OS_WIN)
 
 namespace {

ui/wm/core/shadow_controller.cc

@@ -105,6 +105,7 @@ class ShadowController::Impl :
   void OnWindowPropertyChanged(aura::Window* window,
                                const void* key,
                                intptr_t old) override;
+  void OnWindowVisibilityChanging(aura::Window* window, bool visible) override;
   void OnWindowBoundsChanged(aura::Window* window,
                              const gfx::Rect& old_bounds,
                              const gfx::Rect& new_bounds,
@@ -157,24 +158,45 @@ ShadowController::Impl* ShadowController::Impl::GetInstance() {
 }
 
 void ShadowController::Impl::OnWindowInitialized(aura::Window* window) {
+  // During initialization, the window can't reliably tell whether it will be a
+  // root window. That must be checked in the first visibility change
+  DCHECK(!window->parent());
+  DCHECK(!window->TargetVisibility());
   observer_manager_.Add(window);
-  HandlePossibleShadowVisibilityChange(window);
 }
 
 void ShadowController::Impl::OnWindowPropertyChanged(aura::Window* window,
                                                      const void* key,
                                                      intptr_t old) {
-  if (key == kShadowElevationKey) {
-    if (window->GetProperty(kShadowElevationKey) == old)
-      return;
-  } else if (key == aura::client::kShowStateKey) {
-    if (window->GetProperty(aura::client::kShowStateKey) ==
-        static_cast<ui::WindowShowState>(old)) {
-      return;
-    }
-  } else {
+  bool shadow_will_change = false;
+  if (key == kShadowElevationKey)
+    shadow_will_change = window->GetProperty(kShadowElevationKey) != old;
+
+  if (key == aura::client::kShowStateKey) {
+    shadow_will_change = window->GetProperty(aura::client::kShowStateKey) !=
+                         static_cast<ui::WindowShowState>(old);
+  }
+
+  // Check the target visibility. IsVisible() may return false if a parent layer
+  // is hidden, but |this| only observes calls to Show()/Hide() on |window|.
+  if (shadow_will_change && window->TargetVisibility())
+    HandlePossibleShadowVisibilityChange(window);
+}
+
+void ShadowController::Impl::OnWindowVisibilityChanging(aura::Window* window,
+                                                        bool visible) {
+  // At the time of the first visibility change, |window| will give a correct
+  // answer for whether or not it is a root window. If it is, don't bother
+  // observing: a shadow should never be added. Root windows can only have
+  // shadows in the WindowServer (where a corresponding aura::Window may no
+  // longer be a root window). Without this check, a second shadow is added,
+  // which clips to the root window bounds; filling any rounded corners the
+  // window may have.
+  if (window->IsRootWindow()) {
+    observer_manager_.Remove(window);
     return;
   }
+
   HandlePossibleShadowVisibilityChange(window);
 }
 
@@ -220,7 +242,7 @@ bool ShadowController::Impl::ShouldShowShadowForWindow(
     return false;
   }
 
-  return static_cast<int>(GetShadowElevationConvertDefault(window)) > 0;
+  return GetShadowElevationConvertDefault(window) > 0;
 }
 
 void ShadowController::Impl::HandlePossibleShadowVisibilityChange(
@@ -236,12 +258,19 @@ void ShadowController::Impl::HandlePossibleShadowVisibilityChange(
 }
 
 void ShadowController::Impl::CreateShadowForWindow(aura::Window* window) {
+  DCHECK(!window->IsRootWindow());
   Shadow* shadow = new Shadow();
   window->SetProperty(kShadowLayerKey, shadow);
+
+  int corner_radius = window->GetProperty(aura::client::kWindowCornerRadiusKey);
+  if (corner_radius >= 0)
+    shadow->SetRoundedCornerRadius(corner_radius);
+
   shadow->Init(GetShadowElevationForActiveState(window));
   shadow->SetContentBounds(gfx::Rect(window->bounds().size()));
   shadow->layer()->SetVisible(ShouldShowShadowForWindow(window));
   window->layer()->Add(shadow->layer());
+  window->layer()->StackAtBottom(shadow->layer());
 }
 
 ShadowController::Impl::Impl()

ui/wm/core/shadow_controller_unittest.cc

@@ -61,15 +61,16 @@ TEST_F(ShadowControllerTest, Shadow) {
   window->Init(ui::LAYER_TEXTURED);
   ParentWindow(window.get());
 
-  // We should create the shadow before the window is visible (the shadow's
-  // layer won't get drawn yet since it's a child of the window's layer).
+  // The shadow is not created until the Window is shown (some Windows should
+  // never get shadows, which is checked when the window first becomes visible).
+  EXPECT_FALSE(ShadowController::GetShadowForWindow(window.get()));
+  window->Show();
+
   const Shadow* shadow = ShadowController::GetShadowForWindow(window.get());
   ASSERT_TRUE(shadow != NULL);
   EXPECT_TRUE(shadow->layer()->visible());
 
   // The shadow should remain visible after window visibility changes.
-  window->Show();
-  EXPECT_TRUE(shadow->layer()->visible());
   window->Hide();
   EXPECT_TRUE(shadow->layer()->visible());