Fix crash due to assuming that if we received an INSTANT_COMMITTED notification, we must actually care about that navigation committing.

This also cleans up notification listening in general; the conditions under which I added and removed the global listeners for NAVIGATION_PENDING and INSTANT_COMMITTED were incorrect.  Unit tests have been added covering both these issues.

BUG=142935
TEST=none
Review URL: https://chromiumcodereview.appspot.com/10911002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@154207 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 225 insertions, 107 deletions

diff --git a/chrome/browser/google/google_url_tracker.cc b/chrome/browser/google/google_url_tracker.cc
index baa8365..95f57e3 100644
--- a/chrome/browser/google/google_url_tracker.cc
+++ b/chrome/browser/google/google_url_tracker.cc
@@ -387,12 +387,10 @@ void GoogleURLTracker::Observe(int type,
     case chrome::NOTIFICATION_INSTANT_COMMITTED: {
       TabContents* tab_contents = content::Source<TabContents>(source).ptr();
       content::WebContents* web_contents = tab_contents->web_contents();
-      content::Source<content::NavigationController> source(
-          &web_contents->GetController());
-      InfoBarTabHelper* infobar_helper = tab_contents->infobar_tab_helper();
-      OnNavigationPending(source, content::Source<TabContents>(tab_contents),
-                          infobar_helper, 0);
-      OnNavigationCommittedOrTabClosed(infobar_helper, web_contents->GetURL());
+      OnInstantCommitted(
+          content::Source<content::NavigationController>(
+              &web_contents->GetController()),
+          source, tab_contents->infobar_tab_helper(), web_contents->GetURL());
       break;
     }
 
@@ -436,32 +434,11 @@ void GoogleURLTracker::CancelGoogleURL(const GURL& new_google_url) {
 }
 
 void GoogleURLTracker::InfoBarClosed(const InfoBarTabHelper* infobar_helper) {
-  DCHECK(!search_committed_);
   InfoBarMap::iterator i(infobar_map_.find(infobar_helper));
   DCHECK(i != infobar_map_.end());
-  const MapEntry& map_entry = i->second;
 
-  UnregisterForEntrySpecificNotifications(map_entry, false);
+  UnregisterForEntrySpecificNotifications(i->second, false);
   infobar_map_.erase(i);
-
-  // Our global listeners for these other notifications should be in place iff
-  // we have any non-showing infobars.
-  for (InfoBarMap::const_iterator i(infobar_map_.begin());
-       i != infobar_map_.end(); ++i) {
-    if (!i->second.infobar->showing()) {
-      DCHECK(registrar_.IsRegistered(this,
-          content::NOTIFICATION_NAV_ENTRY_PENDING,
-          content::NotificationService::AllBrowserContextsAndSources()));
-      return;
-    }
-  }
-  if (registrar_.IsRegistered(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
-      content::NotificationService::AllBrowserContextsAndSources())) {
-    registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
-        content::NotificationService::AllBrowserContextsAndSources());
-    registrar_.Remove(this, chrome::NOTIFICATION_INSTANT_COMMITTED,
-        content::NotificationService::AllBrowserContextsAndSources());
-  }
 }
 
 void GoogleURLTracker::SetNeedToFetch() {
@@ -605,6 +582,31 @@ void GoogleURLTracker::OnNavigationCommittedOrTabClosed(
   map_entry.infobar->Show(search_url);
 }
 
+void GoogleURLTracker::OnInstantCommitted(
+    const content::NotificationSource& navigation_controller_source,
+    const content::NotificationSource& tab_contents_source,
+    InfoBarTabHelper* infobar_helper,
+    const GURL& search_url) {
+  // If this was the search we were listening for, OnNavigationPending() should
+  // ensure we're registered for NAV_ENTRY_COMMITTED, and we should call
+  // OnNavigationCommittedOrTabClosed() to simulate that firing.  Otherwise,
+  // this is some sort of non-search navigation, so while we should still call
+  // OnNavigationPending(), that function should then ensure that we're not
+  // listening for NAV_ENTRY_COMMITTED on this tab, and we should not call
+  // OnNavigationCommittedOrTabClosed() afterwards.  Note that we need to save
+  // off |search_committed_| here because OnNavigationPending() will reset it.
+  bool was_search_committed = search_committed_;
+  OnNavigationPending(navigation_controller_source, tab_contents_source,
+                      infobar_helper, 0);
+  InfoBarMap::iterator i(infobar_map_.find(infobar_helper));
+  DCHECK_EQ(was_search_committed, (i != infobar_map_.end()) &&
+      registrar_.IsRegistered(this,
+          content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+          i->second.navigation_controller_source));
+  if (was_search_committed)
+    OnNavigationCommittedOrTabClosed(infobar_helper, search_url);
+}
+
 void GoogleURLTracker::CloseAllInfoBars(bool redo_searches) {
   // Close all infobars, whether they've been added to tabs or not.
   while (!infobar_map_.empty())
@@ -635,4 +637,28 @@ void GoogleURLTracker::UnregisterForEntrySpecificNotifications(
     registrar_.Remove(this, chrome::NOTIFICATION_TAB_CONTENTS_DESTROYED,
                       map_entry.tab_contents_source);
   }
+
+  // Our global listeners for these other notifications should be in place iff
+  // we have any infobars still listening for commits.  These infobars are
+  // either not yet shown or have received a new pending search atop an existing
+  // infobar; in either case we want to catch subsequent pending non-search
+  // navigations. See the various cases inside OnNavigationPending().
+  for (InfoBarMap::const_iterator i(infobar_map_.begin());
+       i != infobar_map_.end(); ++i) {
+    if (registrar_.IsRegistered(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+                                i->second.navigation_controller_source)) {
+      DCHECK(registrar_.IsRegistered(this,
+          content::NOTIFICATION_NAV_ENTRY_PENDING,
+          content::NotificationService::AllBrowserContextsAndSources()));
+      return;
+    }
+  }
+  if (registrar_.IsRegistered(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
+      content::NotificationService::AllBrowserContextsAndSources())) {
+    DCHECK(!search_committed_);
+    registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
+        content::NotificationService::AllBrowserContextsAndSources());
+    registrar_.Remove(this, chrome::NOTIFICATION_INSTANT_COMMITTED,
+        content::NotificationService::AllBrowserContextsAndSources());
+  }
 }
diff --git a/chrome/browser/google/google_url_tracker.h b/chrome/browser/google/google_url_tracker.h
index 2035bc1..0b21351 100644
--- a/chrome/browser/google/google_url_tracker.h
+++ b/chrome/browser/google/google_url_tracker.h
@@ -157,12 +157,11 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
   // notification.  |navigation_controller_source| and |tab_contents_source| are
   // NotificationSources pointing to the associated NavigationController and
   // TabContents, respectively, for this load; |infobar_helper| is the
-  // InfoBarTabHelper of the associated tab; and |search_url| is the actual
-  // search performed by the user, which if necessary we'll re-do on a new
-  // domain later.  If there is already a visible GoogleURLTracker infobar for
-  // this tab, this function resets its associated navigation entry to point at
-  // the new pending entry.  Otherwise this function creates a (still-invisible)
-  // InfoBarDelegate for the associated tab.
+  // InfoBarTabHelper of the associated tab; and |pending_id| is the unique ID
+  // of the newly pending NavigationEntry.  If there is already a visible
+  // GoogleURLTracker infobar for this tab, this function resets its associated
+  // pending entry ID to the new ID.  Otherwise this function creates a
+  // (still-invisible) InfoBarDelegate for the associated tab.
   void OnNavigationPending(
       const content::NotificationSource& navigation_controller_source,
       const content::NotificationSource& tab_contents_source,
@@ -177,14 +176,26 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
   void OnNavigationCommittedOrTabClosed(const InfoBarTabHelper* infobar_helper,
                                         const GURL& search_url);
 
+  // Called by Observe() when an instant navigation occurs.  This will call
+  // OnNavigationPending(), and, depending on whether this is a search we were
+  // listening for, may then also call OnNavigationCommittedOrTabClosed().
+  void OnInstantCommitted(
+    const content::NotificationSource& navigation_controller_source,
+    const content::NotificationSource& tab_contents_source,
+    InfoBarTabHelper* infobar_helper,
+    const GURL& search_url);
+
   // Closes all open infobars.  If |redo_searches| is true, this also triggers
   // each tab to re-perform the user's search, but on the new Google TLD.
   void CloseAllInfoBars(bool redo_searches);
 
   // Unregisters any listeners for the notification sources in |map_entry|.
-  // This also sanity-DCHECKs that these are registered (or not) in the specific
+  // This sanity-DCHECKs that these are registered (or not) in the specific
   // cases we expect.  (|must_be_listening_for_commit| is used purely for this
-  // sanity-checking.)
+  // sanity-checking.)  This also unregisters our global NAV_ENTRY_PENDING/
+  // INSTANT_COMMITTED listeners if there are no remaining listeners for
+  // NAV_ENTRY_COMMITTED, as we no longer need them until another search is
+  // committed.
   void UnregisterForEntrySpecificNotifications(
       const MapEntry& map_entry,
       bool must_be_listening_for_commit);
diff --git a/chrome/browser/google/google_url_tracker_unittest.cc b/chrome/browser/google/google_url_tracker_unittest.cc
index 10893d7..742ba25 100644
--- a/chrome/browser/google/google_url_tracker_unittest.cc
+++ b/chrome/browser/google/google_url_tracker_unittest.cc
@@ -133,10 +133,10 @@ class GoogleURLTrackerTest : public testing::Test {
   GURL google_url() const { return google_url_tracker_->google_url_; }
   void SetLastPromptedGoogleURL(const GURL& url);
   GURL GetLastPromptedGoogleURL();
-  void SetNonSearchPending(int unique_id);
-  void SetSearchPending(int unique_id);
+  void SetNavigationPending(int unique_id, bool is_search);
   void CommitNonSearch(int unique_id);
-  void CommitSearch(const GURL& search_url, int unique_id);
+  void CommitSearch(int unique_id, const GURL& search_url);
+  void DoInstantNavigation(int unique_id, const GURL& search_url);
   void CloseTab(int unique_id);
   TestInfoBarDelegate* GetInfoBar(int unique_id);
   void ExpectDefaultURLs() const;
@@ -234,7 +234,13 @@ GURL GoogleURLTrackerTest::GetLastPromptedGoogleURL() {
   return GURL(profile_.GetPrefs()->GetString(prefs::kLastPromptedGoogleURL));
 }
 
-void GoogleURLTrackerTest::SetNonSearchPending(int unique_id) {
+void GoogleURLTrackerTest::SetNavigationPending(int unique_id, bool is_search) {
+  if (is_search) {
+    google_url_tracker_->SearchCommitted();
+    // Note that the call above might not have actually registered a listener
+    // for NOTIFICATION_NAV_ENTRY_PENDING if the searchdomaincheck response was
+    // bogus.
+  }
   unique_ids_seen_.insert(unique_id);
   if (google_url_tracker_->registrar_.IsRegistered(google_url_tracker_.get(),
       content::NOTIFICATION_NAV_ENTRY_PENDING,
@@ -247,13 +253,6 @@ void GoogleURLTrackerTest::SetNonSearchPending(int unique_id) {
   }
 }
 
-void GoogleURLTrackerTest::SetSearchPending(int unique_id) {
-  google_url_tracker_->SearchCommitted();
-  // Note that the call above might not have actually registered a listener for
-  // NOTIFICATION_NAV_ENTRY_PENDING if the searchdomaincheck response was bogus.
-  SetNonSearchPending(unique_id);
-}
-
 void GoogleURLTrackerTest::CommitNonSearch(int unique_id) {
   GoogleURLTracker::InfoBarMap::iterator i =
       google_url_tracker_->infobar_map_.find(
@@ -272,7 +271,7 @@ void GoogleURLTrackerTest::CommitNonSearch(int unique_id) {
   }
 }
 
-void GoogleURLTrackerTest::CommitSearch(const GURL& search_url, int unique_id) {
+void GoogleURLTrackerTest::CommitSearch(int unique_id, const GURL& search_url) {
   if (google_url_tracker_->registrar_.IsRegistered(google_url_tracker_.get(),
       content::NOTIFICATION_NAV_ENTRY_COMMITTED,
       content::Source<content::NavigationController>(
@@ -283,6 +282,26 @@ void GoogleURLTrackerTest::CommitSearch(const GURL& search_url, int unique_id) {
   }
 }
 
+void GoogleURLTrackerTest::DoInstantNavigation(int unique_id,
+                                               const GURL& search_url) {
+  if (!search_url.is_empty()) {
+    google_url_tracker_->SearchCommitted();
+    // Note that the call above might not have actually registered a listener
+    // for NOTIFICATION_INSTANT_COMMITTED if the searchdomaincheck response was
+    // bogus.
+  }
+  unique_ids_seen_.insert(unique_id);
+  if (google_url_tracker_->registrar_.IsRegistered(google_url_tracker_.get(),
+      chrome::NOTIFICATION_INSTANT_COMMITTED,
+      content::NotificationService::AllBrowserContextsAndSources())) {
+    google_url_tracker_->OnInstantCommitted(
+        content::Source<content::NavigationController>(
+            reinterpret_cast<content::NavigationController*>(unique_id)),
+        content::Source<TabContents>(reinterpret_cast<TabContents*>(unique_id)),
+        reinterpret_cast<InfoBarTabHelper*>(unique_id), search_url);
+  }
+}
+
 void GoogleURLTrackerTest::CloseTab(int unique_id) {
   unique_ids_seen_.erase(unique_id);
   InfoBarTabHelper* infobar_helper =
@@ -325,7 +344,10 @@ void GoogleURLTrackerTest::ExpectListeningForCommit(int unique_id,
   GoogleURLTracker::InfoBarMap::iterator i =
       google_url_tracker_->infobar_map_.find(
           reinterpret_cast<InfoBarTabHelper*>(unique_id));
-  ASSERT_FALSE(i == google_url_tracker_->infobar_map_.end());
+  if (i == google_url_tracker_->infobar_map_.end()) {
+    EXPECT_FALSE(listening);
+    return;
+  }
   EXPECT_EQ(listening, google_url_tracker_->registrar_.IsRegistered(
       google_url_tracker_.get(), content::NOTIFICATION_NAV_ENTRY_COMMITTED,
       i->second.navigation_controller_source));
@@ -389,8 +411,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
   EXPECT_EQ(GURL(), fetched_google_url());
   EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
   EXPECT_FALSE(observer_->notified());
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   EXPECT_TRUE(GetInfoBar(1) == NULL);
 
   // Bad subdomain.
@@ -399,8 +421,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
   EXPECT_EQ(GURL(), fetched_google_url());
   EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
   EXPECT_FALSE(observer_->notified());
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   EXPECT_TRUE(GetInfoBar(1) == NULL);
 
   // Non-empty path.
@@ -409,8 +431,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
   EXPECT_EQ(GURL(), fetched_google_url());
   EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
   EXPECT_FALSE(observer_->notified());
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   EXPECT_TRUE(GetInfoBar(1) == NULL);
 
   // Non-empty query.
@@ -419,8 +441,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
   EXPECT_EQ(GURL(), fetched_google_url());
   EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
   EXPECT_FALSE(observer_->notified());
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   EXPECT_TRUE(GetInfoBar(1) == NULL);
 
   // Non-empty ref.
@@ -429,8 +451,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
   EXPECT_EQ(GURL(), fetched_google_url());
   EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
   EXPECT_FALSE(observer_->notified());
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   EXPECT_TRUE(GetInfoBar(1) == NULL);
 
   // Complete garbage.
@@ -439,8 +461,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
   EXPECT_EQ(GURL(), fetched_google_url());
   EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
   EXPECT_FALSE(observer_->notified());
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   EXPECT_TRUE(GetInfoBar(1) == NULL);
 }
 
@@ -551,8 +573,8 @@ TEST_F(GoogleURLTrackerTest, SearchingDoesNothingIfNoNeedToPrompt) {
   EXPECT_TRUE(observer_->notified());
   observer_->clear_notified();
 
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   TestInfoBarDelegate* infobar = GetInfoBar(1);
   EXPECT_TRUE(infobar == NULL);
   EXPECT_EQ(GURL("http://www.google.co.uk/"), fetched_google_url());
@@ -571,7 +593,7 @@ TEST_F(GoogleURLTrackerTest, TabClosedOnPendingSearch) {
   EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
   EXPECT_FALSE(observer_->notified());
 
-  SetSearchPending(1);
+  SetNavigationPending(1, true);
   TestInfoBarDelegate* infobar = GetInfoBar(1);
   ASSERT_FALSE(infobar == NULL);
   EXPECT_FALSE(infobar->showing());
@@ -593,8 +615,8 @@ TEST_F(GoogleURLTrackerTest, TabClosedOnCommittedSearch) {
   FinishSleep();
   MockSearchDomainCheckResponse("http://www.google.co.jp/");
 
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   TestInfoBarDelegate* infobar = GetInfoBar(1);
   ASSERT_FALSE(infobar == NULL);
   EXPECT_TRUE(infobar->showing());
@@ -612,8 +634,8 @@ TEST_F(GoogleURLTrackerTest, InfobarClosed) {
   FinishSleep();
   MockSearchDomainCheckResponse("http://www.google.co.jp/");
 
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   TestInfoBarDelegate* infobar = GetInfoBar(1);
   ASSERT_FALSE(infobar == NULL);
 
@@ -630,8 +652,8 @@ TEST_F(GoogleURLTrackerTest, InfobarRefused) {
   FinishSleep();
   MockSearchDomainCheckResponse("http://www.google.co.jp/");
 
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   TestInfoBarDelegate* infobar = GetInfoBar(1);
   ASSERT_FALSE(infobar == NULL);
 
@@ -648,8 +670,8 @@ TEST_F(GoogleURLTrackerTest, InfobarAccepted) {
   FinishSleep();
   MockSearchDomainCheckResponse("http://www.google.co.jp/");
 
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   TestInfoBarDelegate* infobar = GetInfoBar(1);
   ASSERT_FALSE(infobar == NULL);
 
@@ -660,6 +682,18 @@ TEST_F(GoogleURLTrackerTest, InfobarAccepted) {
   EXPECT_TRUE(observer_->notified());
 }
 
+TEST_F(GoogleURLTrackerTest, InfobarForInstant) {
+  SetLastPromptedGoogleURL(GURL("http://www.google.co.uk/"));
+  RequestServerCheck();
+  FinishSleep();
+  MockSearchDomainCheckResponse("http://www.google.co.jp/");
+
+  DoInstantNavigation(1, GURL("http://www.google.co.uk/search?q=test"));
+  TestInfoBarDelegate* infobar = GetInfoBar(1);
+  ASSERT_FALSE(infobar == NULL);
+  EXPECT_TRUE(infobar->showing());
+}
+
 TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfobars) {
   RequestServerCheck();
   FinishSleep();
@@ -669,8 +703,8 @@ TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfobars) {
   // should close the infobar.
   NotifyIPAddressChanged();
   MockSearchDomainCheckResponse("http://www.google.co.uk/");
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.com/search?q=test"));
   EXPECT_FALSE(GetInfoBar(1) == NULL);
   NotifyIPAddressChanged();
   MockSearchDomainCheckResponse(google_url().spec());
@@ -680,8 +714,8 @@ TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfobars) {
   // As should fetching a URL that differs from the accepted only by the scheme.
   NotifyIPAddressChanged();
   MockSearchDomainCheckResponse("http://www.google.co.uk/");
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.com/search?q=test"));
   EXPECT_FALSE(GetInfoBar(1) == NULL);
   NotifyIPAddressChanged();
   url_canon::Replacements<char> replacements;
@@ -697,8 +731,8 @@ TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfobars) {
   SetLastPromptedGoogleURL(GURL("http://www.google.co.uk/"));
   NotifyIPAddressChanged();
   MockSearchDomainCheckResponse("http://www.google.co.jp/");
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.com/search?q=test"));
   EXPECT_FALSE(GetInfoBar(1) == NULL);
   NotifyIPAddressChanged();
   MockSearchDomainCheckResponse("http://www.google.co.uk/");
@@ -709,8 +743,8 @@ TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfobars) {
   // And one that differs from the last prompted URL only by the scheme.
   NotifyIPAddressChanged();
   MockSearchDomainCheckResponse("http://www.google.co.jp/");
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.com/search?q=test"));
   EXPECT_FALSE(GetInfoBar(1) == NULL);
   NotifyIPAddressChanged();
   MockSearchDomainCheckResponse("https://www.google.co.uk/");
@@ -721,8 +755,8 @@ TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfobars) {
   // And fetching a different URL entirely.
   NotifyIPAddressChanged();
   MockSearchDomainCheckResponse("http://www.google.co.jp/");
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.com/search?q=test"));
   EXPECT_FALSE(GetInfoBar(1) == NULL);
   NotifyIPAddressChanged();
   MockSearchDomainCheckResponse("https://www.google.co.in/");
@@ -738,7 +772,7 @@ TEST_F(GoogleURLTrackerTest, ResetInfobarGoogleURLs) {
 
   NotifyIPAddressChanged();
   MockSearchDomainCheckResponse("http://www.google.co.uk/");
-  SetSearchPending(1);
+  SetNavigationPending(1, true);
   TestInfoBarDelegate* infobar = GetInfoBar(1);
   ASSERT_FALSE(infobar == NULL);
   EXPECT_EQ(GURL("http://www.google.co.uk/"), infobar->new_google_url());
@@ -754,7 +788,7 @@ TEST_F(GoogleURLTrackerTest, ResetInfobarGoogleURLs) {
   EXPECT_EQ(GURL("https://www.google.co.uk/"), new_infobar->new_google_url());
 
   // Same with an infobar that is showing.
-  CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
+  CommitSearch(1, GURL("http://www.google.com/search?q=test"));
   EXPECT_TRUE(infobar->showing());
   NotifyIPAddressChanged();
   MockSearchDomainCheckResponse("http://www.google.co.uk/");
@@ -772,20 +806,20 @@ TEST_F(GoogleURLTrackerTest, NavigationsAfterPendingSearch) {
   MockSearchDomainCheckResponse("http://www.google.co.jp/");
 
   // A pending non-search after a pending search should close the infobar.
-  SetSearchPending(1);
+  SetNavigationPending(1, true);
   TestInfoBarDelegate* infobar = GetInfoBar(1);
   ASSERT_FALSE(infobar == NULL);
   EXPECT_FALSE(infobar->showing());
-  SetNonSearchPending(1);
+  SetNavigationPending(1, false);
   infobar = GetInfoBar(1);
   EXPECT_TRUE(infobar == NULL);
 
   // A pending search after a pending search should leave the infobar alive.
-  SetSearchPending(1);
+  SetNavigationPending(1, true);
   infobar = GetInfoBar(1);
   ASSERT_FALSE(infobar == NULL);
   EXPECT_FALSE(infobar->showing());
-  SetSearchPending(1);
+  SetNavigationPending(1, true);
   TestInfoBarDelegate* new_infobar = GetInfoBar(1);
   ASSERT_FALSE(new_infobar == NULL);
   EXPECT_EQ(infobar, new_infobar);
@@ -793,7 +827,7 @@ TEST_F(GoogleURLTrackerTest, NavigationsAfterPendingSearch) {
   ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
 
   // Committing this search should show the infobar.
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test2"), 1);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test2"));
   EXPECT_TRUE(infobar->showing());
   EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
   EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
@@ -806,15 +840,15 @@ TEST_F(GoogleURLTrackerTest, NavigationsAfterCommittedSearch) {
   RequestServerCheck();
   FinishSleep();
   MockSearchDomainCheckResponse("http://www.google.co.jp/");
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   TestInfoBarDelegate* infobar = GetInfoBar(1);
   ASSERT_FALSE(infobar == NULL);
   EXPECT_TRUE(infobar->showing());
   ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
 
   // A pending non-search on a visible infobar should basically do nothing.
-  SetNonSearchPending(1);
+  SetNavigationPending(1, false);
   TestInfoBarDelegate* new_infobar = GetInfoBar(1);
   ASSERT_FALSE(new_infobar == NULL);
   EXPECT_EQ(infobar, new_infobar);
@@ -823,7 +857,7 @@ TEST_F(GoogleURLTrackerTest, NavigationsAfterCommittedSearch) {
   ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
 
   // As should another pending non-search after the first.
-  SetNonSearchPending(1);
+  SetNavigationPending(1, false);
   new_infobar = GetInfoBar(1);
   ASSERT_FALSE(new_infobar == NULL);
   EXPECT_EQ(infobar, new_infobar);
@@ -840,12 +874,12 @@ TEST_F(GoogleURLTrackerTest, NavigationsAfterCommittedSearch) {
 
   // A pending search on a visible infobar should cause the infobar to listen
   // for the search to commit.
-  SetSearchPending(1);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   infobar = GetInfoBar(1);
   ASSERT_FALSE(infobar == NULL);
   EXPECT_TRUE(infobar->showing());
-  SetSearchPending(1);
+  SetNavigationPending(1, true);
   new_infobar = GetInfoBar(1);
   ASSERT_FALSE(new_infobar == NULL);
   EXPECT_EQ(infobar, new_infobar);
@@ -854,7 +888,7 @@ TEST_F(GoogleURLTrackerTest, NavigationsAfterCommittedSearch) {
   ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
 
   // But a non-search after this should cancel that state.
-  SetNonSearchPending(1);
+  SetNavigationPending(1, false);
   new_infobar = GetInfoBar(1);
   ASSERT_FALSE(new_infobar == NULL);
   EXPECT_EQ(infobar, new_infobar);
@@ -864,7 +898,7 @@ TEST_F(GoogleURLTrackerTest, NavigationsAfterCommittedSearch) {
 
   // Another pending search after the non-search should put us back into
   // "waiting for commit" mode.
-  SetSearchPending(1);
+  SetNavigationPending(1, true);
   new_infobar = GetInfoBar(1);
   ASSERT_FALSE(new_infobar == NULL);
   EXPECT_EQ(infobar, new_infobar);
@@ -873,7 +907,7 @@ TEST_F(GoogleURLTrackerTest, NavigationsAfterCommittedSearch) {
   ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
 
   // A second pending search after the first should not really change anything.
-  SetSearchPending(1);
+  SetNavigationPending(1, true);
   new_infobar = GetInfoBar(1);
   ASSERT_FALSE(new_infobar == NULL);
   EXPECT_EQ(infobar, new_infobar);
@@ -882,7 +916,7 @@ TEST_F(GoogleURLTrackerTest, NavigationsAfterCommittedSearch) {
   ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
 
   // Committing this search should change the visible infobar's search_url.
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test2"), 1);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test2"));
   new_infobar = GetInfoBar(1);
   ASSERT_FALSE(new_infobar == NULL);
   EXPECT_EQ(infobar, new_infobar);
@@ -901,33 +935,33 @@ TEST_F(GoogleURLTrackerTest, MultipleInfobars) {
   FinishSleep();
   MockSearchDomainCheckResponse("http://www.google.co.jp/");
 
-  SetSearchPending(1);
+  SetNavigationPending(1, true);
   TestInfoBarDelegate* infobar = GetInfoBar(1);
   ASSERT_FALSE(infobar == NULL);
   EXPECT_FALSE(infobar->showing());
 
-  SetSearchPending(2);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test2"), 2);
+  SetNavigationPending(2, true);
+  CommitSearch(2, GURL("http://www.google.co.uk/search?q=test2"));
   TestInfoBarDelegate* infobar2 = GetInfoBar(2);
   ASSERT_FALSE(infobar2 == NULL);
   EXPECT_TRUE(infobar2->showing());
   EXPECT_EQ(GURL("http://www.google.co.uk/search?q=test2"),
             infobar2->search_url());
 
-  SetSearchPending(3);
+  SetNavigationPending(3, true);
   TestInfoBarDelegate* infobar3 = GetInfoBar(3);
   ASSERT_FALSE(infobar3 == NULL);
   EXPECT_FALSE(infobar3->showing());
 
-  SetSearchPending(4);
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test4"), 4);
+  SetNavigationPending(4, true);
+  CommitSearch(4, GURL("http://www.google.co.uk/search?q=test4"));
   TestInfoBarDelegate* infobar4 = GetInfoBar(4);
   ASSERT_FALSE(infobar4 == NULL);
   EXPECT_TRUE(infobar4->showing());
   EXPECT_EQ(GURL("http://www.google.co.uk/search?q=test4"),
             infobar4->search_url());
 
-  CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
   EXPECT_TRUE(infobar->showing());
 
   infobar2->InfoBarClosed();
@@ -942,3 +976,50 @@ TEST_F(GoogleURLTrackerTest, MultipleInfobars) {
   EXPECT_EQ(GURL("http://www.google.co.jp/"), GetLastPromptedGoogleURL());
   EXPECT_TRUE(observer_->notified());
 }
+
+TEST_F(GoogleURLTrackerTest, IgnoreIrrelevantInstantNavigation) {
+  SetLastPromptedGoogleURL(GURL("http://www.google.co.uk/"));
+  RequestServerCheck();
+  FinishSleep();
+  MockSearchDomainCheckResponse("http://www.google.co.jp/");
+
+  // Starting a search pending on any tab should cause us to listen for pending
+  // and instant navigations on all tabs, but we should ignore these when they
+  // are for tabs that we don't care about.
+  SetNavigationPending(1, true);
+  TestInfoBarDelegate* infobar = GetInfoBar(1);
+  ASSERT_FALSE(infobar == NULL);
+  ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
+  ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(2, false));
+
+  DoInstantNavigation(2, GURL());
+  TestInfoBarDelegate* infobar2 = GetInfoBar(2);
+  ASSERT_TRUE(infobar2 == NULL);
+  ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
+  ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(2, false));
+}
+
+TEST_F(GoogleURLTrackerTest, IgnoreIrrelevantNavigation) {
+  SetLastPromptedGoogleURL(GURL("http://www.google.co.uk/"));
+  RequestServerCheck();
+  FinishSleep();
+  MockSearchDomainCheckResponse("http://www.google.co.jp/");
+
+  // This tests a particularly gnarly sequence of events that used to cause us
+  // to erroneously listen for a non-search navigation to commit.
+  SetNavigationPending(1, true);
+  CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
+  SetNavigationPending(2, true);
+  CommitSearch(2, GURL("http://www.google.co.uk/search?q=test2"));
+  TestInfoBarDelegate* infobar = GetInfoBar(1);
+  ASSERT_FALSE(infobar == NULL);
+  EXPECT_TRUE(infobar->showing());
+  TestInfoBarDelegate* infobar2 = GetInfoBar(2);
+  ASSERT_FALSE(infobar2 == NULL);
+  EXPECT_TRUE(infobar2->showing());
+  SetNavigationPending(1, true);
+  ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
+  infobar2->InfoBarClosed();
+  SetNavigationPending(1, false);
+  ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
+}