[Mac] Delay TabContents::Close() when event-tracking.

The close is delayed until the main event loop restarts.

Renderers can cause UI state changes which can badly break
event-tracking loops.  The basic pattern is "Run JavaScript to close
window after a timeout, and start event-tracking loop and keep it
going across the timeout."  Things crash when UI elements attempt to
refer to freed objects.  Examples:

 - Last tab in a window closes while dragging the window.
 - Last tab in a window closes while bookmark bar context menu visible.
 - Last tab in a window closes while download shelf context menu visible.
 - Tab closes while dragging a link over the tab.
 - Tab closes while dragging a link from the tab.
 - Tab closes while back/forward context menu is open.
 - Tab closes while click-hold in the tab's close button.
 - Tab closes while closing info bar.
 - Tab closes while tab context menu is visible.
 - Probably more I'm not aware of.

Supersedes (and reverts) previous fix for issues 25462 and 25465.

BUG=25462, 25463, 25465, 26135, 26136, 26137, 25467
TEST=See bugs for repro cases.

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

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

4 files changed, 80 insertions, 0 deletions

diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index e72df68..35e9b80 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -2179,6 +2179,21 @@ void TabContents::UpdateInspectorSettings(const std::string& raw_settings) {
 }
 
 void TabContents::Close(RenderViewHost* rvh) {
+  // The UI may be in an event-tracking loop, such as between the
+  // mouse-down and mouse-up in text selection or a button click.
+  // Defer the close until after tracking is complete, so that we
+  // don't free objects out from under the UI.
+  // TODO(shess): This could probably be integrated with the
+  // IsDoingDrag() test below.  Punting for now because I need more
+  // research to understand how this impacts platforms other than Mac.
+  // TODO(shess): This could get more fine-grained.  For instance,
+  // closing a tab in another window while selecting text in the
+  // current window's Omnibox should be just fine.
+  if (view()->IsEventTracking()) {
+    view()->CloseTabAfterEventTracking();
+    return;
+  }
+
   // If we close the tab while we're in the middle of a drag, we'll crash.
   // Instead, cancel the drag and close it as soon as the drag ends.
   if (view()->IsDoingDrag()) {
diff --git a/chrome/browser/tab_contents/tab_contents_view.h b/chrome/browser/tab_contents/tab_contents_view.h
index db5fff8..79bee6d 100644
--- a/chrome/browser/tab_contents/tab_contents_view.h
+++ b/chrome/browser/tab_contents/tab_contents_view.h
@@ -136,6 +136,16 @@ class TabContentsView : public RenderViewHostDelegate::View {
   }
   virtual void CancelDragAndCloseTab() {}
 
+  // If we close the tab while a UI control is in an event-tracking
+  // loop, the control may message freed objects and crash.
+  // TabContents::Close() calls IsEventTracking(), and if it returns
+  // true CloseTabAfterEventTracking() is called and the close is not
+  // completed.
+  virtual bool IsEventTracking() const {
+    return false;
+  }
+  virtual void CloseTabAfterEventTracking() {}
+
  protected:
   TabContentsView() {}  // Abstract interface.
 
diff --git a/chrome/browser/tab_contents/tab_contents_view_mac.h b/chrome/browser/tab_contents/tab_contents_view_mac.h
index 99e9f22..4da8267 100644
--- a/chrome/browser/tab_contents/tab_contents_view_mac.h
+++ b/chrome/browser/tab_contents/tab_contents_view_mac.h
@@ -44,6 +44,7 @@ class TabContentsViewMac : public TabContentsView,
   // lifetime. This doesn't need to be the case, but is this way currently
   // because that's what was easiest when they were split.
   explicit TabContentsViewMac(TabContents* web_contents);
+  virtual ~TabContentsViewMac();
 
   // TabContentsView implementation --------------------------------------------
 
@@ -66,6 +67,8 @@ class TabContentsViewMac : public TabContentsView,
                                                         bool activatable);
   virtual void ShowCreatedWidgetInternal(RenderWidgetHostView* widget_host_view,
                                          const gfx::Rect& initial_pos);
+  virtual bool IsEventTracking() const;
+  virtual void CloseTabAfterEventTracking();
 
   // Backend implementation of RenderViewHostDelegate::View.
   virtual void ShowContextMenu(const ContextMenuParams& params);
@@ -82,6 +85,10 @@ class TabContentsViewMac : public TabContentsView,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
+  // A helper method for closing the tab in the
+  // CloseTabAfterEventTracking() implementation.
+  void CloseTab();
+
  private:
   // The Cocoa NSView that lives in the view hierarchy.
   scoped_nsobject<TabContentsViewCocoa> cocoa_view_;
diff --git a/chrome/browser/tab_contents/tab_contents_view_mac.mm b/chrome/browser/tab_contents/tab_contents_view_mac.mm
index 0170dbe..f464c1e 100644
--- a/chrome/browser/tab_contents/tab_contents_view_mac.mm
+++ b/chrome/browser/tab_contents/tab_contents_view_mac.mm
@@ -8,6 +8,7 @@
 
 #include <string>
 
+#import "base/chrome_application_mac.h"
 #include "chrome/browser/browser.h" // TODO(beng): this dependency is awful.
 #import "chrome/browser/cocoa/focus_tracker.h"
 #import "chrome/browser/cocoa/chrome_browser_window.h"
@@ -51,6 +52,8 @@ COMPILE_ASSERT_MATCHING_ENUM(DragOperationEvery);
 - (void)setCurrentDragOperation:(NSDragOperation)operation;
 - (void)startDragWithDropData:(const WebDropData&)dropData
             dragOperationMask:(NSDragOperation)operationMask;
+- (void)cancelDeferredClose;
+- (void)closeTabAfterEvent;
 @end
 
 // static
@@ -64,6 +67,14 @@ TabContentsViewMac::TabContentsViewMac(TabContents* tab_contents)
                  Source<TabContents>(tab_contents));
 }
 
+TabContentsViewMac::~TabContentsViewMac() {
+  // This handles the case where a renderer close call was deferred
+  // while the user was operating a UI control which resulted in a
+  // close.  In that case, the Cocoa view outlives the
+  // TabContentsViewMac instance due to Cocoa retain count.
+  [cocoa_view_ cancelDeferredClose];
+}
+
 void TabContentsViewMac::CreateView(const gfx::Size& initial_size) {
   TabContentsViewCocoa* view =
       [[TabContentsViewCocoa alloc] initWithTabContentsViewMac:this];
@@ -255,6 +266,29 @@ void TabContentsViewMac::ShowCreatedWidgetInternal(
   [widget_view_mac->native_view() release];
 }
 
+bool TabContentsViewMac::IsEventTracking() const {
+  if ([NSApp isKindOfClass:[CrApplication class]] &&
+      [static_cast<CrApplication*>(NSApp) isHandlingSendEvent]) {
+    return true;
+  }
+  return false;
+}
+
+// Arrange to call CloseTab() after we're back to the main event loop.
+// The obvious way to do this would be PostNonNestableTask(), but that
+// will fire when the event-tracking loop polls for events.  So we
+// need to bounce the message via Cocoa, instead.
+void TabContentsViewMac::CloseTabAfterEventTracking() {
+  [cocoa_view_ cancelDeferredClose];
+  [cocoa_view_ performSelector:@selector(closeTabAfterEvent)
+                    withObject:nil
+                    afterDelay:0.0];
+}
+
+void TabContentsViewMac::CloseTab() {
+  tab_contents()->Close(tab_contents()->render_view_host());
+}
+
 void TabContentsViewMac::Observe(NotificationType type,
                                  const NotificationSource& source,
                                  const NotificationDetails& details) {
@@ -289,6 +323,9 @@ void TabContentsViewMac::Observe(NotificationType type,
 }
 
 - (void)dealloc {
+  // Cancel any deferred tab closes, just in case.
+  [self cancelDeferredClose];
+
   // This probably isn't strictly necessary, but can't hurt.
   [self unregisterDraggedTypes];
   [super dealloc];
@@ -474,6 +511,17 @@ void TabContentsViewMac::Observe(NotificationType type,
   return [dropTarget_ performDragOperation:sender view:self];
 }
 
+- (void)cancelDeferredClose {
+  SEL aSel = @selector(closeTabAfterEvent);
+  [NSObject cancelPreviousPerformRequestsWithTarget:self
+                                           selector:aSel
+                                             object:nil];
+}
+
+- (void)closeTabAfterEvent {
+  tabContentsView_->CloseTab();
+}
+
 // Tons of stuff goes here, where we grab events going on in Cocoaland and send
 // them into the C++ system. TODO(avi): all that jazz