Add app_list_presenterr pointer in app_list_presenter_delegate_mus to use dismiss function

chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.cc

Index: chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.cc
diff --git a/chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.cc b/chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.cc
index 979835f6d280510c829273f5b88028e2293c454f..e9d3b79c777296b706d47058ced5764b31ad2ae5 100644
--- a/chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.cc
+++ b/chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.cc
@@ -4,10 +4,13 @@
 
 #include "chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.h"
 
+#include "ui/app_list/presenter/app_list_presenter.h"
 #include "ui/app_list/presenter/app_list_view_delegate_factory.h"
 #include "ui/app_list/views/app_list_view.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
+#include "ui/views/mus/pointer_watcher_event_router.h"
+#include "ui/views/mus/window_manager_connection.h"
 
 namespace {
 
@@ -36,10 +39,19 @@ gfx::Point GetCenterOfDisplay(int64_t display_id, int minimum_height) {
 }  // namespace
 
 AppListPresenterDelegateMus::AppListPresenterDelegateMus(
+    app_list::AppListPresenter* presenter,
     app_list::AppListViewDelegateFactory* view_delegate_factory)
-    : view_delegate_factory_(view_delegate_factory) {}
+    : presenter_(presenter), view_delegate_factory_(view_delegate_factory) {
+  views::WindowManagerConnection::Get()
+      ->pointer_watcher_event_router()
+      ->AddPointerWatcher(this, false);
+}
 
-AppListPresenterDelegateMus::~AppListPresenterDelegateMus() {}
+AppListPresenterDelegateMus::~AppListPresenterDelegateMus() {
+  views::WindowManagerConnection::Get()

[mfomitchev, 2016/09/30 16:36:14]

Sorry.. this is probably not a huge deal in practice, but doing a trip to app
list delegate for every single click/touch anywhere on the screen feels bad. Can
we instead add the watcher when the app list is shown (in OnShown) and remove it
when it is dismissed (in OnDismissed)? And then we also need to remove it in the
destructor if is_visible is true.

[thanhph, 2016/09/30 17:49:23]

By adding log output in constructor and destructor, I notice the trip to app
list delegate only got called once when starting the widget and upon object
exit) (also not get called when we click inside the app list widget). but your
approach is good when the program has a weird behavior that destructor doesn't
get called. I also flip the flag is_visible_ where appropriate (Constructor,
Destructor, OnShown, OnDismissed) according to this approach. Thanks!

[mfomitchev, 2016/09/30 18:25:45]

Oh, I didn't realize that the delegate gets destroyed when the app list is
hidden. If that is the case, this is much less of an issue, feel free to
disregard my comment. 

+      ->pointer_watcher_event_router()
+      ->RemovePointerWatcher(this);
+}
 
 app_list::AppListViewDelegate* AppListPresenterDelegateMus::GetViewDelegate() {
   return view_delegate_factory_->GetDelegate();
@@ -66,8 +78,6 @@ void AppListPresenterDelegateMus::Init(app_list::AppListView* view,
       GetCenterOfDisplay(display_id, GetMinimumBoundsHeightForAppList(view)));
 
   // TODO(mfomitchev): Setup updating bounds on keyboard bounds change.
-  // TODO(mfomitchev): Setup dismissing on mouse/touch gesture anywhere outside
-  // the bounds of the app list.
   // TODO(mfomitchev): Setup dismissing on maximize (touch-view) mode start/end.
   // TODO(mfomitchev): Setup DnD.
   // TODO(mfomitchev): UpdateAutoHideState for shelf
@@ -78,8 +88,6 @@ void AppListPresenterDelegateMus::OnShown(int64_t display_id) {
 }
 
 void AppListPresenterDelegateMus::OnDismissed() {
-  DCHECK(is_visible_);

[mfomitchev, 2016/09/30 16:36:14]

Just noticed this was removed. This code should stay.

-  is_visible_ = false;
 }
 
 void AppListPresenterDelegateMus::UpdateBounds() {
@@ -89,6 +97,18 @@ void AppListPresenterDelegateMus::UpdateBounds() {
   view_->UpdateBounds();
 }
 
+void AppListPresenterDelegateMus::OnPointerEventObserved(
+    const ui::PointerEvent& event,
+    const gfx::Point& location_in_screen,
+    views::Widget* target) {
+  // Dismiss app list on a mouse click or touch outside of the app list window.

[mfomitchev, 2016/09/30 16:36:14]

If you go with my suggestion for adding/removing pointer watcher, we can also
add DCHECK(is_visible_) here.

[thanhph, 2016/09/30 17:49:23]

done, thanks! Is it a good practice to add more DCHECK?

[mfomitchev, 2016/09/30 18:25:46]

Yeah, if there is an invariant you expect to hold,l DCHECK both catches bugs and
serves as a concies replacement for a comment.

+  if (event.type() == ui::ET_TOUCH_PRESSED ||
+      event.type() == ui::ET_POINTER_DOWN) {
+    if (!target || target != view_->GetWidget())
+      presenter_->Dismiss();
+  }
+}
+
 gfx::Vector2d AppListPresenterDelegateMus::GetVisibilityAnimationOffset(
     aura::Window* root_window) {
   // TODO(mfomitchev): Classic ash does different animation here depending on