Make the second click on the app shelf icon close the window-list bubble.

ash/shelf/shelf_view.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/shelf/shelf_view.h"
 
 #include <algorithm>
 
 #include "ash/ash_constants.h"
 #include "ash/ash_switches.h"
@@ skipping 371 matching lines @@
       closing_event_time_(base::TimeDelta()),
       got_deleted_(NULL),
       drag_and_drop_item_pinned_(false),
       drag_and_drop_shelf_id_(0),
       dragged_off_shelf_(false),
       snap_back_from_rip_off_view_(NULL),
       item_manager_(Shell::GetInstance()->shelf_item_delegate_manager()),
       layout_manager_(manager),
       overflow_mode_(false),
       main_shelf_(NULL),
-      dragged_off_from_overflow_to_shelf_(false) {
+      dragged_off_from_overflow_to_shelf_(false),
+      is_usable_event_(true),
+      last_view_index_(-1) {
   DCHECK(model_);
   bounds_animator_.reset(new views::BoundsAnimator(this));
   bounds_animator_->AddObserver(this);
   set_context_menu_controller(this);
   focus_search_.reset(new ShelfFocusSearch(view_model_.get()));
   tooltip_.reset(new ShelfTooltipManager(manager, this));
 }
 
 ShelfView::~ShelfView() {
   bounds_animator_->RemoveObserver(this);
@@ skipping 1161 matching lines @@
 
   int index = view_model_->GetIndexOfView(view);
   if (index == -1)
     return;
 
   ShelfItemDelegate* item_delegate = item_manager_->GetShelfItemDelegate(
       model_->items()[index].id);
   if (view_model_->view_size() <= 1 || !item_delegate->IsDraggable())
     return;  // View is being deleted or not draggable, ignore request.
 
+  if (!IsUsableEvent(event)) {

[Mr4D (OOO till 08-26), 2014/12/05 15:21:47]

No {}'s

[Mr4D (OOO till 08-26), 2014/12/05 15:21:47]

Aha! Nice find! Yes, this is much better then the suggested class override! Good
job!

+    is_usable_event_ = false;
+  }
+
   CHECK_EQ(ShelfButton::kViewClassName, view->GetClassName());
   drag_view_ = static_cast<ShelfButton*>(view);
   drag_origin_ = gfx::Point(event.x(), event.y());
   UMA_HISTOGRAM_ENUMERATION("Ash.ShelfAlignmentUsage",
       layout_manager_->SelectValueForShelfAlignment(
           SHELF_ALIGNMENT_UMA_ENUM_VALUE_BOTTOM,
           SHELF_ALIGNMENT_UMA_ENUM_VALUE_LEFT,
           SHELF_ALIGNMENT_UMA_ENUM_VALUE_RIGHT,
           -1),
       SHELF_ALIGNMENT_UMA_ENUM_VALUE_COUNT);
 }
 
 void ShelfView::PointerDraggedOnButton(views::View* view,
                                        Pointer pointer,
                                        const ui::LocatedEvent& event) {
   // To prepare all drag types (moving an item in the shelf and dragging off),
   // we should check the x-axis and y-axis offset.
   if (!dragging() && drag_view_ &&
       ((std::abs(event.x() - drag_origin_.x()) >= kMinimumDragDistance) ||
        (std::abs(event.y() - drag_origin_.y()) >= kMinimumDragDistance))) {
     PrepareForDrag(pointer, event);
   }
   if (drag_pointer_ == pointer)
     ContinueDrag(event);
 }
 
 void ShelfView::PointerReleasedOnButton(views::View* view,
                                         Pointer pointer,
                                         bool canceled) {

[Mr4D (OOO till 08-26), 2014/12/05 15:21:47]

Reset the variable.

(Note: If you would not do it, you couldn't use the button until you have
clicked another one...)

[xdai1, 2014/12/05 19:01:46]

I reset the variable at line 1675 and line 1680. I think that should do the
work. 

[Mr4D (OOO till 08-26), 2014/12/05 20:57:13]

It's not enough. This could be failing:

Click on the button while the menu is open and hold it pressed. Drag the mouse
out. Then lift the button. go back to the button and click. No menu should show.

Alternatively you could do in line 1576 
is_usable_event_ = IsUsableEvent(event);
and get rid of the if (..).

   if (canceled) {
     CancelDrag(-1);
   } else if (drag_pointer_ == pointer) {
     FinalizeRipOffDrag(false);
     drag_pointer_ = NONE;
     AnimateToIdealBounds();
   }
   // If the drag pointer is NONE, no drag operation is going on and the
   // drag_view can be released.
   if (drag_pointer_ == NONE)
@@ skipping 43 matching lines @@
   if (sender == overflow_button_) {
     ToggleOverflowBubble();
     return;
   }
 
   int view_index = view_model_->GetIndexOfView(sender);
   // May be -1 while in the process of animating closed.
   if (view_index == -1)
     return;
 
-  // If the previous menu was closed by the same event as this one, we ignore
-  // the call.
-  if (!IsUsableEvent(event))
+  // If the menu was just closed by the same event as this one, we ignore
+  // the call. See crbug.com/343005 for more detail.
+  if (!is_usable_event_ && (view_index == last_view_index_)) {

[Mr4D (OOO till 08-26), 2014/12/05 15:21:47]

The index does not need to be checked since we are in a press/release cycle.
As far as I know it is not possible to press multiple buttons at once via touch.

[xdai1, 2014/12/05 19:01:46]

I think we still need to check the index because only if the second click is on
the same shelf icon (view_index == last_view_index_), it should be ignored. But
if the second click is on a different shelf icon (view_index !=
last_view_index_), we still need to handle it (first close the previous menu,
and then open the current menu).

+    is_usable_event_ = true;
     return;
+  }
+
+  // Reset the |is_usable_event_| to true and record the |view_index|.
+  is_usable_event_ = true;
+  last_view_index_ = view_index;
 
   // Don't activate the item twice on double-click. Otherwise the window starts
   // animating open due to the first click, then immediately minimizes due to
   // the second click. The user most likely intended to open or minimize the
   // item once, not do both.
   if (event.flags() & ui::EF_IS_DOUBLE_CLICK)
     return;
 
   {
     ScopedTargetRootWindow scoped_target(
@@ skipping 190 matching lines @@
           ShelfButton* button = static_cast<ShelfButton*>(view);
           button->ClearState(ShelfButton::STATE_HIDDEN);
           break;
         }
       }
       snap_back_from_rip_off_view_ = NULL;
     }
   }
 }
 
 bool ShelfView::IsUsableEvent(const ui::Event& event) {

[Mr4D (OOO till 08-26), 2014/12/05 15:21:47]

Rename to IsRepostEvent() (and negate the logic accordingly). Somehow that
expresses better what this does.

   if (closing_event_time_ == base::TimeDelta())
     return true;
 
   base::TimeDelta delta =
       base::TimeDelta(event.time_stamp() - closing_event_time_);
   closing_event_time_ = base::TimeDelta();
-  // TODO(skuhne): This time seems excessive, but it appears that the reposting
-  // takes that long.  Need to come up with a better way of doing this.
-  return (delta.InMilliseconds() < 0 || delta.InMilliseconds() > 130);
+  // If the current (press down) event is a reposted event, the time stamp of
+  // these two events should be the same.
+  return (delta.InMilliseconds() != 0);

[Mr4D (OOO till 08-26), 2014/12/05 15:21:47]

Ahhhh! Much better!

 }
 
 const ShelfItem* ShelfView::ShelfItemForView(const views::View* view) const {
   int view_index = view_model_->GetIndexOfView(view);
   if (view_index == -1)
     return NULL;
   return &(model_->items()[view_index]);
 }
 
 bool ShelfView::ShouldShowTooltipForView(const views::View* view) const {
@@ skipping 25 matching lines @@
       distance = bounds.x() - coordinate.x();
       break;
     case SHELF_ALIGNMENT_TOP:
       distance = coordinate.y() - bounds.bottom();
       break;
   }
   return distance > 0 ? distance : 0;
 }
 
 }  // namespace ash