Fix rare crash in WebstoreInlineInstaller::CheckInlineInstallPermitted.

chrome/browser/extensions/webstore_inline_installer.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/extensions/webstore_inline_installer.h"
 
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
@@ skipping 12 matching lines @@
 const char kNotFromVerifiedSitesError[] =
     "Installs can only be initiated by one of the Chrome Web Store item's "
     "verified sites.";
 const char kInlineInstallSupportedError[] =
     "Inline installation is not supported for this item. The user will be "
     "redirected to the Chrome Web Store.";
 const char kInitiatedFromPopupError[] =
     "Inline installs can not be initiated from pop-up windows.";
 const char kInitiatedFromFullscreenError[] =
     "Inline installs can not be initiated from fullscreen.";
+const char kInternalError[] = "Unable to complete installation.";
 
 WebstoreInlineInstaller::WebstoreInlineInstaller(
     content::WebContents* web_contents,
     content::RenderFrameHost* host,
     const std::string& webstore_item_id,
     const GURL& requestor_url,
     const Callback& callback)
     : WebstoreStandaloneInstaller(
           webstore_item_id,
           Profile::FromBrowserContext(web_contents->GetBrowserContext()),
@@ skipping 86 matching lines @@
 }
 
 WebContents* WebstoreInlineInstaller::GetWebContents() const {
   return web_contents();
 }
 
 bool WebstoreInlineInstaller::CheckInlineInstallPermitted(
     const base::DictionaryValue& webstore_data,
     std::string* error) const {
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
-  DCHECK(browser);
+  if (!browser) {

[Devlin, 2017/01/31 20:18:47]

Hmm... interesting.  Do you know when this might happen?  Before calling
CheckInlineInstallPermitted(), we call CheckRequestorAlive() (which validates
web_contents()) and early-outs if it's not.  Is there another case in which
FindBrowserWithWebContents() could return null?  I wonder if we could catch this
case earlier, rather than waiting for the round-trip from the webstore?

[robertshield, 2017/02/01 14:21:32]

Yes, I believe in this case, web_contents() is not null, but the containing tab
has been stripped out of the tab_strip_model used by the TabContentsIterator
used by FindBrowserWithWebContents
(https://codesearch.chromium.org/chromium/src/chrome/browser/ui/tab_contents/t...).


I haven't dug very far into exactly when things get pruned out of the
tab_strip_model_, happens during window closing though and it can happen
slightly before the WebContents gets destroyed as per
https://codesearch.chromium.org/chromium/src/chrome/browser/ui/tabs/tab_strip...

Checking before the webstore response comes in is an interesting idea, but I'm
not sure that the WebContents still being attached to a Browser before the
request goes out necessarily guarantees it will still be attached to a Browser
when the response has come in.

[Devlin, 2017/02/06 16:28:41]

Hmm, okay.  What about merging this logic with CheckRequestorAlive()?  I think
for all intents and purposes, if the web contents is no longer attached to a
browser, we don't want to continue.  Then the check in
WebstoreStandaloneInstaller() for CheckRequestorAlive() would catch this case.

[robertshield, 2017/02/10 15:25:23]

That does seem better. Made it so. Apologies for the delay, I took a stab at a
browser_test but couldn't figure out a non-insanely-invasive way of writing one.


Let me know if this is ok as is. 

+    *error = kInternalError;
+    return false;
+  }
   if (browser->is_type_popup()) {
     *error = kInitiatedFromPopupError;
     return false;
   }
   FullscreenController* controller =
       browser->exclusive_access_manager()->fullscreen_controller();
   if (controller->IsFullscreenForBrowser() || controller->IsTabFullscreen()) {
     *error = kInitiatedFromFullscreenError;
     return false;
   }
@@ skipping 95 matching lines @@
     DLOG(WARNING) << "Could not parse " << verified_site_pattern_spec <<
         " as URL pattern " << parse_result;
     return false;
   }
   verified_site_pattern.SetScheme("*");
 
   return verified_site_pattern.MatchesURL(requestor_url);
 }
 
 }  // namespace extensions