Solved 2 bugs which caused Chrome to maximize itself whendouble clicking, either on the new tab button, on the closetab button or on a single tab.BUG=2827BUG=3787The problem comes from the Windows event sequence upon adouble-click (simplified here):1 - hit-test2 - mouse-down4 - mouse-up/click5 - hit-test6 - mouse down7 - mouse up/double-clickThe 1st hit-test is always performed correctly, returningclient for tabs and non-client for the tab-strip (background).The 2nd hit test is not performed correctly to avoid crashesin Chromebot from events being processed while tabs are animating.Since we have no record of these crashes, Ben prefers we keepthis special-case, even though we are responding incorrectlyto the windows hit-test. So, when the tabs are animating wereturn a HTNOWHERE hit which the caller translates into anHTCAPTION hit. This even though a tab-control (new-tab/close-tab)may have been hit.The problem is that having returned HTCAPTION to Windows defaultmessage handling, we get a NON-CLIENT double-click event insteadof a standard one.To keep the behavior of the second hit-test AND prevent theChrome window from maximizing, this change simply declaresthe non-client double-click as handled when the tabs areanimating.Another trick we pulled in the hit-test is to return HTCAPTIONwhen a single tab is present. This allows the entire window to be dragged but causes the context menu to be wrong and the windowto maximize when double clicking on the single tab.The solution here is to correct return a client hit for a singletab and, upon handling a client single-click, delegate to thenon-client single-click default handler.

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

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

11 files changed, 72 insertions, 42 deletions

diff --git a/chrome/browser/views/tabs/tab.cc b/chrome/browser/views/tabs/tab.cc
index cdc3502..86f0b2c 100644
--- a/chrome/browser/views/tabs/tab.cc
+++ b/chrome/browser/views/tabs/tab.cc
@@ -158,7 +158,16 @@ void Tab::GetHitTestMask(gfx::Path* mask) const {
 }
 
 bool Tab::OnMousePressed(const views::MouseEvent& event) {
-  if (event.IsOnlyLeftMouseButton()) {
+  if (event.IsLeftMouseButton()) {
+    // When only one tab is present, instead of ripping it out on drag,
+    // it dragged the whole window. This is done by sending a non-client
+    // message which is handled by the default window procedure and causes
+    // the window get the default drag-on-caption behavior.
+    if (delegate_->ContainsExactlyOneTab()) {
+      SendMessage(GetWidget()->GetHWND(), WM_NCLBUTTONDOWN,
+                  HTCAPTION, MAKELPARAM(event.x(), event.y()));
+      return false;
+    }
     // Store whether or not we were selected just now... we only want to be
     // able to drag foreground tabs, so we don't start dragging the tab if
     // it was in the background.
diff --git a/chrome/browser/views/tabs/tab.h b/chrome/browser/views/tabs/tab.h
index de3c981..40577cf 100644
--- a/chrome/browser/views/tabs/tab.h
+++ b/chrome/browser/views/tabs/tab.h
@@ -69,6 +69,9 @@ class Tab : public TabRenderer,
     // other than the user releasing the mouse. Returns whether the tab has been
     // destroyed.
     virtual bool EndDrag(bool canceled) = 0;
+
+    // Returns true if only one tab exists.
+    virtual bool ContainsExactlyOneTab() const = 0;
   };
 
   explicit Tab(TabDelegate* delegate);
diff --git a/chrome/browser/views/tabs/tab_strip.cc b/chrome/browser/views/tabs/tab_strip.cc
index 693ba35..efd7658 100644
--- a/chrome/browser/views/tabs/tab_strip.cc
+++ b/chrome/browser/views/tabs/tab_strip.cc
@@ -529,11 +529,6 @@ bool TabStrip::PointIsWithinWindowCaption(const gfx::Point& point) {
   if (v == this)
     return true;
 
-  // If the point is within the bounds of a Tab, the point can be considered
-  // part of the caption if there are no available drag operations for the Tab.
-  if (v->GetClassName() == Tab::kTabClassName && !HasAvailableDragActions())
-    return true;
-
   // Check to see if the point is within the non-button parts of the new tab
   // button. The button has a non-rectangular shape, so if it's not in the
   // visual portions of the button we treat it as a click to the caption.
@@ -1027,6 +1022,10 @@ bool TabStrip::EndDrag(bool canceled) {
   return drag_controller_.get() ? drag_controller_->EndDrag(canceled) : false;
 }
 
+bool TabStrip::ContainsExactlyOneTab() const {
+  return GetTabCount() == 1;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // TabStrip, views::BaseButton::ButtonListener implementation:
 
diff --git a/chrome/browser/views/tabs/tab_strip.h b/chrome/browser/views/tabs/tab_strip.h
index 36398c7..3dcf1a0 100644
--- a/chrome/browser/views/tabs/tab_strip.h
+++ b/chrome/browser/views/tabs/tab_strip.h
@@ -142,6 +142,7 @@ class TabStrip : public views::View,
   virtual void MaybeStartDrag(Tab* tab, const views::MouseEvent& event);
   virtual void ContinueDrag(const views::MouseEvent& event);
   virtual bool EndDrag(bool canceled);
+  virtual bool ContainsExactlyOneTab() const;
 
   // views::Button::ButtonListener implementation:
   virtual void ButtonPressed(views::BaseButton* sender);
diff --git a/chrome/views/custom_frame_window.cc b/chrome/views/custom_frame_window.cc
index e4a7ef6..4e57ee0 100644
--- a/chrome/views/custom_frame_window.cc
+++ b/chrome/views/custom_frame_window.cc
@@ -1149,20 +1149,6 @@ void CustomFrameWindow::OnNCLButtonDown(UINT ht_component,
   }
 }
 
-void CustomFrameWindow::OnNCMButtonDown(UINT ht_component,
-                                        const CPoint& point) {
-  if (ht_component == HTCAPTION) {
-    // When there's only one window and only one tab, the tab area is reported
-    // to be part of the caption area of the window. However users should still
-    // be able to middle click that tab to close it so we need to make sure
-    // these messages reach the View system.
-    ProcessNCMousePress(point, MK_MBUTTON);
-    SetMsgHandled(FALSE);
-    return;
-  }
-  WidgetWin::OnNCMButtonDown(ht_component, point);
-}
-
 LRESULT CustomFrameWindow::OnNCUAHDrawCaption(UINT msg, WPARAM w_param,
                                               LPARAM l_param) {
   // See comment in widget_win.h at the definition of WM_NCUAHDRAWCAPTION for
@@ -1319,8 +1305,7 @@ void CustomFrameWindow::ProcessNCMousePress(const CPoint& point, int flags) {
   if ((GetKeyState(VK_SHIFT) & 0x80) == 0x80)
     message_flags |= MK_SHIFT;
   message_flags |= flags;
-  ProcessMousePressed(temp, message_flags, false);
+  ProcessMousePressed(temp, message_flags, false, false);
 }
 
 }  // namespace views
-
diff --git a/chrome/views/custom_frame_window.h b/chrome/views/custom_frame_window.h
index 73ec4ef3..3d16874 100644
--- a/chrome/views/custom_frame_window.h
+++ b/chrome/views/custom_frame_window.h
@@ -55,7 +55,6 @@ class CustomFrameWindow : public Window {
   virtual LRESULT OnNCHitTest(const CPoint& point);
   virtual void OnNCPaint(HRGN rgn);
   virtual void OnNCLButtonDown(UINT ht_component, const CPoint& point);
-  virtual void OnNCMButtonDown(UINT ht_component, const CPoint& point);
   virtual LRESULT OnNCUAHDrawCaption(UINT msg, WPARAM w_param, LPARAM l_param);
   virtual LRESULT OnNCUAHDrawFrame(UINT msg, WPARAM w_param, LPARAM l_param);
   virtual LRESULT OnSetCursor(HWND window, UINT hittest_code, UINT message);
diff --git a/chrome/views/event.h b/chrome/views/event.h
index 35ebfbe..6a6501e 100644
--- a/chrome/views/event.h
+++ b/chrome/views/event.h
@@ -156,7 +156,10 @@ class LocatedEvent : public Event {
 class MouseEvent : public LocatedEvent {
  public:
   // Flags specific to mouse events
-  enum MouseEventFlags { EF_IS_DOUBLE_CLICK = 1 << 16 };
+  enum MouseEventFlags {
+    EF_IS_DOUBLE_CLICK = 1 << 16,
+    EF_IS_NON_CLIENT = 1 << 17
+  };
 
   // Create a new mouse event
   MouseEvent(EventType type, int x, int y, int flags)
diff --git a/chrome/views/root_view.cc b/chrome/views/root_view.cc
index 10c11fc..4ec6eda 100644
--- a/chrome/views/root_view.cc
+++ b/chrome/views/root_view.cc
@@ -265,8 +265,19 @@ void RootView::SetFocusOnMousePressed(bool f) {
 }
 
 bool RootView::OnMousePressed(const MouseEvent& e) {
-  UpdateCursor(e);
+  static View* last_click_handler = 0;
+
+  // This function is not to handle non-client messages, so we return that
+  // we are not handling it quickly except for the double-click because we
+  // need to absorb it when it occurs on a different view than its single
+  // click part.
+  if ((e.GetFlags() & MouseEvent::EF_IS_NON_CLIENT) &&
+      !(e.GetFlags() & MouseEvent::EF_IS_DOUBLE_CLICK)) {
+    last_click_handler = 0;
+    return false;
+  }
 
+  UpdateCursor(e);
   SetMouseLocationAndFlags(e);
 
   // If mouse_pressed_handler_ is non null, we are currently processing
@@ -296,6 +307,8 @@ bool RootView::OnMousePressed(const MouseEvent& e) {
     const MouseEvent mouse_pressed_event(e, this, mouse_pressed_handler_);
     drag_info.Reset();
     const bool handled =
+        (!(e.GetFlags() & MouseEvent::EF_IS_DOUBLE_CLICK) ||
+        (mouse_move_handler_ == last_click_handler)) &&
         mouse_pressed_handler_->ProcessMousePressed(mouse_pressed_event,
                                                     &drag_info);
 
@@ -312,8 +325,10 @@ bool RootView::OnMousePressed(const MouseEvent& e) {
     // If the view handled the event, leave mouse_pressed_handler_ set and
     // return true, which will cause subsequent drag/release events to get
     // forwarded to that view.
-    if (handled)
+    if (handled) {
+      last_click_handler = mouse_pressed_handler_;
       return true;
+    }
   }
 
   // Reset mouse_pressed_handler_ to indicate that no processing is occurring.
@@ -329,6 +344,15 @@ bool RootView::OnMousePressed(const MouseEvent& e) {
     NOTIMPLEMENTED();
 #endif
   }
+
+  // If we go through the whole hierarchy and we did not find the same handler
+  // for the double-click as we did for the single-click, then mark it as
+  // handled to eat up any double-click that ends up in a different location
+  // than its single-click part.
+  if (last_click_handler && e.GetFlags() & MouseEvent::EF_IS_DOUBLE_CLICK)
+    hit_disabled_view = true;
+
+  last_click_handler = 0;
   return hit_disabled_view;
 }
 
diff --git a/chrome/views/view.cc b/chrome/views/view.cc
index 8d1926f..0385df4 100644
--- a/chrome/views/view.cc
+++ b/chrome/views/view.cc
@@ -457,7 +457,8 @@ bool View::ProcessMousePressed(const MouseEvent& e, DragInfo* drag_info) {
     drag_operations = GetDragOperations(e.x(), e.y());
   else
     drag_operations = 0;
-  ContextMenuController* context_menu_controller = context_menu_controller_;
+  ContextMenuController* context_menu_controller =
+      e.IsRightMouseButton()? context_menu_controller_ : 0;
 
   const bool result = OnMousePressed(e);
   // WARNING: we may have been deleted, don't use any View variables;
diff --git a/chrome/views/widget_win.cc b/chrome/views/widget_win.cc
index 5e60e38..a92d6a0 100644
--- a/chrome/views/widget_win.cc
+++ b/chrome/views/widget_win.cc
@@ -517,7 +517,7 @@ void WidgetWin::OnKeyUp(TCHAR c, UINT rep_cnt, UINT flags) {
 }
 
 void WidgetWin::OnLButtonDown(UINT flags, const CPoint& point) {
-  ProcessMousePressed(point, flags | MK_LBUTTON, false);
+  ProcessMousePressed(point, flags | MK_LBUTTON, false, false);
 }
 
 void WidgetWin::OnLButtonUp(UINT flags, const CPoint& point) {
@@ -525,11 +525,11 @@ void WidgetWin::OnLButtonUp(UINT flags, const CPoint& point) {
 }
 
 void WidgetWin::OnLButtonDblClk(UINT flags, const CPoint& point) {
-  ProcessMousePressed(point, flags | MK_LBUTTON, true);
+  ProcessMousePressed(point, flags | MK_LBUTTON, true, false);
 }
 
 void WidgetWin::OnMButtonDown(UINT flags, const CPoint& point) {
-  ProcessMousePressed(point, flags | MK_MBUTTON, false);
+  ProcessMousePressed(point, flags | MK_MBUTTON, false, false);
 }
 
 void WidgetWin::OnMButtonUp(UINT flags, const CPoint& point) {
@@ -537,7 +537,7 @@ void WidgetWin::OnMButtonUp(UINT flags, const CPoint& point) {
 }
 
 void WidgetWin::OnMButtonDblClk(UINT flags, const CPoint& point) {
-  ProcessMousePressed(point, flags | MK_MBUTTON, true);
+  ProcessMousePressed(point, flags | MK_MBUTTON, true, false);
 }
 
 LRESULT WidgetWin::OnMouseActivate(HWND window, UINT hittest_code,
@@ -572,11 +572,11 @@ LRESULT WidgetWin::OnMouseRange(UINT msg, WPARAM w_param, LPARAM l_param) {
 }
 
 void WidgetWin::OnNCLButtonDblClk(UINT flags, const CPoint& point) {
-  SetMsgHandled(FALSE);
+  SetMsgHandled(ProcessMousePressed(point, flags | MK_LBUTTON, true, true));
 }
 
 void WidgetWin::OnNCLButtonDown(UINT flags, const CPoint& point) {
-  SetMsgHandled(FALSE);
+  SetMsgHandled(ProcessMousePressed(point, flags | MK_LBUTTON, false, true));
 }
 
 void WidgetWin::OnNCLButtonUp(UINT flags, const CPoint& point) {
@@ -584,11 +584,11 @@ void WidgetWin::OnNCLButtonUp(UINT flags, const CPoint& point) {
 }
 
 void WidgetWin::OnNCMButtonDblClk(UINT flags, const CPoint& point) {
-  SetMsgHandled(FALSE);
+  SetMsgHandled(ProcessMousePressed(point, flags | MK_MBUTTON, true, true));
 }
 
 void WidgetWin::OnNCMButtonDown(UINT flags, const CPoint& point) {
-  SetMsgHandled(FALSE);
+  SetMsgHandled(ProcessMousePressed(point, flags | MK_MBUTTON, false, true));
 }
 
 void WidgetWin::OnNCMButtonUp(UINT flags, const CPoint& point) {
@@ -613,11 +613,11 @@ LRESULT WidgetWin::OnNCMouseMove(UINT flags, const CPoint& point) {
 }
 
 void WidgetWin::OnNCRButtonDblClk(UINT flags, const CPoint& point) {
-  SetMsgHandled(FALSE);
+  SetMsgHandled(ProcessMousePressed(point, flags | MK_RBUTTON, true, true));
 }
 
 void WidgetWin::OnNCRButtonDown(UINT flags, const CPoint& point) {
-  SetMsgHandled(FALSE);
+  SetMsgHandled(ProcessMousePressed(point, flags | MK_RBUTTON, false, true));
 }
 
 void WidgetWin::OnNCRButtonUp(UINT flags, const CPoint& point) {
@@ -643,7 +643,7 @@ void WidgetWin::OnPaint(HDC dc) {
 }
 
 void WidgetWin::OnRButtonDown(UINT flags, const CPoint& point) {
-  ProcessMousePressed(point, flags | MK_RBUTTON, false);
+  ProcessMousePressed(point, flags | MK_RBUTTON, false, false);
 }
 
 void WidgetWin::OnRButtonUp(UINT flags, const CPoint& point) {
@@ -651,7 +651,7 @@ void WidgetWin::OnRButtonUp(UINT flags, const CPoint& point) {
 }
 
 void WidgetWin::OnRButtonDblClk(UINT flags, const CPoint& point) {
-  ProcessMousePressed(point, flags | MK_RBUTTON, true);
+  ProcessMousePressed(point, flags | MK_RBUTTON, true, false);
 }
 
 LRESULT WidgetWin::OnSettingChange(UINT msg, WPARAM w_param, LPARAM l_param) {
@@ -708,13 +708,16 @@ void WidgetWin::TrackMouseEvents(DWORD mouse_tracking_flags) {
   }
 }
 
-bool WidgetWin::ProcessMousePressed(const CPoint& point, UINT flags,
-                                    bool dbl_click) {
+bool WidgetWin::ProcessMousePressed(const CPoint& point,
+                                    UINT flags,
+                                    bool dbl_click,
+                                    bool non_client) {
   last_mouse_event_was_move_ = false;
   MouseEvent mouse_pressed(Event::ET_MOUSE_PRESSED,
                            point.x,
                            point.y,
                            (dbl_click ? MouseEvent::EF_IS_DOUBLE_CLICK : 0) |
+                           (non_client ? MouseEvent::EF_IS_NON_CLIENT : 0) |
                            Event::ConvertWindowsFlags(flags));
   if (root_view_->OnMousePressed(mouse_pressed)) {
     is_mouse_down_ = true;
diff --git a/chrome/views/widget_win.h b/chrome/views/widget_win.h
index e16c3f6..6710aee 100644
--- a/chrome/views/widget_win.h
+++ b/chrome/views/widget_win.h
@@ -463,7 +463,10 @@ class WidgetWin : public Widget,
   // Actually handle mouse events. These functions are called by subclasses who
   // override the message handlers above to do the actual real work of handling
   // the event in the View system.
-  bool ProcessMousePressed(const CPoint& point, UINT flags, bool dbl_click);
+  bool ProcessMousePressed(const CPoint& point,
+                           UINT flags,
+                           bool dbl_click,
+                           bool non_client);
   void ProcessMouseDragged(const CPoint& point, UINT flags);
   void ProcessMouseReleased(const CPoint& point, UINT flags);
   void ProcessMouseMoved(const CPoint& point, UINT flags, bool is_nonclient);