app_list: Dimiss launcher in common code paths

When opening app in app list or in the search result, we want to dismiss
the launcher first while waiting the app to be opened. This cl
consolidates the dismiss codes to common code paths.

Bug: 851555
Test: manual.
Change-Id: I0230838c368c74c0acbf3e889ac9162be3abdb87
Reviewed-on: https://chromium-review.googlesource.com/1105279
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568264}

chrome/browser/ui/app_list/arc/arc_app_item.cc

@@ -54,15 +54,10 @@ void ArcAppItem::Activate(int event_flags) {
                       GetController()->GetAppListDisplayId())) {
     return;
   }
-
-  // Manually close app_list view because focus is not changed on ARC app start,
-  // and current view remains active. Do not close app list for home launcher.
-  if (!GetController()->IsHomeLauncherEnabledInTabletMode())
-    GetController()->DismissView();
 }
 
 void ArcAppItem::ExecuteLaunchCommand(int event_flags) {
-  Activate(event_flags);
+  PerformActivate(event_flags);
 }
 
 void ArcAppItem::SetName(const std::string& name) {

chrome/browser/ui/app_list/arc/arc_app_unittest.cc

@@ -912,9 +912,9 @@ TEST_P(ArcAppModelBuilderTest, LaunchApps) {
   ArcAppItem* item_last = FindArcItem(ArcAppTest::GetAppId(app_last));
   ASSERT_NE(nullptr, item_first);
   ASSERT_NE(nullptr, item_last);
-  item_first->Activate(0);
-  item_last->Activate(0);
-  item_first->Activate(0);
+  item_first->PerformActivate(0);
+  item_last->PerformActivate(0);
+  item_first->PerformActivate(0);
 
   const std::vector<std::unique_ptr<arc::FakeAppInstance::Request>>&
       launch_requests = app_instance()->launch_requests();
@@ -928,7 +928,7 @@ TEST_P(ArcAppModelBuilderTest, LaunchApps) {
   item_first = FindArcItem(ArcAppTest::GetAppId(app_first));
   ASSERT_NE(nullptr, item_first);
   size_t launch_request_count_before = app_instance()->launch_requests().size();
-  item_first->Activate(0);
+  item_first->PerformActivate(0);
   // Number of launch requests must not change.
   EXPECT_EQ(launch_request_count_before,
             app_instance()->launch_requests().size());
@@ -949,9 +949,9 @@ TEST_P(ArcAppModelBuilderTest, LaunchShortcuts) {
   ArcAppItem* item_last = FindArcItem(ArcAppTest::GetAppId(app_last));
   ASSERT_NE(nullptr, item_first);
   ASSERT_NE(nullptr, item_last);
-  item_first->Activate(0);
-  item_last->Activate(0);
-  item_first->Activate(0);
+  item_first->PerformActivate(0);
+  item_last->PerformActivate(0);
+  item_first->PerformActivate(0);
 
   const std::vector<std::string>& launch_intents =
       app_instance()->launch_intents();
@@ -965,7 +965,7 @@ TEST_P(ArcAppModelBuilderTest, LaunchShortcuts) {
   item_first = FindArcItem(ArcAppTest::GetAppId(app_first));
   ASSERT_NE(nullptr, item_first);
   size_t launch_request_count_before = app_instance()->launch_intents().size();
-  item_first->Activate(0);
+  item_first->PerformActivate(0);
   // Number of launch requests must not change.
   EXPECT_EQ(launch_request_count_before,
             app_instance()->launch_intents().size());

chrome/browser/ui/app_list/chrome_app_list_item.cc

@@ -91,6 +91,15 @@ ash::mojom::AppListItemMetadataPtr ChromeAppListItem::CloneMetadata() const {
   return metadata_.Clone();
 }
 
+void ChromeAppListItem::PerformActivate(int event_flags) {
+  Activate(event_flags);
+
+  // Launching apps can take some time. It looks nicer to dismiss the app list.
+  // Do not close app list for home launcher.
+  if (!GetController()->IsHomeLauncherEnabledInTabletMode())
+    GetController()->DismissView();
+}
+
 void ChromeAppListItem::Activate(int event_flags) {}
 
 const char* ChromeAppListItem::GetItemType() const {

chrome/browser/ui/app_list/chrome_app_list_item.h

@@ -84,6 +84,9 @@ class ChromeAppListItem {
   void SetChromeName(const std::string& name);
   void SetChromePosition(const syncer::StringOrdinal& position);
 
+  // Call |Activate()| and dismiss launcher if necessary.
+  void PerformActivate(int event_flags);
+
   // Activates (opens) the item. Does nothing by default.
   virtual void Activate(int event_flags);
 

chrome/browser/ui/app_list/chrome_app_list_model_updater.cc

@@ -9,6 +9,7 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/app_list/app_list_client_impl.h"
+#include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
 #include "chrome/browser/ui/app_list/chrome_app_list_item.h"
 #include "chrome/browser/ui/app_list/search/chrome_search_result.h"
 #include "chrome/browser/ui/ash/ash_util.h"
@@ -151,7 +152,7 @@ void ChromeAppListModelUpdater::ActivateChromeItem(const std::string& id,
   if (!item)
     return;
   DCHECK(!item->is_folder());
-  item->Activate(event_flags);
+  item->PerformActivate(event_flags);
 }
 
 ////////////////////////////////////////////////////////////////////////////////

chrome/browser/ui/app_list/crostini/crostini_app_item.cc

@@ -50,12 +50,6 @@ void CrostiniAppItem::Activate(int event_flags) {
   ChromeLauncherController::instance()->ActivateApp(
       id(), ash::LAUNCH_FROM_APP_LIST, event_flags,
       GetController()->GetAppListDisplayId());
-
-  // Launching Crostini apps can take a few seconds if the container is not
-  // currently running. We show a spinner in the shelf but it looks nicer to
-  // also dismiss the app list.
-  if (!GetController()->IsHomeLauncherEnabledInTabletMode())
-    GetController()->DismissView();
 }
 
 void CrostiniAppItem::GetContextMenuModel(GetMenuModelCallback callback) {

chrome/browser/ui/app_list/search/arc_app_result.cc

@@ -54,11 +54,6 @@ void ArcAppResult::Open(int event_flags) {
                       controller()->GetAppListDisplayId())) {
     return;
   }
-
-  // Manually close app_list view because focus is not changed on ARC app start,
-  // and current view remains active. Do not close app list for home launcher.
-  if (!controller()->IsHomeLauncherEnabledInTabletMode())
-    controller()->DismissView();
 }
 
 void ArcAppResult::GetContextMenuModel(GetMenuModelCallback callback) {

chrome/browser/ui/app_list/search/crostini_app_result.cc

@@ -34,11 +34,6 @@ void CrostiniAppResult::Open(int event_flags) {
   ChromeLauncherController::instance()->ActivateApp(
       id(), ash::LAUNCH_FROM_APP_LIST_SEARCH, event_flags,
       controller()->GetAppListDisplayId());
-
-  // Manually dismiss the app list as it can take several seconds for apps to
-  // launch.
-  if (!controller()->IsHomeLauncherEnabledInTabletMode())
-    controller()->DismissView();
 }
 
 void CrostiniAppResult::GetContextMenuModel(GetMenuModelCallback callback) {

chrome/browser/ui/app_list/search/search_controller.cc

@@ -13,14 +13,17 @@
 #include "base/bind.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
 #include "chrome/browser/ui/app_list/app_list_model_updater.h"
 #include "chrome/browser/ui/app_list/search/chrome_search_result.h"
 #include "chrome/browser/ui/app_list/search/search_provider.h"
 
 namespace app_list {
 
-SearchController::SearchController(AppListModelUpdater* model_updater)
-    : mixer_(std::make_unique<Mixer>(model_updater)) {}
+SearchController::SearchController(AppListModelUpdater* model_updater,
+                                   AppListControllerDelegate* list_controller)
+    : mixer_(std::make_unique<Mixer>(model_updater)),
+      list_controller_(list_controller) {}
 
 SearchController::~SearchController() {}
 
@@ -42,6 +45,11 @@ void SearchController::OpenResult(ChromeSearchResult* result, int event_flags) {
     return;
 
   result->Open(event_flags);
+
+  // Launching apps can take some time. It looks nicer to dismiss the app list.
+  // Do not close app list for home launcher.
+  if (!list_controller_->IsHomeLauncherEnabledInTabletMode())
+    list_controller_->DismissView();
 }
 
 void SearchController::InvokeResultAction(ChromeSearchResult* result,

chrome/browser/ui/app_list/search/search_controller.h

@@ -15,6 +15,7 @@
 #include "base/strings/string16.h"
 #include "chrome/browser/ui/app_list/search/mixer.h"
 
+class AppListControllerDelegate;
 class AppListModelUpdater;
 class ChromeSearchResult;
 
@@ -27,7 +28,8 @@ class SearchProvider;
 // results to the given SearchResults UI model.
 class SearchController {
  public:
-  explicit SearchController(AppListModelUpdater* model_updater);
+  SearchController(AppListModelUpdater* model_updater,
+                   AppListControllerDelegate* list_controller);
   virtual ~SearchController();
 
   void Start(const base::string16& query);
@@ -58,6 +60,7 @@ class SearchController {
   using Providers = std::vector<std::unique_ptr<SearchProvider>>;
   Providers providers_;
   std::unique_ptr<Mixer> mixer_;
+  AppListControllerDelegate* list_controller_;
 
   DISALLOW_COPY_AND_ASSIGN(SearchController);
 };

chrome/browser/ui/app_list/search/search_controller_factory.cc

@@ -61,7 +61,7 @@ std::unique_ptr<SearchController> CreateSearchController(
     AppListModelUpdater* model_updater,
     AppListControllerDelegate* list_controller) {
   std::unique_ptr<SearchController> controller =
-      std::make_unique<SearchController>(model_updater);
+      std::make_unique<SearchController>(model_updater, list_controller);
 
   // Add mixer groups. There are four main groups: answer card, apps, webstore
   // and omnibox. Each group has a "soft" maximum number of results. However, if