Fix crash when clicking home launcher shelf item.

Bug: 891579
Change-Id: I0151617076021d7ed0923853302faa733782b662
Reviewed-on: https://chromium-review.googlesource.com/c/1258610Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596354}

ash/app_list/app_list_controller_impl.cc

@@ -33,16 +33,6 @@
 #include "ui/base/ui_base_features.h"
 #include "ui/display/screen.h"
 
-namespace {
-
-int64_t GetDisplayIdToShowAppListOn() {
-  return display::Screen::GetScreen()
-      ->GetDisplayNearestWindow(ash::Shell::GetRootWindowForNewWindows())
-      .id();
-}
-
-}  // namespace
-
 namespace ash {
 
 AppListControllerImpl::AppListControllerImpl(ws::WindowService* window_service)
@@ -387,15 +377,14 @@ void AppListControllerImpl::OnAppListItemAdded(app_list::AppListItem* item) {
 
 void AppListControllerImpl::OnActiveUserPrefServiceChanged(
     PrefService* /* pref_service */) {
-  if (!IsHomeLauncherEnabledInTabletMode() ||
-      !display::Display::HasInternalDisplay()) {
+  if (!IsHomeLauncherEnabledInTabletMode()) {
     DismissAppList();
     return;
   }
 
   // Show the app list after signing in in tablet mode.
-  Show(display::Display::InternalDisplayId(),
-       app_list::AppListShowSource::kTabletMode, base::TimeTicks());
+  Show(GetDisplayIdToShowAppListOn(), app_list::AppListShowSource::kTabletMode,
+       base::TimeTicks());
 
   // The app list is not dismissed before switching user, suggestion chips will
   // not be shown. So reset app list state and trigger an initial search here to
@@ -536,7 +525,7 @@ void AppListControllerImpl::OnTabletModeStarted() {
     presenter_.GetView()->OnTabletModeChanged(true);
   }
 
-  if (!is_home_launcher_enabled_ || !display::Display::HasInternalDisplay())
+  if (!is_home_launcher_enabled_)
     return;
 
   SessionController const* session_controller =
@@ -545,8 +534,7 @@ void AppListControllerImpl::OnTabletModeStarted() {
     return;
 
   // Show the app list if the tablet mode starts.
-  Show(display::Display::InternalDisplayId(), app_list::kTabletMode,
-       base::TimeTicks());
+  Show(GetDisplayIdToShowAppListOn(), app_list::kTabletMode, base::TimeTicks());
   UpdateHomeLauncherVisibility();
   Shelf::ForWindow(presenter_.GetWindow())->MaybeUpdateShelfBackground();
 }
@@ -865,4 +853,16 @@ void AppListControllerImpl::UpdateAssistantVisibility() {
       controller->allowed_state() == mojom::AssistantAllowedState::ALLOWED);
 }
 
+int64_t AppListControllerImpl::GetDisplayIdToShowAppListOn() {
+  if (IsHomeLauncherEnabledInTabletMode()) {
+    return display::Display::HasInternalDisplay()
+               ? display::Display::InternalDisplayId()
+               : display::Screen::GetScreen()->GetPrimaryDisplay().id();
+  }
+
+  return display::Screen::GetScreen()
+      ->GetDisplayNearestWindow(ash::Shell::GetRootWindowForNewWindows())
+      .id();
+}
+
 }  // namespace ash

ash/app_list/app_list_controller_impl.h

@@ -235,6 +235,8 @@ class ASH_EXPORT AppListControllerImpl
   // Update the visibility of Assistant functionality.
   void UpdateAssistantVisibility();
 
+  int64_t GetDisplayIdToShowAppListOn();
+
   ws::WindowService* window_service_;
 
   base::string16 last_raw_query_;

ash/app_list/app_list_presenter_delegate_unittest.cc

@@ -1124,9 +1124,6 @@ class AppListPresenterDelegateHomeLauncherTest
          app_list_features::kEnableBackgroundBlur},
         {});
     AppListPresenterDelegateTest::SetUp();
-    // Home launcher is only enabled on internal display.
-    display::test::DisplayManagerTestApi(Shell::Get()->display_manager())
-        .SetFirstDisplayAsInternalDisplay();
     GetAppListTestHelper()->WaitUntilIdle();
   }
 

ash/app_list/presenter/app_list_presenter_impl.cc

@@ -136,6 +136,7 @@ void AppListPresenterImpl::Show(int64_t display_id,
     delegate_->Init(view, display_id, current_apps_page_);
     SetView(view);
   }
+  view_->ShowWhenReady();
   delegate_->OnShown(display_id);
   NotifyTargetVisibilityChanged(GetTargetVisibility());
   NotifyVisibilityChanged(GetTargetVisibility(), display_id);
@@ -279,7 +280,6 @@ void AppListPresenterImpl::SetView(AppListView* view) {
   // Sync the |onscreen_keyboard_shown_| in case |view_| is not initiated when
   // the on-screen is shown.
   view_->set_onscreen_keyboard_shown(delegate_->GetOnScreenKeyboardShown());
-  view_->ShowWhenReady();
 }
 
 void AppListPresenterImpl::ResetView() {

ash/shelf/app_list_shelf_item_delegate.cc

@@ -75,6 +75,8 @@ void AppListShelfItemDelegate::ItemSelected(std::unique_ptr<ui::Event> event,
 
   if (back_action)
     Shell::Get()->app_list_controller()->Back();
+
+  std::move(callback).Run(SHELF_ACTION_APP_LIST_SHOWN, base::nullopt);
 }
 
 void AppListShelfItemDelegate::ExecuteCommand(bool from_context_menu,

ash/shelf/shelf_view.cc

@@ -598,9 +598,9 @@ void ShelfView::ButtonPressed(views::Button* sender,
   // Notify the item of its selection; handle the result in AfterItemSelected.
   model_->GetShelfItemDelegate(item.id)->ItemSelected(
       ui::Event::Clone(event), GetDisplayIdForView(this), LAUNCH_FROM_UNKNOWN,
-      base::Bind(&ShelfView::AfterItemSelected, weak_factory_.GetWeakPtr(),
-                 item, sender, base::Passed(ui::Event::Clone(event)),
-                 ink_drop));
+      base::BindOnce(&ShelfView::AfterItemSelected, weak_factory_.GetWeakPtr(),
+                     item, sender, base::Passed(ui::Event::Clone(event)),
+                     ink_drop));
 }
 
 ////////////////////////////////////////////////////////////////////////////////

ash/wm/splitview/split_view_controller_unittest.cc

@@ -2814,6 +2814,9 @@ TEST_F(SplitViewTabDraggingTest, SourceWindowBackgroundTest) {
   EXPECT_TRUE(window3->IsVisible());
   EXPECT_TRUE(window4->IsVisible());
 
+  if (Shell::Get()->app_list_controller()->IsHomeLauncherEnabledInTabletMode())
+    EXPECT_TRUE(Shell::Get()->app_list_controller()->IsVisible());
+
   // 1) Start dragging |window1|. |window2| is the source window.
   std::unique_ptr<WindowResizer> resizer =
       StartDrag(window1.get(), window2.get());