cros: Properly call RemovePreTargetHandler() for AppListPresenterDelegateImpl.

AddPreTargetHandler() is called every time the app list is opened from closed
state, but RemovePreTargetHandler() is called in the destructor which is not
called when the app list is closed. This change properly calls
RemovePreTargetHandler() when the app list is closed.

Change-Id: Ic6f2556a3dabab43a7a7d2b63e81814b31058d60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1566478
Commit-Queue: Matthew Mourgos <mmourgos@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#650626}

ash/app_list/app_list_presenter_delegate_impl.cc

@@ -112,6 +112,7 @@ void AppListPresenterDelegateImpl::OnClosing() {
   DCHECK(is_visible_);
   DCHECK(view_);
   is_visible_ = false;
+  Shell::Get()->RemovePreTargetHandler(this);
   controller_->ViewClosing();
 }
 
@@ -230,7 +231,7 @@ void AppListPresenterDelegateImpl::ProcessLocatedEvent(
   }
 
   aura::Window* window = view_->GetWidget()->GetNativeView()->parent();
-  if (!window->Contains(target) && !presenter_->CloseOpenedPage() &&
+  if (!window->Contains(target) && !presenter_->HandleCloseOpenFolder() &&
       !app_list::switches::ShouldNotDismissOnBlur() && !IsTabletMode()) {
     const aura::Window* status_window =
         shelf->shelf_widget()->status_area_widget()->GetNativeWindow();
@@ -242,6 +243,10 @@ void AppListPresenterDelegateImpl::ProcessLocatedEvent(
     if (status_window && status_window->Contains(target))
       auto_hide_lock.emplace(shelf);
 
+    // Since event happened outside the app list, close the open search box if
+    // it is open.
+    presenter_->HandleCloseOpenSearchBox();
+
     // Keep app list opened if event happened in the shelf area.
     if (!shelf_window || !shelf_window->Contains(target))
       presenter_->Dismiss(event->time_stamp());

ash/app_list/app_list_presenter_delegate_unittest.cc

@@ -841,8 +841,8 @@ TEST_F(AppListPresenterDelegateTest,
       GetPrimaryShelf()->shelf_layout_manager()->GetShelfBackgroundType());
 }
 
-// Tests that the half app list transitions to peeking state and then closes
-// itself if the user taps outside its bounds.
+// Tests that the half app list closes itself if the user taps outside its
+// bounds.
 TEST_P(AppListPresenterDelegateTest, TapAndClickOutsideClosesHalfAppList) {
   // TODO(newcomer): Investigate mash failures crbug.com/726838
   GetAppListTestHelper()->ShowAndRunLoop(GetPrimaryDisplayId());
@@ -857,16 +857,6 @@ TEST_P(AppListPresenterDelegateTest, TapAndClickOutsideClosesHalfAppList) {
   gfx::Point to_point(
       0, GetAppListView()->GetWidget()->GetWindowBoundsInScreen().y() - 1);
 
-  // Clicking/tapping outside the bounds closes the search results page.
-  if (TestMouseEventParam()) {
-    generator->MoveMouseTo(to_point);
-    generator->ClickLeftButton();
-  } else {
-    generator->GestureTapAt(to_point);
-  }
-  GetAppListTestHelper()->WaitUntilIdle();
-  GetAppListTestHelper()->CheckState(ash::mojom::AppListViewState::kPeeking);
-
   // Clicking/tapping outside the bounds closes the app list.
   if (TestMouseEventParam()) {
     generator->MoveMouseTo(to_point);

ash/app_list/presenter/app_list_presenter_impl.cc

@@ -158,14 +158,12 @@ void AppListPresenterImpl::Dismiss(base::TimeTicks event_time_stamp) {
   base::RecordAction(base::UserMetricsAction("Launcher_Dismiss"));
 }
 
-bool AppListPresenterImpl::CloseOpenedPage() {
-  if (!is_visible_)
-    return false;
-
-  // If the app list is currently visible, there should be an existing view.
-  DCHECK(view_);
+bool AppListPresenterImpl::HandleCloseOpenFolder() {
+  return is_visible_ && view_ && view_->HandleCloseOpenFolder();
+}
 
-  return view_->CloseOpenedPage();
+bool AppListPresenterImpl::HandleCloseOpenSearchBox() {
+  return is_visible_ && view_ && view_->HandleCloseOpenSearchBox();
 }
 
 ash::ShelfAction AppListPresenterImpl::ToggleAppList(

ash/app_list/presenter/app_list_presenter_impl.h

@@ -81,9 +81,13 @@ class APP_LIST_PRESENTER_EXPORT AppListPresenterImpl
   // one AppListShowSource or focusing out side of the launcher.
   void Dismiss(base::TimeTicks event_time_stamp);
 
-  // Closes opened folder or search result page if they are opened. Returns
-  // whether the action was handled.
-  bool CloseOpenedPage();
+  // If app list has an opened folder, close it. Returns whether an opened
+  // folder was closed.
+  bool HandleCloseOpenFolder();
+
+  // If app list has an open search box, close it. Returns whether an open
+  // search box was closed.
+  bool HandleCloseOpenSearchBox();
 
   // Show the app list if it is visible, hide it if it is hidden. If
   // |event_time_stamp| is not 0, it means |ToggleAppList()| was triggered by

ash/app_list/views/app_list_view.cc

@@ -421,19 +421,31 @@ void AppListView::Dismiss() {
   GetWidget()->Deactivate();
 }
 
-bool AppListView::CloseOpenedPage() {
-  if (!app_list_main_view_)
-    return false;
+void AppListView::CloseOpenedPage() {
+  if (HandleCloseOpenFolder())
+    return;
+
+  HandleCloseOpenSearchBox();
+}
+
+bool AppListView::HandleCloseOpenFolder() {
+  if (GetAppsContainerView()->IsInFolderView()) {
+    GetAppsContainerView()->app_list_folder_view()->CloseFolderPage();
+    return true;
+  }
+  return false;
+}
 
-  if (app_list_main_view_->contents_view()->IsShowingSearchResults() ||
-      GetAppsContainerView()->IsInFolderView()) {
-    return app_list_main_view_->contents_view()->Back();
+bool AppListView::HandleCloseOpenSearchBox() {
+  if (app_list_main_view_ &&
+      app_list_main_view_->contents_view()->IsShowingSearchResults()) {
+    return Back();
   }
   return false;
 }
 
-void AppListView::Back() {
-  app_list_main_view_->contents_view()->Back();
+bool AppListView::Back() {
+  return app_list_main_view_->contents_view()->Back();
 }
 
 void AppListView::OnPaint(gfx::Canvas* canvas) {
@@ -461,9 +473,8 @@ bool AppListView::AcceleratorPressed(const ui::Accelerator& accelerator) {
     case ui::VKEY_BROWSER_BACK:
       // If the ContentsView does not handle the back action, then this is the
       // top level, so we close the app list.
-      if (!app_list_main_view_->contents_view()->Back() && !is_tablet_mode()) {
+      if (!Back() && !is_tablet_mode())
         Dismiss();
-      }
       break;
     default:
       NOTREACHED();

ash/app_list/views/app_list_view.h

@@ -147,12 +147,18 @@ class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView,
   // Dismisses the UI, cleans up and sets the state to CLOSED.
   void Dismiss();
 
-  // Closes opened folder or search result page if they are opened. Returns
-  // whether the action was handled.
-  bool CloseOpenedPage();
+  // Closes opened folder or search result page if they are opened.
+  void CloseOpenedPage();
+
+  // If a folder is open, close it. Returns whether an opened folder was closed.
+  bool HandleCloseOpenFolder();
+
+  // If a search box is open, close it. Returns whether an open search box was
+  // closed.
+  bool HandleCloseOpenSearchBox();
 
   // Performs the 'back' action for the active page.
-  void Back();
+  bool Back();
 
   // views::View:
   void OnPaint(gfx::Canvas* canvas) override;