Fix a crash that could occur if the user closed a tab with an uncommitted search navigation that we…

chrome/browser/google/google_url_tracker.h

 // 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.
 
 #ifndef CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_H_
 #define CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_H_
 
 #include <map>
 #include <string>
 #include <utility>
@@ skipping 84 matching lines @@
   // No one but GoogleURLTrackerMapEntry should call this.
   void DeleteMapEntryForHelper(const InfoBarTabHelper* infobar_helper);
 
   static const char kDefaultGoogleHomepage[];
   static const char kSearchDomainCheckURL[];
 
  private:
   friend class GoogleURLTrackerTest;
 
   typedef std::map<const InfoBarTabHelper*,
-                   GoogleURLTrackerMapEntry*> InfoBarMap;
+                   GoogleURLTrackerMapEntry*> EntryMap;
 
   // net::URLFetcherDelegate:
   virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
 
   // content::NotificationObserver:
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
   // NetworkChangeNotifier::IPAddressObserver:
@@ skipping 29 matching lines @@
   // 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& web_contents_source,
       InfoBarTabHelper* infobar_helper,
       int pending_id);
 
-  // Called by Observe() once a load we're watching commits, or the associated
-  // tab is closed.  |infobar_helper| is the same as for OnNavigationPending();
-  // |search_url| is valid when this call is due to a successful navigation
-  // (indicating that we should show or update the relevant infobar) as opposed
-  // to tab closure (which means we should delete the infobar).
-  void OnNavigationCommittedOrTabClosed(InfoBarTabHelper* infobar_helper,
-                                        const GURL& search_url);
+  // Called by Observe() once a load we're watching commits.  |infobar_helper|
+  // is the same as for OnNavigationPending(); |search_url| is guaranteed to be
+  // valid.
+  void OnNavigationCommitted(InfoBarTabHelper* infobar_helper,
+                             const GURL& search_url);
+
+  // Called by Observe() when a tab closes.  Because the InfoBarTabHelper may
+  // have already been torn down in this case, we look up the appropriate map
+  // entry by |web_contents_source| instead.
+  void OnTabClosed(const content::NotificationSource& web_contents_source);
 
   // 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().
+  // listening for, may then also call OnNavigationCommitted().
   void OnInstantCommitted(
     const content::NotificationSource& navigation_controller_source,
     const content::NotificationSource& web_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);
+  // Closes all map entries.  If |redo_searches| is true, this also triggers
+  // each tab with an infobar to re-perform the user's search, but on the new
+  // Google TLD.
+  void CloseAllEntries(bool redo_searches);

[Ilya Sherman, 2012/10/30 23:49:40]

nit: Perhaps "CloseAllMappedInfoBars"?  Closing an "entry" doesn't seem like a
very intuitive notion to me.

[Peter Kasting, 2012/10/30 23:55:04]

I called it this because the MapEntry function is called Close(), and it's
meaningful even in the absence of an infobar.

I am not really sure how to name that function better :/

 
   // Unregisters any listeners for the notification sources in |map_entry|.
   // 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.)  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 GoogleURLTrackerMapEntry& map_entry,
@@ skipping 22 matching lines @@
   bool need_to_fetch_;     // True if a consumer actually wants us to fetch an
                            // updated URL.  If this is never set, we won't
                            // bother to fetch anything.
                            // Consumers should observe
                            // chrome::NOTIFICATION_GOOGLE_URL_UPDATED.
   bool need_to_prompt_;    // True if the last fetched Google URL is not
                            // matched with current user's default Google URL
                            // nor the last prompted Google URL.
   bool search_committed_;  // True when we're expecting a notification of a new
                            // pending search navigation.
-  InfoBarMap infobar_map_;
+  EntryMap entry_map_;
 
   DISALLOW_COPY_AND_ASSIGN(GoogleURLTracker);
 };
 
 #endif  // CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_H_