arc: Fix crash on deferred app launch.

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.h"
 
 #include <stddef.h>
 
 #include "ash/wm/window_util.h"
 #include "base/memory/ptr_util.h"
@@ skipping 170 matching lines @@
     Browser* browser = chrome::FindBrowserWithWebContents(content[i]);
     if (!browser || !IsBrowserFromActiveUser(browser))
       continue;
     TabStripModel* tab_strip = browser->tab_strip_model();
     int index = tab_strip->GetIndexOfWebContents(content[i]);
     DCHECK(index != TabStripModel::kNoTab);
     tab_strip->CloseWebContentsAt(index, TabStripModel::CLOSE_NONE);
   }
 }
 
+bool AppShortcutLauncherItemController::IsShortcut() const {
+  return true;

[msw, 2017/03/14 01:24:59]

What does it mean if LauncherItemController::IsShortcut() != (ShelfItem::type ==
TYPE_APP_SHORTCUT)? Instead, could this launcher item controller subclass ensure
that the associated shelf item has type ash::TYPE_APP_SHORTCUT (perhaps by
setting the item type to that value in the ctor)? Or is the issue that
non-"IsShortcut" items were marked as TYPE_APP_SHORTCUT? Either way, we should
avoid ShelfItemDelegate/LauncherItemController expansion going forward.

[khmel, 2017/03/14 15:25:44]

Hi,
I don't clear understand your idea. The reason of this crash is impossibility to
determine if shelf item represents running app or just a shortcut. With recent
refactoring ash::TYPE_APP_SHORTCUT represents and shortcut and running app. When
app is not running it has type ash::TYPE_APP_SHORTCUT. When app is running its
shelf item still has type ash::TYPE_APP_SHORTCUT. Before in this scenario it was
changed from ash::TYPE_APP_SHORTCUT to ash::TYPE_APP. Do you suggest to restore
functionality by passing type to base class as it was here:
https://codereview.chromium.org/2545923004/diff/80001/chrome/browser/ui/ash/l...
?

[msw, 2017/03/14 17:19:32]

My idea is to update and check ShelfItem struct values, instead of calling
controller instance functions. So, something should be updating the ShelfItem's
|type| and/or |status| when the item changes from a shortcut to a running app.
There are two possibilities:
1) Running apps should have ShelfItem::type == ash::TYPE_APP, but shortcuts
(pinned items that are not running) should have ShelfItem::type ==
ash::TYPE_APP_SHORTCUT. See ShelfItem::type here:
https://cs.chromium.org/chromium/src/ash/common/shelf/shelf_item_types.h?l=25
2) Even better, we disregard ShelfItem::type (both should eventually just be
app...) and we check the ShelfItem::status value (either STATUS_CLOSED or not).
See ShelfItem::status here:
https://cs.chromium.org/chromium/src/ash/common/shelf/shelf_item_types.h?l=34
I hope that's clearer. What do you think?

[khmel, 2017/03/14 20:19:56]

Done as per offline discussion.

+}
+
 std::vector<content::WebContents*>
 AppShortcutLauncherItemController::GetRunningApplications() {
   std::vector<content::WebContents*> items;
 
   URLPattern refocus_pattern(URLPattern::SCHEME_ALL);
   refocus_pattern.SetMatchAllURLs(true);
 
   if (!refocus_url_.is_empty()) {
     refocus_pattern.SetMatchAllURLs(false);
     refocus_pattern.Parse(refocus_url_.spec());
@@ skipping 158 matching lines @@
 
 bool AppShortcutLauncherItemController::AllowNextLaunchAttempt() {
   if (last_launch_attempt_.is_null() ||
       last_launch_attempt_ + base::TimeDelta::FromMilliseconds(
           kClickSuppressionInMS) < base::Time::Now()) {
     last_launch_attempt_ = base::Time::Now();
     return true;
   }
   return false;
 }