Fix phantom windows for small windows when using --ash-enable-alternate-caption-button

ash/wm/workspace/phantom_window_controller.cc

Index: ash/wm/workspace/phantom_window_controller.cc
diff --git a/ash/wm/workspace/phantom_window_controller.cc b/ash/wm/workspace/phantom_window_controller.cc
index 67706f228c77ca8e290b8e58069df881b92332bc..8e55c5cd53afd769c980839a6605ab41c795dc91 100644
--- a/ash/wm/workspace/phantom_window_controller.cc
+++ b/ash/wm/workspace/phantom_window_controller.cc
@@ -4,6 +4,8 @@
 
 #include "ash/wm/workspace/phantom_window_controller.h"
 
+#include <math.h>
+
 #include "ash/ash_switches.h"
 #include "ash/shell.h"
 #include "ash/shell_window_ids.h"
@@ -32,22 +34,32 @@ const int kAnimationDurationMs = 200;
 // The size of the phantom window at the beginning of the show animation in
 // relation to the size of the phantom window at the end of the animation when
 // using the alternate caption button style.
-const float kAlternateCaptionButtonStyleAnimationInitialBoundsRatio = 0.85f;
+const float kAlternateStyleStartBoundsRatio = 0.85f;
 
 // The amount of pixels that the phantom window's shadow should extend past
 // the bounds passed into Show(). There is no shadow when not using the
 // alternate caption button style.
-const int kShadowThickness = 60;
+const int kAlternateStyleShadowThickness = 15;
+
+// The minimum size of a phantom window including the shadow when using the
+// alternate caption button style. The minimum size is derived from the size of
+// the IDR_AURA_PHANTOM_WINDOW image assets.
+const int kAlternateStyleMinSizeWithShadow = 100;
 
 // Converts the bounds of a phantom window without a shadow to those of a
 // phantom window with a shadow.
-gfx::Rect GetBoundsWithShadow(const gfx::Rect& bounds) {
-  gfx::Rect bounds_with_shadow(bounds);
-  // Phantom windows have a shadow solely when using the alternate caption
-  // button style.
-  if (switches::UseAlternateFrameCaptionButtonStyle())
-    bounds_with_shadow.Inset(-kShadowThickness, -kShadowThickness);
-  return bounds_with_shadow;
+gfx::Rect GetAdjustedBounds(const gfx::Rect& bounds) {
+  gfx::Rect adjusted_bounds(bounds);
+  if (switches::UseAlternateFrameCaptionButtonStyle()) {
+    int x_inset = bounds.width() < kAlternateStyleMinSizeWithShadow ?
+        ceil((kAlternateStyleMinSizeWithShadow - bounds.width()) / 2.0f) :
+        kAlternateStyleShadowThickness;
+    int y_inset = bounds.height() < kAlternateStyleMinSizeWithShadow ?
+        ceil((kAlternateStyleMinSizeWithShadow - bounds.height()) / 2.0f) :
+        kAlternateStyleShadowThickness;
+    adjusted_bounds.Inset(-x_inset, -y_inset);
+  }
+  return adjusted_bounds;
 }
 
 // Starts an animation of |widget| to |new_bounds_in_screen|. No-op if |widget|
@@ -138,17 +150,23 @@ PhantomWindowController::~PhantomWindowController() {
 }
 
 void PhantomWindowController::Show(const gfx::Rect& bounds_in_screen) {

[Mr4D (OOO till 08-26), 2014/01/28 17:45:25]

Couldn't you get the GetAdjustedBounds only once here?

Or even better - GetAdjustedBounds only changes things when 
UseAlternateFrameCaptionButtonStyle is set. You do the same check in line 157
and you could return in line 174. Wouldn't it be much clearer to call from Show
into either ShowAlternate and / or ShowLegacy? That would also make this long
function better readable and avoids calling that function in the legacy case
altogether.

-  if (GetBoundsWithShadow(bounds_in_screen) == target_bounds_in_screen_)
+  if (GetAdjustedBounds(bounds_in_screen) == target_bounds_in_screen_)
     return;
-  target_bounds_in_screen_ = GetBoundsWithShadow(bounds_in_screen);
+  target_bounds_in_screen_ = GetAdjustedBounds(bounds_in_screen);
 
   if (switches::UseAlternateFrameCaptionButtonStyle()) {
     gfx::Rect start_bounds_in_screen = target_bounds_in_screen_;
-    float inset_ratio =
-        (1.0f - kAlternateCaptionButtonStyleAnimationInitialBoundsRatio) / 2;
+    int start_width = std::max(
+        kAlternateStyleMinSizeWithShadow,
+        static_cast<int>(
+            start_bounds_in_screen.width() * kAlternateStyleStartBoundsRatio));
+    int start_height = std::max(
+        kAlternateStyleMinSizeWithShadow,
+        static_cast<int>(
+            start_bounds_in_screen.height() * kAlternateStyleStartBoundsRatio));
     start_bounds_in_screen.Inset(
-        static_cast<int>(start_bounds_in_screen.width() * inset_ratio),
-        static_cast<int>(start_bounds_in_screen.height() * inset_ratio));
+        (start_bounds_in_screen.width() - start_width) / 2,
+        (start_bounds_in_screen.height() - start_height) / 2);
     phantom_widget_in_target_root_ = CreatePhantomWidget(
         wm::GetRootWindowMatching(target_bounds_in_screen_),
         start_bounds_in_screen);
@@ -158,8 +176,7 @@ void PhantomWindowController::Show(const gfx::Rect& bounds_in_screen) {
   } else {
     gfx::Rect start_bounds_in_screen;
     if (!phantom_widget_in_target_root_) {
-      start_bounds_in_screen =
-          GetBoundsWithShadow(window_->GetBoundsInScreen());
+      start_bounds_in_screen = GetAdjustedBounds(window_->GetBoundsInScreen());
     } else {
       start_bounds_in_screen =
           phantom_widget_in_target_root_->GetWindowBoundsInScreen();