Merge 250874 "Fixing M33 and M34 stable blocker crash with Inser..."

> Fixing M33 and M34 stable blocker crash with InsertAppLauncherItem
> 
> The problem appears to be that the function GetLastActiveWebContents returns a WebContents pointer to some content which got already deleted. (See comment from CL).
> 
> Note that this is only used in InsertAppLauncherItem to find the activity state. Note that this indicates that we could even get rid of that map and use app_id_to_web_contents to find if a tab is currently active. But again - keeping the CL small for the backport.
> 
> This seems to be the most secure and isolated for for M33 and 34 - I will create an issue to follow up on this the proper way.
> 
> BUG=341250
> TEST=none (no test case known)
> 
> Review URL: https://codereview.chromium.org/160803002

TBR=skuhne@chromium.org

Review URL: https://codereview.chromium.org/164343002

git-svn-id: svn://svn.chromium.org/chrome/branches/1750/src@251097 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 24 insertions, 3 deletions

diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
index 93eeb16..149b4b2 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
@@ -1696,12 +1696,33 @@ void ChromeLauncherController::SetShelfBehaviorsFromPrefs() {
 
 WebContents* ChromeLauncherController::GetLastActiveWebContents(
     const std::string& app_id) {
-  AppIDToWebContentsListMap::const_iterator i =
+  AppIDToWebContentsListMap::iterator i =
       app_id_to_web_contents_list_.find(app_id);
   if (i == app_id_to_web_contents_list_.end())
     return NULL;
-  DCHECK_GT(i->second.size(), 0u);
-  return *i->second.begin();
+
+  // There are many crash records (crbug.com/341250) which indicate that the
+  // app_id_to_web_contents_list_ contains deleted content entries - so there
+  // must be a way that the content does not get properly updated. To fix
+  // M33 and M34 we filter out the invalid items here, but this should be
+  // addressed by a later patch correctly. Looking at the code however, the
+  // real culprit is possibly BrowserStatusMonitor::UpdateAppItemState which
+  // does not call "UpdateAppState(.., APP_STATE_REMOVED)" because due to a
+  // Browser::SwapTabContent operation it isn't able to get the browser. I
+  // think that the real patch is to call anyway when APP_STATE_REMOVED is
+  // requested, but for a backport that seems risky.
+  WebContentsList* list = &i->second;
+  while (!list->empty()) {
+    WebContents* contents = *list->begin();
+    if (chrome::FindBrowserWithWebContents(contents))
+      return contents;
+    list->erase(list->begin());
+    // This might not be necessary, but since we do not know why the lists
+    // diverged we also erase it since it cannot be correct either.
+    web_contents_to_app_id_.erase(contents);
+  }
+  app_id_to_web_contents_list_.erase(app_id);
+  return NULL;
 }
 
 ash::LauncherID ChromeLauncherController::InsertAppLauncherItem(