Fixes possible crash in confirm dialog. If you press and hold space,

then press escape (with space still down) and release we end up
notifying the delegate twice. The first the time the delegate is
notified causes some action so that the second time through we end up
crashing (delegate deleted). The fix is to only notify delegate once.

BUG=71940
TEST=see bug

Review URL: http://codereview.chromium.org/6429001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@74131 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 27 insertions, 21 deletions

diff --git a/views/window/client_view.h b/views/window/client_view.h
index fc3d531..08ddcd4 100644
--- a/views/window/client_view.h
+++ b/views/window/client_view.h
@@ -37,7 +37,7 @@ class ClientView : public View {
   // ClientView subclasses can override this default behavior to allow the
   // close to be blocked until the user corrects mistakes, accepts a warning
   // dialog, etc.
-  virtual bool CanClose() const { return true; }
+  virtual bool CanClose() { return true; }
 
   // Notification that the window is closing.  The default implementation
   // forwards the notification to the delegate.
diff --git a/views/window/dialog_client_view.cc b/views/window/dialog_client_view.cc
index 172cceb..86b3d01 100644
--- a/views/window/dialog_client_view.cc
+++ b/views/window/dialog_client_view.cc
@@ -115,7 +115,7 @@ DialogClientView::DialogClientView(Window* owner, View* contents_view)
       default_button_(NULL),
       extra_view_(NULL),
       size_extra_view_height_to_buttons_(false),
-      accepted_(false),
+      notified_delegate_(false),
       listening_to_focus_(false),
       saved_focus_manager_(NULL),
       bottom_view_(NULL) {
@@ -225,13 +225,13 @@ void DialogClientView::UpdateDialogButtons() {
 }
 
 void DialogClientView::AcceptWindow() {
-  if (accepted_) {
-    // We should only get into AcceptWindow once.
-    NOTREACHED();
+  if (notified_delegate_) {
+    // Only notify the delegate once. See comment in header above
+    // notified_delegate_ for details.
     return;
   }
   if (GetDialogDelegate()->Accept(false)) {
-    accepted_ = true;
+    notified_delegate_ = true;
     Close();
   }
 }
@@ -267,16 +267,19 @@ void DialogClientView::NativeViewHierarchyChanged(bool attached,
 ///////////////////////////////////////////////////////////////////////////////
 // DialogClientView, ClientView overrides:
 
-bool DialogClientView::CanClose() const {
-  if (!accepted_) {
-    DialogDelegate* dd = GetDialogDelegate();
-    int buttons = dd->GetDialogButtons();
-    if (buttons & MessageBoxFlags::DIALOGBUTTON_CANCEL)
-      return dd->Cancel();
-    if (buttons & MessageBoxFlags::DIALOGBUTTON_OK)
-      return dd->Accept(true);
-  }
-  return true;
+bool DialogClientView::CanClose() {
+  if (notified_delegate_)
+    return true;
+
+  DialogDelegate* dd = GetDialogDelegate();
+  int buttons = dd->GetDialogButtons();
+  bool close = true;
+  if (buttons & MessageBoxFlags::DIALOGBUTTON_CANCEL)
+    close = dd->Cancel();
+  else if (buttons & MessageBoxFlags::DIALOGBUTTON_OK)
+    close = dd->Accept(true);
+  notified_delegate_ = close;
+  return close;
 }
 
 void DialogClientView::WindowClosing() {
diff --git a/views/window/dialog_client_view.h b/views/window/dialog_client_view.h
index 1ac2fee..0f83186 100644
--- a/views/window/dialog_client_view.h
+++ b/views/window/dialog_client_view.h
@@ -65,7 +65,7 @@ class DialogClientView : public ClientView,
                                           RootView* root_view);
 
   // Overridden from ClientView:
-  virtual bool CanClose() const;
+  virtual bool CanClose();
   virtual void WindowClosing();
   virtual int NonClientHitTest(const gfx::Point& point);
   virtual DialogClientView* AsDialogClientView() { return this; }
@@ -134,8 +134,11 @@ class DialogClientView : public ClientView,
   // The layout rect of the size box, when visible.
   gfx::Rect size_box_bounds_;
 
-  // True if the window was Accepted by the user using the OK button.
-  bool accepted_;
+  // True if we've notified the delegate the window is closing and the delegate
+  // allosed the close. In some situations it's possible to get two closes (see
+  // http://crbug.com/71940). This is used to avoid notifying the delegate
+  // twice, which can have bad consequences.
+  bool notified_delegate_;
 
   // true if focus listener is added.
   bool listening_to_focus_;
diff --git a/views/window/non_client_view.cc b/views/window/non_client_view.cc
index 51936aa..f759d31 100644
--- a/views/window/non_client_view.cc
+++ b/views/window/non_client_view.cc
@@ -48,7 +48,7 @@ void NonClientView::SetFrameView(NonClientFrameView* frame_view) {
     AddChildView(kFrameViewIndex, frame_view_.get());
 }
 
-bool NonClientView::CanClose() const {
+bool NonClientView::CanClose() {
   return client_view_->CanClose();
 }
 
diff --git a/views/window/non_client_view.h b/views/window/non_client_view.h
index 2cd78f6..3a4ea28 100644
--- a/views/window/non_client_view.h
+++ b/views/window/non_client_view.h
@@ -151,7 +151,7 @@ class NonClientView : public View {
 
   // Returns true if the ClientView determines that the containing window can be
   // closed, false otherwise.
-  bool CanClose() const;
+  bool CanClose();
 
   // Called by the containing Window when it is closed.
   void WindowClosing();