Added restore functionality for maximize full/left/right

ash/wm/workspace/frame_maximize_button.cc

Index: ash/wm/workspace/frame_maximize_button.cc
diff --git a/ash/wm/workspace/frame_maximize_button.cc b/ash/wm/workspace/frame_maximize_button.cc
index 8f8f7e8ff2ebd70d2bc6bc90a2a7479edd74ee43..383724eee1e60688bb8b266b60d985d0e832e2bd 100644
--- a/ash/wm/workspace/frame_maximize_button.cc
+++ b/ash/wm/workspace/frame_maximize_button.cc
@@ -35,6 +35,8 @@ const int kUpdateDelayMS = 400;
 // The delay of the bubble appearance.
 const int kBubbleAppearanceDelayMS = 500;
 
+// The minimum sanp size in percent of the screen width.
+const int kMinSnapSizePercent = 50;
 }
 
 // EscapeEventFilter is installed on the RootWindow to track when the escape key
@@ -212,7 +214,7 @@ void FrameMaximizeButton::OnMouseEntered(const ui::MouseEvent& event) {
     }
     maximizer_.reset(new MaximizeBubbleController(
         this,
-        frame_->GetWidget()->IsMaximized(),
+        GetMaximizeBubbleFrameState(),
         bubble_appearance_delay_ms_));
   }
 }
@@ -304,7 +306,7 @@ void FrameMaximizeButton::ProcessStartEvent(const ui::LocatedEvent& event) {
   if (!maximizer_.get()) {
     maximizer_.reset(new MaximizeBubbleController(
         this,
-        frame_->GetWidget()->IsMaximized(),
+        GetMaximizeBubbleFrameState(),
         bubble_appearance_delay_ms_));
   } else {
     // If the menu did not show up yet, we delay it even a bit more.
@@ -432,17 +434,18 @@ void FrameMaximizeButton::UpdateSnap(const gfx::Point& location) {
 
 SnapType FrameMaximizeButton::SnapTypeForLocation(
     const gfx::Point& location) const {
+  MaximizeBubbleFrameState maximize_type = GetMaximizeBubbleFrameState();
   int delta_x = location.x() - press_location_.x();
   int delta_y = location.y() - press_location_.y();
   if (!views::View::ExceededDragThreshold(delta_x, delta_y))
-    return !frame_->GetWidget()->IsMaximized() ? SNAP_MAXIMIZE : SNAP_RESTORE;
+    return maximize_type != FRAME_STATE_FULL ? SNAP_MAXIMIZE : SNAP_RESTORE;
   else if (delta_x < 0 && delta_y > delta_x && delta_y < -delta_x)
-    return SNAP_LEFT;
+    return maximize_type == FRAME_STATE_SNAP_LEFT ? SNAP_RESTORE : SNAP_LEFT;
   else if (delta_x > 0 && delta_y > -delta_x && delta_y < delta_x)
-    return SNAP_RIGHT;
+    return maximize_type == FRAME_STATE_SNAP_RIGHT ? SNAP_RESTORE : SNAP_RIGHT;
   else if (delta_y > 0)
     return SNAP_MINIMIZE;
-  return !frame_->GetWidget()->IsMaximized() ? SNAP_MAXIMIZE : SNAP_RESTORE;
+  return maximize_type != FRAME_STATE_FULL ? SNAP_MAXIMIZE : SNAP_RESTORE;
 }
 
 gfx::Rect FrameMaximizeButton::ScreenBoundsForType(
@@ -461,7 +464,8 @@ gfx::Rect FrameMaximizeButton::ScreenBoundsForType(
           ScreenAsh::GetMaximizedWindowBoundsInParent(window));
     case SNAP_MINIMIZE: {
       Launcher* launcher = Shell::GetInstance()->launcher();
-      gfx::Rect item_rect(launcher->GetScreenBoundsOfItemIconForWindow(window));
+      gfx::Rect item_rect(launcher->GetScreenBoundsOfItemIconForWindow(
+          window));
       if (!item_rect.IsEmpty()) {
         // PhantomWindowController insets slightly, outset it so the phantom
         // doesn't appear inset.
@@ -489,31 +493,64 @@ gfx::Point FrameMaximizeButton::LocationForSnapSizer(
 }
 
 void FrameMaximizeButton::Snap(const SnapSizer& snap_sizer) {
+  views::Widget* widget = frame_->GetWidget();
   switch (snap_type_) {
     case SNAP_LEFT:
-    case SNAP_RIGHT:
-      if (frame_->GetWidget()->IsMaximized()) {
-        ash::SetRestoreBoundsInScreen(frame_->GetWidget()->GetNativeWindow(),
+    case SNAP_RIGHT: {
+      // Get the window coordinates on the screen for restore purposes.
+      gfx::Rect restore = widget->GetNativeWindow()->bounds();
+      if (widget->IsMaximized()) {
+        // If it was maximized we need to recover the old restore set.
+        restore = *ash::GetRestoreBoundsInScreen(widget->GetNativeWindow());

[sky, 2012/08/29 17:29:20]

This will crash if GetRestoreBoundsInScreen returns NULL.

[Mr4D (OOO till 08-26), 2012/08/29 18:57:34]

I thought that in maximized mode the restore rectangle HAS TO BE SET... No?
Added a DCHECK.
However I am a bit irritated now. Usually you want it to crash if something goes
wrong - and / or you do not want to check for obvious failures.

[sky, 2012/08/29 21:08:50]

A DCHECK is the right thing. It says you expect this to be non-null.

+        // Set the restore size we want to restore to.
+        ash::SetRestoreBoundsInScreen(widget->GetNativeWindow(),

[sky, 2012/08/29 17:29:20]

Why do you need to set the restore bounds here as well as 514?

[Mr4D (OOO till 08-26), 2012/08/29 18:57:34]

This is from the original code: We need to exit maximize and we have to 'revert'
to the left/right snapped locations.

The second one is then used for setting the real restore value so that we know
where to restore to when restore from snapped mode.

                                       ScreenBoundsForType(snap_type_,
                                                           snap_sizer));
-        frame_->GetWidget()->Restore();
+        widget->Restore();

[sky, 2012/08/29 17:29:20]

Gah. Looking at this whole branch it seems wrong. I think we should set the
frame to normal, then set the bounds.

[Mr4D (OOO till 08-26), 2012/08/29 18:57:34]

This code predates my doing and was only taken over.

But... that is exactly what we do. Note that we do not want to set two different
sizes (normal, bounds) since that could cause a 'flash' or something when an
animation is used not/used.

[sky, 2012/08/29 21:08:50]

Good point.

       } else {
-        frame_->GetWidget()->SetBounds(ScreenBoundsForType(snap_type_,
-                                                           snap_sizer));
+        widget->SetBounds(ScreenBoundsForType(snap_type_, snap_sizer));
+      }
+      // Remember the widow's bounds for restoration.
+      ash::SetRestoreBoundsInScreen(widget->GetNativeWindow(), restore);
       }
       break;
     case SNAP_MAXIMIZE:
-      frame_->GetWidget()->Maximize();
+      widget->Maximize();
       break;
     case SNAP_MINIMIZE:
-      frame_->GetWidget()->Minimize();
+      widget->Minimize();
       break;
     case SNAP_RESTORE:
-      frame_->GetWidget()->Restore();
+      widget->Restore();
       break;
     case SNAP_NONE:
       NOTREACHED();
   }
 }
 
+MaximizeBubbleFrameState FrameMaximizeButton::GetMaximizeBubbleFrameState(
+    ) const {

[sky, 2012/08/29 17:29:20]

Wrap FMB::GetMa.. to the next line, eg:
MaximizeBubbleFrameState
FrameMaximizeButton::GetMaximizeBubbleFrameState() const {
}

[Mr4D (OOO till 08-26), 2012/08/29 18:57:34]

Done.

+  // When there are no restore bounds, we are in normal mode.
+  if (!ash::GetRestoreBoundsInScreen(
+           frame_->GetWidget()->GetNativeWindow()))
+    return FRAME_STATE_NONE;
+  // The normal maximized test can be used.
+  if (frame_->GetWidget()->IsMaximized())
+    return FRAME_STATE_FULL;
+  // For Left/right maximize we need to check the dimensions.
+  gfx::Rect bounds = frame_->GetWidget()->GetWindowBoundsInScreen();
+  gfx::Rect screen = gfx::Screen::GetDisplayMatching(bounds).work_area();
+  if (bounds.width() < (screen.width() * kMinSnapSizePercent) / 100)
+    return FRAME_STATE_NONE;
+  // We have to be in a maximize mode at this point.
+  DCHECK(bounds.y() == screen.y());
+  DCHECK(bounds.height() >= screen.height());
+  if (bounds.x() == screen.x())
+    return FRAME_STATE_SNAP_LEFT;
+  if (bounds.right() == screen.right())
+    return FRAME_STATE_SNAP_RIGHT;
+  NOTREACHED();

[sky, 2012/08/29 17:29:20]

Won't you hit this if you resize the window horizontally more than 50% of the
screen, then double click the vertical resize edge to fill screen? 

[Mr4D (OOO till 08-26), 2012/08/29 18:57:34]

This state is still ominous to me and contradicts everything. I looked at the
code and see that it also populates the restore rect. I therefore removed that
NOTREACHED and added a comment that this case can happen - and why.

Note: This also required some other changes.

+  return FRAME_STATE_NONE;
+}
+
 }  // namespace ash