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..4fb633d69489b8da69c2a4e51ddee6dd9704eb0b 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
@@ -3,11 +3,12 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.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/window_manager_connection.h"
+#include "ui/views/pointer_watcher.h"

[mfomitchev, 2016/09/29 18:38:25]

We already include this in .h

[thanhph, 2016/09/29 19:43:04]

Good catch. Thanks!

 
 namespace {
 
@@ -36,10 +37,16 @@ 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) {}
+    : view_delegate_factory_(view_delegate_factory), presenter_(presenter) {}

[mfomitchev, 2016/09/29 18:38:25]

Nit: Would be better to have the argument order to match the init order (which
should also match the declaration order in .h). app_list_presenter_delegate.cc
has presenter_ first, so would be good to do the same here for consistency.

[thanhph, 2016/09/29 19:43:04]

I agree. I still don't know why I get the below error when I put the variable
presenter_ before view_delegate_factory_

chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.cc:41:8: error:
field 'presenter_' will be initialized after field 'view_delegate_factory_'
[-Werror,-Wreorder]
    :  presenter_(presenter), view_delegate_factory_(view_delegate_factory) {}

[mfomitchev, 2016/09/29 20:00:35]

The declaration order (in .h) needs to match the initialisation order in the
constructor.

[thanhph, 2016/09/29 20:42:51]

done. thanks!

 
-AppListPresenterDelegateMus::~AppListPresenterDelegateMus() {}
+AppListPresenterDelegateMus::~AppListPresenterDelegateMus() {
+  // Remove mouse/touch monitor for this delegate

[mfomitchev, 2016/09/29 18:38:25]

Nit: This comment isn't particularly helpful, I'd just remove it.

[thanhph, 2016/09/29 19:43:04]

Ok. Removed!

+  views::WindowManagerConnection::Get()
+      ->pointer_watcher_event_router()
+      ->RemovePointerWatcher(this);
+}
 
 app_list::AppListViewDelegate* AppListPresenterDelegateMus::GetViewDelegate() {
   return view_delegate_factory_->GetDelegate();
@@ -48,6 +55,7 @@ app_list::AppListViewDelegate* AppListPresenterDelegateMus::GetViewDelegate() {
 void AppListPresenterDelegateMus::Init(app_list::AppListView* view,
                                        int64_t display_id,
                                        int current_apps_page) {
+  LOG(ERROR) << "\n\n!!!!!. Initializing AppListPresenterDelegateMus\n\n\n";

[mfomitchev, 2016/09/29 18:38:25]

remove

[thanhph, 2016/09/29 19:43:04]

done!

   view_ = view;
 
   // Note: This would place the app list into the USER_WINDOWS container, unlike
@@ -65,6 +73,11 @@ void AppListPresenterDelegateMus::Init(app_list::AppListView* view,
   view->SetAnchorPoint(
       GetCenterOfDisplay(display_id, GetMinimumBoundsHeightForAppList(view)));
 
+  // add mouse-touch capture function to this delegate

[mfomitchev, 2016/09/29 18:38:26]

Nit: This comment isn't particularly helpful, I'd just remove it.

[thanhph, 2016/09/29 19:43:04]

removed. thanks!

+  views::WindowManagerConnection::Get()
+      ->pointer_watcher_event_router()
+      ->AddPointerWatcher(this, true);
+
   // TODO(mfomitchev): Setup updating bounds on keyboard bounds change.
   // TODO(mfomitchev): Setup dismissing on mouse/touch gesture anywhere outside

[mfomitchev, 2016/09/29 18:38:25]

You can now remove this TODO :)

[thanhph, 2016/09/29 19:43:04]

nice!

   // the bounds of the app list.
@@ -78,17 +91,32 @@ void AppListPresenterDelegateMus::OnShown(int64_t display_id) {
 }
 
 void AppListPresenterDelegateMus::OnDismissed() {
-  DCHECK(is_visible_);
-  is_visible_ = false;
 }
 
 void AppListPresenterDelegateMus::UpdateBounds() {
   if (!view_ || !is_visible_)
+

[mfomitchev, 2016/09/29 18:38:26]

remove blank line

[thanhph, 2016/09/29 20:42:51]

ok

     return;
 
   view_->UpdateBounds();
 }
 
+void AppListPresenterDelegateMus::OnPointerEventObserved(
+    const ui::PointerEvent& event,
+    const gfx::Point& location_in_screen,
+    views::Widget* target) {
+  if (event.type() == ui::ET_TOUCH_PRESSED ||

[mfomitchev, 2016/09/29 18:38:26]

Nit: Add a comment in the beginning saying that the purpose of this is to
dismiss app list on a mouse click or touch outside of the app list window.

[thanhph, 2016/09/29 19:43:04]

done! thanks!

+      event.type() == ui::ET_POINTER_DOWN) {
+    if (!target) {
+      presenter_->Dismiss();
+    } else if (!target->GetWindowBoundsInScreen().ApproximatelyEqual(

[mfomitchev, 2016/09/29 18:38:25]

Explain to me again why can't we just compare target against view_->GetWidget()?

[thanhph, 2016/09/29 19:43:04]

It works indeed. I change the code to target != view_->GetWidget(), which looks
cleaner. thanks!

+                   view_->GetWidget()->GetWindowBoundsInScreen(), 0)) {
+      // else close the app list widget if clicks elsewhere
+      presenter_->Dismiss();
+    }
+  }
+}
+
 gfx::Vector2d AppListPresenterDelegateMus::GetVisibilityAnimationOffset(
     aura::Window* root_window) {
   // TODO(mfomitchev): Classic ash does different animation here depending on