arc: Fix crash activating Play Store in deferred mode.

chrome/browser/ui/ash/launcher/arc_playstore_shortcut_launcher_item_controller.cc

Index: chrome/browser/ui/ash/launcher/arc_playstore_shortcut_launcher_item_controller.cc
diff --git a/chrome/browser/ui/ash/launcher/arc_playstore_shortcut_launcher_item_controller.cc b/chrome/browser/ui/ash/launcher/arc_playstore_shortcut_launcher_item_controller.cc
index da6cfe61c47154852cfcc458e0652d8a617e7b53..7c2d4ee09e5efabb32faafef8aa2f47dfcac7752 100644
--- a/chrome/browser/ui/ash/launcher/arc_playstore_shortcut_launcher_item_controller.cc
+++ b/chrome/browser/ui/ash/launcher/arc_playstore_shortcut_launcher_item_controller.cc
@@ -27,8 +27,14 @@ void ArcPlaystoreShortcutLauncherItemController::ItemSelected(
     int64_t display_id,
     ash::ShelfLaunchSource source,
     const ItemSelectedCallback& callback) {
-  playstore_launcher_ = base::MakeUnique<ArcAppLauncher>(
-      controller()->profile(), arc::kPlayStoreAppId, true, true);
-  callback.Run(ash::SHELF_ACTION_NONE,
-               GetAppMenuItems(event ? event->flags() : ui::EF_NONE));
+  // Note, ArcAppLauncher may invoke arc::LaunchApp that in some cases creates

[msw, 2017/04/04 01:42:05]

I don't quite understand this comment or why |playstore_launcher_| needs a
conditional assignment below... Can you explain the defect and fix a bit more?

[khmel, 2017/04/04 16:52:26]

Problem following. This is shortcut controller. User activates it. As result we
start an app. In our case this is Play Store app. Usually application starts
asynchronously. Once app started we remove (it gets deleted) shortcut controller
and replace it with app controller.
In case of deferred launch we have different timing. ArcAppLauncher is a helper
that waits until ARC app appears and launch it.
In case leading to crash, ARC app (Play Store) exists but not ready yet. That
means we are using deferred launch (spinning icon in shelf). Check for app
exists is made in ArcAppLauncher controller. That means this instance of
ArcPlaystoreShortcutLauncherItemController is deleted on exiting from 
ArcAppLauncher CTOR. That means access to playstore_launcher_ and 
GetAppMenuItems is invalid once this is deleled.

rephrased comment

+  // deferred launch item which substitutes current controller. It is not safe

[msw, 2017/04/04 01:42:05]

what is the "current controller"?

[khmel, 2017/04/04 16:52:26]

Explained above

+  // to access this object if Play Store was actually launched.

[msw, 2017/04/04 01:42:05]

by "this object", do you mean this ArcPlaystoreShortcutLauncherItemController
instance?

[khmel, 2017/04/04 16:52:26]

yes

+  DCHECK(GetAppMenuItems(event ? event->flags() : ui::EF_NONE).empty());
+  std::unique_ptr<ArcAppLauncher> playstore_launcher =
+      base::MakeUnique<ArcAppLauncher>(controller()->profile(),
+                                       arc::kPlayStoreAppId, true, true);
+  if (!playstore_launcher->app_launched())
+    playstore_launcher_ = std::move(playstore_launcher);
+  callback.Run(ash::SHELF_ACTION_NONE, MenuItemList());
 }