Fixing many problems around the maximize bubble

ash/wm/workspace/frame_maximize_button.cc

 // Copyright (c) 2012 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 "ash/wm/workspace/frame_maximize_button.h"
 
 #include "ash/launcher/launcher.h"
 #include "ash/screen_ash.h"
 #include "ash/shell.h"
 #include "ash/wm/property_util.h"
@@ skipping 95 matching lines @@
       frame_(frame),
       is_snap_enabled_(false),
       exceeded_drag_threshold_(false),
       window_(NULL),
       snap_type_(SNAP_NONE) {
   // TODO(sky): nuke this. It's temporary while we don't have good images.
   SetImageAlignment(ALIGN_LEFT, ALIGN_BOTTOM);
 }
 
 FrameMaximizeButton::~FrameMaximizeButton() {
+  maximizer_.reset();

[sky, 2012/08/07 23:11:18]

Add a comment as to why this needs to be first.

[Mr4D (OOO till 08-26), 2012/08/08 01:02:28]

Done.

   if (window_)
     OnWindowDestroying(window_);
 }
 
 void FrameMaximizeButton::SnapButtonHovered(SnapType type) {
   // Make sure to only show hover operations when no button is pressed and
   // a similar snap operation in progress does not get re-applied.
   if (is_snap_enabled_ || (type == snap_type_ && snap_sizer_.get()))
     return;
   // Prime the mouse location with the center of the (local) button.
@@ skipping 18 matching lines @@
       return;
     default:
       // We should not come here.
       NOTREACHED();
   }
   UpdateSnap(location);
 }
 
 void FrameMaximizeButton::ExecuteSnapAndCloseMenu(SnapType snap_type) {
   DCHECK_NE(snap_type_, SNAP_NONE);
-  snap_type_ = snap_type;
-  Snap();
+  // SNAP_MINIMIZE might destroy |this| for windows like the file manager.

[sky, 2012/08/07 23:11:18]

This is too fragile. Can't the code be like ProcessEndEvent? In fact can't we
make the two the same?

[Mr4D (OOO till 08-26), 2012/08/08 01:02:28]

Okay. Changed it. However - I did not move the cases together because the
ProcessEndEvent is clearly only meant for touch/drag operations and adding a
flag to avoid most actions there is clearly not desirable.

+  // To avoid crashes we have therefore to execute that command at the end.
+  // All other commands need the snap_sizer to know the target size, so they
+  // have to be executed before we start destroying the rest.
+  if (snap_type != SNAP_MINIMIZE) {
+    snap_type_ = snap_type;
+    Snap();
+  }
   // Remove any pending snap previews.
   SnapButtonHovered(SNAP_NONE);
   // At this point the operation has been performed and the menu should be
   // closed - if not, it'll get now closed.
   maximizer_.reset();
+  if (snap_type == SNAP_MINIMIZE) {
+    snap_type_ = snap_type;
+    Snap();
+    // |this| might now be destroyed.
+  }
 }
 
 void FrameMaximizeButton::DestroyMaximizeMenu() {
   maximizer_.reset();
 }
 
 void FrameMaximizeButton::OnWindowBoundsChanged(
     aura::Window* window,
     const gfx::Rect& old_bounds,
     const gfx::Rect& new_bounds) {
@@ skipping 302 matching lines @@
       break;
     case SNAP_RESTORE:
       frame_->GetWidget()->Restore();
       break;
     case SNAP_NONE:
       NOTREACHED();
   }
 }
 
 }  // namespace ash