shelf: Clicking shelf items will open windows on the display with click.

Currently works on browser windows due to ScopedRootForNewWindows due
to opening sync (or before AfterItemSelected callback is fired). V1 and
V2 apps are launched async though.

This moves deleting the scoped object until ShelfItemStatusChanged
which can be called by window activation change, which is async. It's
a little worrisome that we might not clear the changed root window so
added a delayed task to ensure deletion (when I tested window
activation always happened though).

Tried adding a test in ChromeLauncherControllerTest but it doesn't seem
like there was support for launching a app in a separate window, even
by launching manually and not clicking on the shelf shortcut.

Test: manual
Bug: 1080390
Change-Id: I0d5f6126571f92d86c5d4530d205b6cfc3e635be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2198550Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#768528}

ash/shelf/shelf_view.cc

@@ -672,10 +672,19 @@ void ShelfView::ButtonPressed(views::Button* sender,
   last_pressed_index_ = view_model_->GetIndexOfView(sender);
   DCHECK_LT(-1, last_pressed_index_);
 
-  // Place new windows on the same display as the button.
+  // Place new windows on the same display as the button. Opening windows is
+  // usually an async operation so we wait until window activation changes
+  // (ShelfItemStatusChanged) before destroying the scoped object. Post a task
+  // to destroy the scoped object just in case the window activation event does
+  // not get fired.
   aura::Window* window = sender->GetWidget()->GetNativeWindow();
   scoped_root_window_for_new_windows_ =
       std::make_unique<ScopedRootWindowForNewWindows>(window->GetRootWindow());
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(&ShelfView::DestroyScopedRootWindow,
+                     weak_factory_.GetWeakPtr()),
+      base::TimeDelta::FromMilliseconds(100));
 
   // Slow down activation animations if Control key is pressed.
   std::unique_ptr<ui::ScopedAnimationDurationScaleMode> slowing_animations;
@@ -1920,6 +1929,8 @@ void ShelfView::ShelfItemDelegateChanged(const ShelfID& id,
 }
 
 void ShelfView::ShelfItemStatusChanged(const ShelfID& id) {
+  scoped_root_window_for_new_windows_.reset();
+
   int index = model_->ItemIndexByID(id);
   if (index < 0)
     return;
@@ -2005,7 +2016,6 @@ void ShelfView::AfterItemSelected(const ShelfItem& item,
     }
   }
   shelf_->shelf_layout_manager()->OnShelfItemSelected(action);
-  scoped_root_window_for_new_windows_.reset();
 }
 
 void ShelfView::ShowShelfContextMenu(
@@ -2185,4 +2195,8 @@ void ShelfView::UpdateVisibleIndices() {
   last_visible_index_ = model_->item_count() - 1;
 }
 
+void ShelfView::DestroyScopedRootWindow() {
+  scoped_root_window_for_new_windows_.reset();
+}
+
 }  // namespace ash

ash/shelf/shelf_view.h

@@ -487,6 +487,8 @@ class ASH_EXPORT ShelfView : public views::AccessiblePaneView,
 
   bool ShouldHandleGestures(const ui::GestureEvent& event) const;
 
+  void DestroyScopedRootWindow();
+
   // Different from GetTitleForView, |view| here must be a child view.
   base::string16 GetTitleForChildView(const views::View* view) const;
 
@@ -542,6 +544,9 @@ class ASH_EXPORT ShelfView : public views::AccessiblePaneView,
 
   // Responsible for building and running all menus.
   std::unique_ptr<ShelfMenuModelAdapter> shelf_menu_model_adapter_;
+
+  // Created when a shelf icon is pressed, so that new windows will be on the
+  // same root window as the press event.
   std::unique_ptr<ScopedRootWindowForNewWindows>
       scoped_root_window_for_new_windows_;