Issue 21268: The RemakeSee http://codereview.chromium.org/21268 for the...

Issue 21268: The RemakeSee http://codereview.chromium.org/21268 for the original issue which was partially reverted due to a possible regression. Issue 8819 was created to track the part of the original fix which was reverted.A double-click come as an event which follows a single-click. The core of this change is to makesure that a double-click arrives at the object which handled the single-click which preceeded it.If it does not, then the double-click is transformed into a single-click at the dispatch level.The tricky part is that a single-click and the double-click which follows it may not both be clientor non-client. So, all double-clicks (client or non-client) are now centrally handled and replacedwhen needed by second single-clicks.Note that the behavior which was reverted was to discard a double-click rather than reducing it toa single-click. The bug filed as a regression involved using a double-middle-click as two single-clicks when destroying compressed tabs sequentially.BUG=8819
Review URL: http://codereview.chromium.org/50017

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

chrome/views/event.h

@@ -70,6 +70,10 @@ class Event {
     return flags_;
   }
 
+  void set_flags(int flags) {
+    flags_ = flags;
+  }
+
   // Return whether the shift modifier is down
   bool IsShiftDown() const {
     return (flags_ & EF_SHIFT_DOWN) != 0;
@@ -166,7 +170,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)

chrome/views/view.cc

@@ -462,7 +462,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;

chrome/views/widget/root_view.cc

@@ -59,6 +59,7 @@ const char RootView::kViewClassName[] = "chrome/views/RootView";
 RootView::RootView(Widget* widget)
   : mouse_pressed_handler_(NULL),
     mouse_move_handler_(NULL),
+    last_click_handler_(NULL),
     widget_(widget),
     invalid_rect_urgent_(false),
     pending_paint_task_(NULL),
@@ -272,8 +273,18 @@ void RootView::SetFocusOnMousePressed(bool f) {
 }
 
 bool RootView::OnMousePressed(const MouseEvent& e) {
-  UpdateCursor(e);
+  // This function does not normally handle non-client messages except for
+  // non-client double-clicks. Actually, all double-clicks are special as the
+  // are formed from a single-click followed by a double-click event. When the
+  // double-click event lands on a different view than its single-click part,
+  // we transform it into a single-click which prevents odd things.
+  if ((e.GetFlags() & MouseEvent::EF_IS_NON_CLIENT) &&
+      !(e.GetFlags() & MouseEvent::EF_IS_DOUBLE_CLICK)) {
+    last_click_handler_ = NULL;
+    return false;
+  }
 
+  UpdateCursor(e);
   SetMouseLocationAndFlags(e);
 
   // If mouse_pressed_handler_ is non null, we are currently processing
@@ -300,11 +311,17 @@ bool RootView::OnMousePressed(const MouseEvent& e) {
     }
 
     // See if this view wants to handle the mouse press.
-    const MouseEvent mouse_pressed_event(e, this, mouse_pressed_handler_);
+    MouseEvent mouse_pressed_event(e, this, mouse_pressed_handler_);
+
+    // Remove the double-click flag if the handler is different than the
+    // one which got the first click part of the double-click.
+    if (mouse_pressed_handler_ != last_click_handler_)
+      mouse_pressed_event.set_flags(e.GetFlags() &
+                                    ~MouseEvent::EF_IS_DOUBLE_CLICK);
+
     drag_info.Reset();
-    const bool handled =
-        mouse_pressed_handler_->ProcessMousePressed(mouse_pressed_event,
-                                                    &drag_info);
+    bool handled = mouse_pressed_handler_->ProcessMousePressed(
+        mouse_pressed_event, &drag_info);
 
     // The view could have removed itself from the tree when handling
     // OnMousePressed().  In this case, the removal notification will have
@@ -319,8 +336,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.
@@ -336,6 +355,15 @@ bool RootView::OnMousePressed(const MouseEvent& e) {
     NOTIMPLEMENTED();
 #endif
   }
+
+  // In the event that a double-click is not handled after traversing the
+  // entire hierarchy (even as a single-click when sent to a different view),
+  // it must be marked as handled to avoid anything happening from default
+  // processing if it the first click-part was handled by us.
+  if (last_click_handler_ && e.GetFlags() & MouseEvent::EF_IS_DOUBLE_CLICK)
+    hit_disabled_view = true;
+
+  last_click_handler_ = NULL;
   return hit_disabled_view;
 }
 

chrome/views/widget/root_view.h

@@ -286,6 +286,10 @@ class RootView : public View,
   // The view currently handling enter / exit
   View* mouse_move_handler_;
 
+  // The last view to handle a mouse click, so that we can determine if
+  // a double-click lands on the same view as its single-click part.
+  View* last_click_handler_;
+
   // The host Widget
   Widget* widget_;
 

chrome/views/widget/widget_win.cc

@@ -520,7 +520,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) {
@@ -528,11 +528,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) {
@@ -540,7 +540,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,
@@ -582,11 +582,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) {
@@ -594,11 +594,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) {
@@ -623,11 +623,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) {
@@ -653,7 +653,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) {
@@ -661,7 +661,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);
 }
 
 void WidgetWin::OnSettingChange(UINT flags, const wchar_t* section) {
@@ -712,16 +712,21 @@ 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;
   // Windows gives screen coordinates for nonclient events, while the RootView
   // expects window coordinates; convert if necessary.
   gfx::Point converted_point(point);
+  if (non_client)
+    View::ConvertPointToView(NULL, root_view_.get(), &converted_point);
   MouseEvent mouse_pressed(Event::ET_MOUSE_PRESSED,
                            converted_point.x(),
                            converted_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;

chrome/views/widget/widget_win.h

@@ -476,7 +476,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);

chrome/views/window/window_win.cc

@@ -493,7 +493,7 @@ LRESULT WindowWin::OnDwmCompositionChanged(UINT msg, WPARAM w_param,
   force_hidden_ = true;
   ::ShowWindow(GetNativeView(), SW_RESTORE);
   force_hidden_ = false;
-  
+
   // We respond to this in response to WM_DWMCOMPOSITIONCHANGED since that is
   // the only thing we care about - we don't actually respond to WM_THEMECHANGED
   // messages.
@@ -1210,7 +1210,7 @@ void WindowWin::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);
 }
 
 LRESULT WindowWin::CallDefaultNCActivateHandler(BOOL active) {