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() {
+  // Before the window gets destroyed, the maximizer dialog needs to be shut
+  // down since it would otherwise call into a deleted object.
+  maximizer_.reset();
   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);
+  Cancel(true);
+  // Tell our menu to close.
+  maximizer_.reset();
   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();
+  // Note: |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 152 matching lines @@
   phantom_window_.reset();
   Snap();
   return true;
 }
 
 void FrameMaximizeButton::Cancel(bool keep_menu_open) {
   if (!keep_menu_open) {
     maximizer_.reset();
     UninstallEventFilter();
     is_snap_enabled_ = false;
+    snap_sizer_.reset();

[sky, 2012/08/08 04:39:17]

Persisting the snap_sizer_ makes me nervous. In particular I'm worried we might
not reset snap_sizer_ at the appropriate time resulting it in containing the
wrong value. Do we need to reset snap_sizer_ in DestroyMaximizeMenu now?

[Mr4D (OOO till 08-26), 2012/08/08 14:26:46]

When doing the change I thought the same thing - but I thought this is okay
because:
a. It is a scoped_ptr -> So it destroys itself guaranteed.
b. ProcessEndEvent doesn't destroy the snap_sizer_ either.

   }
   phantom_window_.reset();
-  snap_sizer_.reset();
   snap_type_ = SNAP_NONE;
   update_timer_.Stop();
   SchedulePaint();
 }
 
 void FrameMaximizeButton::InstallEventFilter() {
   if (escape_event_filter_.get())
     return;
 
   escape_event_filter_.reset(new EscapeEventFilter(this));
@@ skipping 127 matching lines @@
       break;
     case SNAP_RESTORE:
       frame_->GetWidget()->Restore();
       break;
     case SNAP_NONE:
       NOTREACHED();
   }
 }
 
 }  // namespace ash