Prevent a possible crash on omnibox navigation.

chrome/browser/ui/omnibox/omnibox_navigation_observer.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/omnibox/omnibox_navigation_observer.h"
 
 #include "chrome/browser/autocomplete/shortcuts_backend.h"
 #include "chrome/browser/autocomplete/shortcuts_backend_factory.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/intranet_redirect_detector.h"
@@ skipping 68 matching lines @@
 void OmniboxNavigationObserver::OnSuccessfulNavigation() {
   if (shortcuts_backend_.get())
     shortcuts_backend_->AddOrUpdateShortcut(text_, match_);
 }
 
 void OmniboxNavigationObserver::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   DCHECK_EQ(content::NOTIFICATION_NAV_ENTRY_PENDING, type);
+
+  // It's possible for an attempted omnibox navigation to cause the extensions
+  // system to synchronously navigate an extension background page.  Not only is
+  // this navigation not the one we want to observe, the associated WebContents
+  // is invisible and has no InfoBarService, so trying to show an infobar in it
+  // later will crash.  Just ignore this navigation and keep listening.
+  content::NavigationController* controller =
+      content::Source<content::NavigationController>(source).ptr();
+  content::WebContents* web_contents = controller->GetWebContents();
+  if (!InfoBarService::FromWebContents(web_contents))

[Vitaly Buka (NO REVIEWS), 2015/01/22 21:59:09]

Using InfoBarservice for unwanted web-contents looks confusing.
It would be nicer to have more direct check.

Maybe opposite?

if (match_.destination_url != ...) {
...
}
and CHECK for InfoBarService...

[Peter Kasting, 2015/01/22 22:02:26]

There's no theoretical reason why an extension background page couldn't load the
same page the user is trying to load, so there's a chance (although it seems
very unlikely) that doing things that way would break.

+    return;
+
+  CHECK_EQ(match_.destination_url,
+           content::Details<content::NavigationEntry>(details)->GetURL());
   registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
                     content::NotificationService::AllSources());
-  content::NavigationController* controller =
-      content::Source<content::NavigationController>(source).ptr();
   if (fetcher_) {
     fetcher_->SetRequestContext(
         controller->GetBrowserContext()->GetRequestContext());
   }
-  WebContentsObserver::Observe(controller->GetWebContents());
+  WebContentsObserver::Observe(web_contents);
   // DidStartNavigationToPendingEntry() will be called for this load as well.
 }
 
 void OmniboxNavigationObserver::NavigationEntryCommitted(
     const content::LoadCommittedDetails& load_details) {
   load_state_ = LOAD_COMMITTED;
   if (ResponseCodeIndicatesSuccess(load_details.http_status_code) &&
       IsValidNavigation(match_.destination_url, load_details.entry->GetURL()))
     OnSuccessfulNavigation();
   if (!fetcher_ || (fetch_state_ != FETCH_NOT_COMPLETE))
@@ skipping 32 matching lines @@
     OnAllLoadingFinished();  // deletes |this|!
 }
 
 void OmniboxNavigationObserver::OnAllLoadingFinished() {
   if (fetch_state_ == FETCH_SUCCEEDED) {
     AlternateNavInfoBarDelegate::Create(
         web_contents(), text_, alternate_nav_match_, match_.destination_url);
   }
   delete this;
 }