diff options
author | stuartmorgan@chromium.org <stuartmorgan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-02-26 12:33:59 +0000 |
---|---|---|
committer | stuartmorgan@chromium.org <stuartmorgan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-02-26 12:33:59 +0000 |
commit | fc37052a3267c29bd239250861aa8f3e787a7736 (patch) | |
tree | 5eabb62574298c2b37ceef930184264c1f696522 | |
parent | 42283014e4f8ba57023973b7f483dfbaa27a7e7f (diff) | |
download | chromium_src-fc37052a3267c29bd239250861aa8f3e787a7736.zip chromium_src-fc37052a3267c29bd239250861aa8f3e787a7736.tar.gz chromium_src-fc37052a3267c29bd239250861aa8f3e787a7736.tar.bz2 |
Split WebContents and navigation notifications out of GoogleURLTracker
Separates the content-implementation-specific aspects of
GoogleURLTracker--WebContents usage, and notifications from the
implementation of NavigationController--into a helper class. This class
also hides the extra complexity of Instant, so that it is transparent
to GoogleURLTracker.
This is primarily to support GoogleURLTracker use in iOS, which doesn't
use WebContents or NavigationControllerImpl. An iOS implementation
of GoogleURLTrackerNavigationObserver will follow at some later point.
BUG=169452
Review URL: https://codereview.chromium.org/11856011
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@184633 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/google/DEPS | 8 | ||||
-rw-r--r-- | chrome/browser/google/google_url_tracker.cc | 171 | ||||
-rw-r--r-- | chrome/browser/google/google_url_tracker.h | 90 | ||||
-rw-r--r-- | chrome/browser/google/google_url_tracker_factory.cc | 6 | ||||
-rw-r--r-- | chrome/browser/google/google_url_tracker_map_entry.cc | 7 | ||||
-rw-r--r-- | chrome/browser/google/google_url_tracker_map_entry.h | 18 | ||||
-rw-r--r-- | chrome/browser/google/google_url_tracker_navigation_helper.h | 58 | ||||
-rw-r--r-- | chrome/browser/google/google_url_tracker_navigation_helper_impl.cc | 180 | ||||
-rw-r--r-- | chrome/browser/google/google_url_tracker_navigation_helper_impl.h | 58 | ||||
-rw-r--r-- | chrome/browser/google/google_url_tracker_unittest.cc | 193 | ||||
-rw-r--r-- | chrome/chrome_browser.gypi | 3 |
11 files changed, 507 insertions, 285 deletions
diff --git a/chrome/browser/google/DEPS b/chrome/browser/google/DEPS new file mode 100644 index 0000000..f40a6ca --- /dev/null +++ b/chrome/browser/google/DEPS @@ -0,0 +1,8 @@ +specific_include_rules = { + # GoogleURLTracker can't use WebContents, since it is used by iOS. + # If WebContents functionality is needed, add it to + # GoogleURLTrackerNavigationHelperImpl. + 'google_url_tracker\.': [ + "-content/public/browser/web_contents.h", + ], +} diff --git a/chrome/browser/google/google_url_tracker.cc b/chrome/browser/google/google_url_tracker.cc index 6e616fe..5397838 100644 --- a/chrome/browser/google/google_url_tracker.cc +++ b/chrome/browser/google/google_url_tracker.cc @@ -11,6 +11,7 @@ #include "chrome/browser/api/infobars/infobar_service.h" #include "chrome/browser/google/google_url_tracker_factory.h" #include "chrome/browser/google/google_url_tracker_infobar_delegate.h" +#include "chrome/browser/google/google_url_tracker_navigation_helper.h" #include "chrome/browser/google/google_util.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/chrome_notification_types.h" @@ -19,7 +20,6 @@ #include "content/public/browser/navigation_controller.h" #include "content/public/browser/navigation_entry.h" #include "content/public/browser/notification_service.h" -#include "content/public/browser/web_contents.h" #include "net/base/load_flags.h" #include "net/base/net_util.h" #include "net/url_request/url_fetcher.h" @@ -31,8 +31,12 @@ const char GoogleURLTracker::kDefaultGoogleHomepage[] = const char GoogleURLTracker::kSearchDomainCheckURL[] = "https://www.google.com/searchdomaincheck?format=url&type=chrome"; -GoogleURLTracker::GoogleURLTracker(Profile* profile, Mode mode) +GoogleURLTracker::GoogleURLTracker( + Profile* profile, + scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper, + Mode mode) : profile_(profile), + nav_helper_(nav_helper.Pass()), infobar_creator_(base::Bind(&GoogleURLTrackerInfoBarDelegate::Create)), google_url_(mode == UNIT_TEST_MODE ? kDefaultGoogleHomepage : profile->GetPrefs()->GetString(prefs::kLastKnownGoogleURL)), @@ -44,6 +48,7 @@ GoogleURLTracker::GoogleURLTracker(Profile* profile, Mode mode) need_to_prompt_(false), search_committed_(false) { net::NetworkChangeNotifier::AddIPAddressObserver(this); + nav_helper_->SetGoogleURLTracker(this); // Because this function can be called during startup, when kicking off a URL // fetch can eat up 20 ms of time, we delay five seconds, which is hopefully @@ -185,72 +190,13 @@ void GoogleURLTracker::OnURLFetchComplete(const net::URLFetcher* source) { } } -void GoogleURLTracker::Observe(int type, - const content::NotificationSource& source, - const content::NotificationDetails& details) { - switch (type) { - case content::NOTIFICATION_NAV_ENTRY_PENDING: { - content::NavigationController* controller = - content::Source<content::NavigationController>(source).ptr(); - content::WebContents* web_contents = controller->GetWebContents(); - InfoBarService* infobar_service = - InfoBarService::FromWebContents(web_contents); - // Because we're listening to all sources, there may be no - // InfoBarService for some notifications, e.g. navigations in - // bubbles/balloons etc. - if (infobar_service) { - OnNavigationPending( - source, content::Source<content::WebContents>(web_contents), - infobar_service, controller->GetPendingEntry()->GetUniqueID()); - } - break; - } - - case content::NOTIFICATION_NAV_ENTRY_COMMITTED: { - content::NavigationController* controller = - content::Source<content::NavigationController>(source).ptr(); - // Here we're only listening to notifications where we already know - // there's an associated InfoBarService. - content::WebContents* web_contents = controller->GetWebContents(); - InfoBarService* infobar_service = - InfoBarService::FromWebContents(web_contents); - DCHECK(infobar_service); - const GURL& search_url = controller->GetActiveEntry()->GetURL(); - if (!search_url.is_valid()) // Not clear if this can happen. - OnTabClosed(content::Source<content::WebContents>(web_contents)); - OnNavigationCommitted(infobar_service, search_url); - break; - } - - case content::NOTIFICATION_WEB_CONTENTS_DESTROYED: - OnTabClosed(source); - break; - - case chrome::NOTIFICATION_INSTANT_COMMITTED: { - content::WebContents* web_contents = - content::Source<content::WebContents>(source).ptr(); - const GURL& search_url = web_contents->GetURL(); - if (!search_url.is_valid()) // Not clear if this can happen. - OnTabClosed(source); - OnInstantCommitted( - content::Source<content::NavigationController>( - &web_contents->GetController()), - source, InfoBarService::FromWebContents(web_contents), search_url); - break; - } - - default: - NOTREACHED() << "Unknown notification received:" << type; - } -} - void GoogleURLTracker::OnIPAddressChanged() { already_fetched_ = false; StartFetchIfDesirable(); } void GoogleURLTracker::Shutdown() { - registrar_.RemoveAll(); + nav_helper_.reset(); weak_ptr_factory_.InvalidateWeakPtrs(); fetcher_.reset(); net::NetworkChangeNotifier::RemoveIPAddressObserver(this); @@ -320,19 +266,13 @@ void GoogleURLTracker::SearchCommitted() { search_committed_ = true; // These notifications will fire a bit later in the same call chain we're // currently in. - if (!registrar_.IsRegistered(this, content::NOTIFICATION_NAV_ENTRY_PENDING, - content::NotificationService::AllBrowserContextsAndSources())) { - registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_PENDING, - content::NotificationService::AllBrowserContextsAndSources()); - registrar_.Add(this, chrome::NOTIFICATION_INSTANT_COMMITTED, - content::NotificationService::AllBrowserContextsAndSources()); - } + if (!nav_helper_->IsListeningForNavigationStart()) + nav_helper_->SetListeningForNavigationStart(true); } } void GoogleURLTracker::OnNavigationPending( - const content::NotificationSource& navigation_controller_source, - const content::NotificationSource& web_contents_source, + content::NavigationController* navigation_controller, InfoBarService* infobar_service, int pending_id) { EntryMap::iterator i(entry_map_.find(infobar_service)); @@ -343,11 +283,9 @@ void GoogleURLTracker::OnNavigationPending( // load to commit, so we can show and/or update the infobar when it does. // (We may already be registered for this if there is an existing infobar // that had a previous pending search that hasn't yet committed.) - if (!registrar_.IsRegistered(this, - content::NOTIFICATION_NAV_ENTRY_COMMITTED, - navigation_controller_source)) { - registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED, - navigation_controller_source); + if (!nav_helper_->IsListeningForNavigationCommit(navigation_controller)) { + nav_helper_->SetListeningForNavigationCommit(navigation_controller, + true); } if (i == entry_map_.end()) { // This is a search on a tab that doesn't have one of our infobars, so @@ -356,13 +294,11 @@ void GoogleURLTracker::OnNavigationPending( // yet have an infobar and we're already registered for this, or it has an // infobar and the infobar's owner will handle tearing it down when the // tab is destroyed. - registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED, - web_contents_source); + nav_helper_->SetListeningForTabDestruction(navigation_controller, true); entry_map_.insert(std::make_pair( infobar_service, new GoogleURLTrackerMapEntry(this, infobar_service, - navigation_controller_source, - web_contents_source))); + navigation_controller))); } else if (i->second->has_infobar()) { // This is a new search on a tab where we already have an infobar. i->second->infobar()->set_pending_id(pending_id); @@ -413,10 +349,9 @@ void GoogleURLTracker::OnNavigationCommitted(InfoBarService* infobar_service, } void GoogleURLTracker::OnTabClosed( - const content::NotificationSource& web_contents_source) { - // Because InfoBarService tears itself down in response to - // NOTIFICATION_WEB_CONTENTS_DESTROYED, it may or may not be possible to - // get a non-NULL pointer back from InfoBarService::FromWebContents() here, + content::NavigationController* navigation_controller) { + // Because InfoBarService tears itself down in on tab destruction, it may + // or may not be possible to get a non-NULL InfoBarService pointer here, // depending on which order notifications fired in. Likewise, the pointer in // |entry_map_| (and in its associated MapEntry) may point to deleted memory. // Therefore, if we were to access to the InfoBarService* we have for this @@ -424,7 +359,7 @@ void GoogleURLTracker::OnTabClosed( // dereferenced it. This function doesn't need to do even that, but others in // the call chain from here might (and have comments pointing back here). for (EntryMap::iterator i(entry_map_.begin()); i != entry_map_.end(); ++i) { - if (i->second->web_contents_source() == web_contents_source) { + if (i->second->navigation_controller() == navigation_controller) { i->second->Close(false); return; } @@ -432,31 +367,6 @@ void GoogleURLTracker::OnTabClosed( NOTREACHED(); } -void GoogleURLTracker::OnInstantCommitted( - const content::NotificationSource& navigation_controller_source, - const content::NotificationSource& web_contents_source, - InfoBarService* infobar_service, - 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 - // OnNavigationCommitted() 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 - // OnNavigationCommitted() 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, web_contents_source, - infobar_service, 0); - EntryMap::iterator i(entry_map_.find(infobar_service)); - DCHECK_EQ(was_search_committed, (i != entry_map_.end()) && - registrar_.IsRegistered(this, - content::NOTIFICATION_NAV_ENTRY_COMMITTED, - i->second->navigation_controller_source())); - if (was_search_committed) - OnNavigationCommitted(infobar_service, search_url); -} - void GoogleURLTracker::CloseAllEntries(bool redo_searches) { // Delete all entries, whether they have infobars or not. while (!entry_map_.empty()) @@ -468,23 +378,22 @@ void GoogleURLTracker::UnregisterForEntrySpecificNotifications( bool must_be_listening_for_commit) { // For tabs with map entries but no infobars, we should always be listening // for both these notifications. For tabs with infobars, we may be listening - // for NOTIFICATION_NAV_ENTRY_COMMITTED if the user has performed a new search - // on this tab. - if (registrar_.IsRegistered(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED, - map_entry.navigation_controller_source())) { - registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED, - map_entry.navigation_controller_source()); + // for navigation commits if the user has performed a new search on this tab. + if (nav_helper_->IsListeningForNavigationCommit( + map_entry.navigation_controller())) { + nav_helper_->SetListeningForNavigationCommit( + map_entry.navigation_controller(), false); } else { DCHECK(!must_be_listening_for_commit); DCHECK(map_entry.has_infobar()); } - const bool registered_for_web_contents_destroyed = registrar_.IsRegistered( - this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED, - map_entry.web_contents_source()); - DCHECK_NE(registered_for_web_contents_destroyed, map_entry.has_infobar()); - if (registered_for_web_contents_destroyed) { - registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED, - map_entry.web_contents_source()); + const bool registered_for_tab_destruction = + nav_helper_->IsListeningForTabDestruction( + map_entry.navigation_controller()); + DCHECK_NE(registered_for_tab_destruction, map_entry.has_infobar()); + if (registered_for_tab_destruction) { + nav_helper_->SetListeningForTabDestruction( + map_entry.navigation_controller(), false); } // Our global listeners for these other notifications should be in place iff @@ -494,20 +403,14 @@ void GoogleURLTracker::UnregisterForEntrySpecificNotifications( // See the various cases inside OnNavigationPending(). for (EntryMap::const_iterator i(entry_map_.begin()); i != entry_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())); + if (nav_helper_->IsListeningForNavigationCommit( + i->second->navigation_controller())) { + DCHECK(nav_helper_->IsListeningForNavigationStart()); return; } } - if (registrar_.IsRegistered(this, content::NOTIFICATION_NAV_ENTRY_PENDING, - content::NotificationService::AllBrowserContextsAndSources())) { + if (nav_helper_->IsListeningForNavigationStart()) { DCHECK(!search_committed_); - registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_PENDING, - content::NotificationService::AllBrowserContextsAndSources()); - registrar_.Remove(this, chrome::NOTIFICATION_INSTANT_COMMITTED, - content::NotificationService::AllBrowserContextsAndSources()); + nav_helper_->SetListeningForNavigationStart(false); } } diff --git a/chrome/browser/google/google_url_tracker.h b/chrome/browser/google/google_url_tracker.h index 620f12d..c51dd7a 100644 --- a/chrome/browser/google/google_url_tracker.h +++ b/chrome/browser/google/google_url_tracker.h @@ -20,12 +20,12 @@ #include "net/url_request/url_fetcher.h" #include "net/url_request/url_fetcher_delegate.h" +class GoogleURLTrackerNavigationHelper; class PrefService; class Profile; namespace content { class NavigationController; -class WebContents; } // This object is responsible for checking the Google URL once per network @@ -42,7 +42,6 @@ class WebContents; // performed (ever) unless at least one consumer registers interest by calling // RequestServerCheck(). class GoogleURLTracker : public net::URLFetcherDelegate, - public content::NotificationObserver, public net::NetworkChangeNotifier::IPAddressObserver, public ProfileKeyedService { public: @@ -59,7 +58,9 @@ class GoogleURLTracker : public net::URLFetcherDelegate, // Only the GoogleURLTrackerFactory and tests should call this. No code other // than the GoogleURLTracker itself should actually use // GoogleURLTrackerFactory::GetForProfile(). - GoogleURLTracker(Profile* profile, Mode mode); + GoogleURLTracker(Profile* profile, + scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper, + Mode mode); virtual ~GoogleURLTracker(); @@ -79,9 +80,9 @@ class GoogleURLTracker : public net::URLFetcherDelegate, static void RequestServerCheck(Profile* profile); // Notifies the tracker that the user has started a Google search. - // If prompting is necessary, we then listen for the subsequent - // NAV_ENTRY_PENDING notification to get the appropriate NavigationController. - // When the load commits, we'll show the infobar. + // If prompting is necessary, we then listen for the subsequent pending + // navigation to get the appropriate NavigationController. When the load + // commits, we'll show the infobar. // // When |profile| is NULL or a testing profile, this function does nothing. static void GoogleURLSearchCommitted(Profile* profile); @@ -95,6 +96,30 @@ class GoogleURLTracker : public net::URLFetcherDelegate, // No one but GoogleURLTrackerMapEntry should call this. void DeleteMapEntryForService(const InfoBarService* infobar_service); + // Called by the navigation observer after SearchCommitted() registers + // listeners, to indicate that we've received the "load now pending" + // notification. |navigation_controller| is the NavigationController for this + // load; |infobar_service| is the InfoBarService 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. + virtual void OnNavigationPending( + content::NavigationController* navigation_controller, + InfoBarService* infobar_service, + int pending_id); + + // Called by the navigation observer once a load we're watching commits. + // |infobar_service| is the same as for OnNavigationPending(); + // |search_url| is guaranteed to be valid. + virtual void OnNavigationCommitted(InfoBarService* infobar_service, + const GURL& search_url); + + // Called by the navigation observer when a tab closes. + virtual void OnTabClosed( + content::NavigationController* navigation_controller); + static const char kDefaultGoogleHomepage[]; static const char kSearchDomainCheckURL[]; @@ -106,11 +131,6 @@ class GoogleURLTracker : public net::URLFetcherDelegate, // 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: virtual void OnIPAddressChanged() OVERRIDE; @@ -135,60 +155,24 @@ class GoogleURLTracker : public net::URLFetcherDelegate, // the notifications sent when the actual load is triggered. void SearchCommitted(); - // Called by Observe() after SearchCommitted() registers notification - // listeners, to indicate that we've received the "load now pending" - // notification. |navigation_controller_source| and |web_contents_source| are - // NotificationSources pointing to the associated NavigationController and - // WebContents, respectively, for this load; |infobar_service| is the - // InfoBarService 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& web_contents_source, - InfoBarService* infobar_service, - int pending_id); - - // Called by Observe() once a load we're watching commits. |infobar_service| - // is the same as for OnNavigationPending(); |search_url| is guaranteed to be - // valid. - void OnNavigationCommitted(InfoBarService* infobar_service, - const GURL& search_url); - - // Called by Observe() when a tab closes. Because the InfoBarService 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 OnNavigationCommitted(). - void OnInstantCommitted( - const content::NotificationSource& navigation_controller_source, - const content::NotificationSource& web_contents_source, - InfoBarService* infobar_service, - const GURL& search_url); - // 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); - // Unregisters any listeners for the notification sources in |map_entry|. + // Unregisters any listeners for the navigation controller 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. + // sanity-checking.) This also unregisters the global navigation pending + // listener if there are no remaining listeners for navigation commits, as we + // no longer need them until another search is committed. void UnregisterForEntrySpecificNotifications( const GoogleURLTrackerMapEntry& map_entry, bool must_be_listening_for_commit); Profile* profile_; - content::NotificationRegistrar registrar_; + + scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper_; // Creates an infobar delegate and adds it to the provided InfoBarService. // Returns the delegate pointer on success or NULL on failure. The caller diff --git a/chrome/browser/google/google_url_tracker_factory.cc b/chrome/browser/google/google_url_tracker_factory.cc index f23269f..750ecb9 100644 --- a/chrome/browser/google/google_url_tracker_factory.cc +++ b/chrome/browser/google/google_url_tracker_factory.cc @@ -6,6 +6,7 @@ #include "base/prefs/pref_service.h" #include "chrome/browser/google/google_url_tracker.h" +#include "chrome/browser/google/google_url_tracker_navigation_helper_impl.h" #include "chrome/browser/prefs/pref_registry_syncable.h" #include "chrome/browser/profiles/profile_dependency_manager.h" #include "chrome/common/pref_names.h" @@ -32,7 +33,10 @@ GoogleURLTrackerFactory::~GoogleURLTrackerFactory() { ProfileKeyedService* GoogleURLTrackerFactory::BuildServiceInstanceFor( Profile* profile) const { - return new GoogleURLTracker(profile, GoogleURLTracker::NORMAL_MODE); + scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper( + new GoogleURLTrackerNavigationHelperImpl()); + return new GoogleURLTracker(profile, nav_helper.Pass(), + GoogleURLTracker::NORMAL_MODE); } void GoogleURLTrackerFactory::RegisterUserPrefs( diff --git a/chrome/browser/google/google_url_tracker_map_entry.cc b/chrome/browser/google/google_url_tracker_map_entry.cc index 89a3ea6..5a2d617 100644 --- a/chrome/browser/google/google_url_tracker_map_entry.cc +++ b/chrome/browser/google/google_url_tracker_map_entry.cc @@ -9,18 +9,17 @@ #include "chrome/browser/infobars/infobar.h" #include "chrome/common/chrome_notification_types.h" #include "content/public/browser/notification_details.h" +#include "content/public/browser/notification_source.h" GoogleURLTrackerMapEntry::GoogleURLTrackerMapEntry( GoogleURLTracker* google_url_tracker, InfoBarService* infobar_service, - const content::NotificationSource& navigation_controller_source, - const content::NotificationSource& web_contents_source) + const content::NavigationController* navigation_controller) : google_url_tracker_(google_url_tracker), infobar_service_(infobar_service), infobar_(NULL), - navigation_controller_source_(navigation_controller_source), - web_contents_source_(web_contents_source) { + navigation_controller_(navigation_controller) { } GoogleURLTrackerMapEntry::~GoogleURLTrackerMapEntry() { diff --git a/chrome/browser/google/google_url_tracker_map_entry.h b/chrome/browser/google/google_url_tracker_map_entry.h index fbe6fc2..2a190f8 100644 --- a/chrome/browser/google/google_url_tracker_map_entry.h +++ b/chrome/browser/google/google_url_tracker_map_entry.h @@ -7,30 +7,29 @@ #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" -#include "content/public/browser/notification_source.h" class GoogleURLTracker; class GoogleURLTrackerInfoBarDelegate; class InfoBarService; +namespace content { +class NavigationController; +} + class GoogleURLTrackerMapEntry : public content::NotificationObserver { public: GoogleURLTrackerMapEntry( GoogleURLTracker* google_url_tracker, InfoBarService* infobar_service, - const content::NotificationSource& navigation_controller_source, - const content::NotificationSource& web_contents_source); + const content::NavigationController* navigation_controller); virtual ~GoogleURLTrackerMapEntry(); bool has_infobar() const { return !!infobar_; } GoogleURLTrackerInfoBarDelegate* infobar() { return infobar_; } void SetInfoBar(GoogleURLTrackerInfoBarDelegate* infobar); - const content::NotificationSource& navigation_controller_source() const { - return navigation_controller_source_; - } - const content::NotificationSource& web_contents_source() const { - return web_contents_source_; + const content::NavigationController* navigation_controller() const { + return navigation_controller_; } void Close(bool redo_search); @@ -47,8 +46,7 @@ class GoogleURLTrackerMapEntry : public content::NotificationObserver { GoogleURLTracker* const google_url_tracker_; const InfoBarService* const infobar_service_; GoogleURLTrackerInfoBarDelegate* infobar_; - const content::NotificationSource navigation_controller_source_; - const content::NotificationSource web_contents_source_; + const content::NavigationController* const navigation_controller_; DISALLOW_COPY_AND_ASSIGN(GoogleURLTrackerMapEntry); }; diff --git a/chrome/browser/google/google_url_tracker_navigation_helper.h b/chrome/browser/google/google_url_tracker_navigation_helper.h new file mode 100644 index 0000000..f48e6f0 --- /dev/null +++ b/chrome/browser/google/google_url_tracker_navigation_helper.h @@ -0,0 +1,58 @@ +// 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_NAVIGATION_HELPER_H_ +#define CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_NAVIGATION_HELPER_H_ + +class GoogleURLTracker; +class InfoBarService; +class Profile; + +namespace content { +class NavigationController; +} + +// A helper class for GoogleURLTracker that abstracts the details of listening +// for various navigation events. +class GoogleURLTrackerNavigationHelper { + public: + virtual ~GoogleURLTrackerNavigationHelper() {} + + // Sets the GoogleURLTracker that should receive callbacks from this observer. + virtual void SetGoogleURLTracker(GoogleURLTracker* tracker) = 0; + + // Enables or disables listening for navigation starts. OnNavigationPending + // will be called for each navigation start if listening is enabled. + virtual void SetListeningForNavigationStart(bool listen) = 0; + + // Returns whether or not the observer is currently listening for navigation + // starts. + virtual bool IsListeningForNavigationStart() = 0; + + // Enables or disables listening for navigation commits for the given + // NavigationController. OnNavigationCommitted will be called for each + // navigation commit if listening is enabled. + virtual void SetListeningForNavigationCommit( + const content::NavigationController* nav_controller, + bool listen) = 0; + + // Returns whether or not the observer is currently listening for navigation + // commits for the given NavigationController. + virtual bool IsListeningForNavigationCommit( + const content::NavigationController* nav_controller) = 0; + + // Enables or disables listening for tab destruction for the given + // NavigationController. OnTabClosed will be called on tab destruction if + // listening is enabled. + virtual void SetListeningForTabDestruction( + const content::NavigationController* nav_controller, + bool listen) = 0; + + // Returns whether or not the observer is currently listening for tab + // destruction for the given NavigationController. + virtual bool IsListeningForTabDestruction( + const content::NavigationController* nav_controller) = 0; +}; + +#endif // CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_NAVIGATION_HELPER_H_ diff --git a/chrome/browser/google/google_url_tracker_navigation_helper_impl.cc b/chrome/browser/google/google_url_tracker_navigation_helper_impl.cc new file mode 100644 index 0000000..28987d0 --- /dev/null +++ b/chrome/browser/google/google_url_tracker_navigation_helper_impl.cc @@ -0,0 +1,180 @@ +// 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/google/google_url_tracker_navigation_helper_impl.h" + +#include "chrome/browser/api/infobars/infobar_service.h" +#include "chrome/browser/google/google_url_tracker.h" +#include "chrome/common/chrome_notification_types.h" +#include "content/public/browser/navigation_controller.h" +#include "content/public/browser/navigation_entry.h" +#include "content/public/browser/notification_service.h" +#include "content/public/browser/web_contents.h" + +GoogleURLTrackerNavigationHelperImpl:: + GoogleURLTrackerNavigationHelperImpl() : tracker_(NULL) { +} + +GoogleURLTrackerNavigationHelperImpl:: + ~GoogleURLTrackerNavigationHelperImpl() { +} + +void GoogleURLTrackerNavigationHelperImpl::SetGoogleURLTracker( + GoogleURLTracker* tracker) { + DCHECK(tracker); + tracker_ = tracker; +} + +void GoogleURLTrackerNavigationHelperImpl::SetListeningForNavigationStart( + bool listen) { + if (listen) { + registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_PENDING, + content::NotificationService::AllBrowserContextsAndSources()); + registrar_.Add(this, chrome::NOTIFICATION_INSTANT_COMMITTED, + content::NotificationService::AllBrowserContextsAndSources()); + } else { + registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_PENDING, + content::NotificationService::AllBrowserContextsAndSources()); + registrar_.Remove(this, chrome::NOTIFICATION_INSTANT_COMMITTED, + content::NotificationService::AllBrowserContextsAndSources()); + } +} + +bool GoogleURLTrackerNavigationHelperImpl::IsListeningForNavigationStart() { + return registrar_.IsRegistered(this, content::NOTIFICATION_NAV_ENTRY_PENDING, + content::NotificationService::AllBrowserContextsAndSources()); +} + +void GoogleURLTrackerNavigationHelperImpl::SetListeningForNavigationCommit( + const content::NavigationController* nav_controller, + bool listen) { + content::NotificationSource navigation_controller_source = + content::Source<content::NavigationController>(nav_controller); + if (listen) { + registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED, + navigation_controller_source); + } else { + registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED, + navigation_controller_source); + } +} + +bool GoogleURLTrackerNavigationHelperImpl::IsListeningForNavigationCommit( + const content::NavigationController* nav_controller) { + content::NotificationSource navigation_controller_source = + content::Source<content::NavigationController>(nav_controller); + return registrar_.IsRegistered( + this, content::NOTIFICATION_NAV_ENTRY_COMMITTED, + navigation_controller_source); +} + +void GoogleURLTrackerNavigationHelperImpl::SetListeningForTabDestruction( + const content::NavigationController* nav_controller, + bool listen) { + content::NotificationSource navigation_controller_source = + content::Source<content::NavigationController>(nav_controller); + if (listen) { + registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED, + GetWebContentsSource(navigation_controller_source)); + } else { + registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED, + GetWebContentsSource(navigation_controller_source)); + } +} + +bool GoogleURLTrackerNavigationHelperImpl::IsListeningForTabDestruction( + const content::NavigationController* nav_controller) { + content::NotificationSource navigation_controller_source = + content::Source<content::NavigationController>(nav_controller); + return registrar_.IsRegistered( + this, + content::NOTIFICATION_WEB_CONTENTS_DESTROYED, + GetWebContentsSource(navigation_controller_source)); +} + +content::NotificationSource + GoogleURLTrackerNavigationHelperImpl::GetWebContentsSource( + const content::NotificationSource& nav_controller_source) { + content::NavigationController* controller = + content::Source<content::NavigationController>( + nav_controller_source).ptr(); + content::WebContents* web_contents = controller->GetWebContents(); + return content::Source<content::WebContents>(web_contents); +} + +void GoogleURLTrackerNavigationHelperImpl::Observe( + int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) { + switch (type) { + case content::NOTIFICATION_NAV_ENTRY_PENDING: { + content::NavigationController* controller = + content::Source<content::NavigationController>(source).ptr(); + content::WebContents* web_contents = controller->GetWebContents(); + InfoBarService* infobar_service = + InfoBarService::FromWebContents(web_contents); + // Because we're listening to all sources, there may be no + // InfoBarService for some notifications, e.g. navigations in + // bubbles/balloons etc. + if (infobar_service) { + tracker_->OnNavigationPending( + controller, infobar_service, + controller->GetPendingEntry()->GetUniqueID()); + } + break; + } + + case content::NOTIFICATION_NAV_ENTRY_COMMITTED: { + content::NavigationController* controller = + content::Source<content::NavigationController>(source).ptr(); + // Here we're only listening to notifications where we already know + // there's an associated InfoBarService. + content::WebContents* web_contents = controller->GetWebContents(); + InfoBarService* infobar_service = + InfoBarService::FromWebContents(web_contents); + DCHECK(infobar_service); + const GURL& search_url = controller->GetActiveEntry()->GetURL(); + if (!search_url.is_valid()) // Not clear if this can happen. + tracker_->OnTabClosed(controller); + tracker_->OnNavigationCommitted(infobar_service, search_url); + break; + } + + case content::NOTIFICATION_WEB_CONTENTS_DESTROYED: { + content::WebContents* web_contents = + content::Source<content::WebContents>(source).ptr(); + tracker_->OnTabClosed(&web_contents->GetController()); + break; + } + + case chrome::NOTIFICATION_INSTANT_COMMITTED: { + content::WebContents* web_contents = + content::Source<content::WebContents>(source).ptr(); + content::NavigationController* nav_controller = + &web_contents->GetController(); + const GURL& search_url = web_contents->GetURL(); + if (!search_url.is_valid()) // Not clear if this can happen. + tracker_->OnTabClosed(nav_controller); + OnInstantCommitted(nav_controller, + InfoBarService::FromWebContents(web_contents), + search_url); + break; + } + + default: + NOTREACHED() << "Unknown notification received:" << type; + } +} + +void GoogleURLTrackerNavigationHelperImpl::OnInstantCommitted( + content::NavigationController* nav_controller, + InfoBarService* infobar_service, + const GURL& search_url) { + // Call OnNavigationPending, giving |tracker_| the option to register for + // navigation commit messages for this navigation controller. If it does + // register for them, simulate the commit as well. + tracker_->OnNavigationPending(nav_controller, infobar_service, 0); + if (IsListeningForNavigationCommit(nav_controller)) + tracker_->OnNavigationCommitted(infobar_service, search_url); +} diff --git a/chrome/browser/google/google_url_tracker_navigation_helper_impl.h b/chrome/browser/google/google_url_tracker_navigation_helper_impl.h new file mode 100644 index 0000000..a710b5e --- /dev/null +++ b/chrome/browser/google/google_url_tracker_navigation_helper_impl.h @@ -0,0 +1,58 @@ +// 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_NAVIGATION_HELPER_IMPL_H_ +#define CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_NAVIGATION_HELPER_IMPL_H_ + +#include "chrome/browser/google/google_url_tracker_navigation_helper.h" +#include "content/public/browser/notification_observer.h" +#include "content/public/browser/notification_registrar.h" +#include "googleurl/src/gurl.h" + +class GoogleURLTrackerNavigationHelperImpl + : public GoogleURLTrackerNavigationHelper, + public content::NotificationObserver { + public: + explicit GoogleURLTrackerNavigationHelperImpl(); + virtual ~GoogleURLTrackerNavigationHelperImpl(); + + // GoogleURLTrackerNavigationHelper. + virtual void SetGoogleURLTracker(GoogleURLTracker* tracker) OVERRIDE; + virtual void SetListeningForNavigationStart(bool listen) OVERRIDE; + virtual bool IsListeningForNavigationStart() OVERRIDE; + virtual void SetListeningForNavigationCommit( + const content::NavigationController* nav_controller, + bool listen) OVERRIDE; + virtual bool IsListeningForNavigationCommit( + const content::NavigationController* nav_controller) OVERRIDE; + virtual void SetListeningForTabDestruction( + const content::NavigationController* nav_controller, + bool listen) OVERRIDE; + virtual bool IsListeningForTabDestruction( + const content::NavigationController* nav_controller) OVERRIDE; + + private: + friend class GoogleURLTrackerNavigationHelperTest; + + // content::NotificationObserver: + virtual void Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) OVERRIDE; + + // Handles instant commit notifications by simulating the relevant navigation + // callbacks. + void OnInstantCommitted(content::NavigationController* nav_controller, + InfoBarService* infobar_service, + const GURL& search_url); + + // Returns a WebContents NavigationSource for the WebContents corresponding to + // the given NavigationController NotificationSource. + virtual content::NotificationSource GetWebContentsSource( + const content::NotificationSource& nav_controller_source); + + GoogleURLTracker* tracker_; + content::NotificationRegistrar registrar_; +}; + +#endif // CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_NAVIGATION_HELPER_IMPL_H_ diff --git a/chrome/browser/google/google_url_tracker_unittest.cc b/chrome/browser/google/google_url_tracker_unittest.cc index 8ccfc6c..1fd9365 100644 --- a/chrome/browser/google/google_url_tracker_unittest.cc +++ b/chrome/browser/google/google_url_tracker_unittest.cc @@ -12,6 +12,7 @@ #include "chrome/browser/api/infobars/infobar_delegate.h" #include "chrome/browser/google/google_url_tracker_factory.h" #include "chrome/browser/google/google_url_tracker_infobar_delegate.h" +#include "chrome/browser/google/google_url_tracker_navigation_helper.h" #include "chrome/browser/infobars/infobar.h" #include "chrome/common/chrome_notification_types.h" #include "chrome/common/pref_names.h" @@ -94,6 +95,89 @@ void TestNotificationObserver::Observe( notified_ = true; } + +// TestGoogleURLTrackerNavigationHelper ------------------------------------- + +class TestGoogleURLTrackerNavigationHelper + : public GoogleURLTrackerNavigationHelper { + public: + TestGoogleURLTrackerNavigationHelper(); + virtual ~TestGoogleURLTrackerNavigationHelper(); + + virtual void SetGoogleURLTracker(GoogleURLTracker* tracker) OVERRIDE; + virtual void SetListeningForNavigationStart(bool listen) OVERRIDE; + virtual bool IsListeningForNavigationStart() OVERRIDE; + virtual void SetListeningForNavigationCommit( + const content::NavigationController* nav_controller, + bool listen) OVERRIDE; + virtual bool IsListeningForNavigationCommit( + const content::NavigationController* nav_controller) OVERRIDE; + virtual void SetListeningForTabDestruction( + const content::NavigationController* nav_controller, + bool listen) OVERRIDE; + virtual bool IsListeningForTabDestruction( + const content::NavigationController* nav_controller) OVERRIDE; + + private: + GoogleURLTracker* tracker_; + bool observe_nav_start_; + std::set<const content::NavigationController*> + nav_controller_commit_listeners_; + std::set<const content::NavigationController*> + nav_controller_tab_close_listeners_; +}; + +TestGoogleURLTrackerNavigationHelper::TestGoogleURLTrackerNavigationHelper() + : tracker_(NULL), + observe_nav_start_(false) { +} + +TestGoogleURLTrackerNavigationHelper:: + ~TestGoogleURLTrackerNavigationHelper() { +} + +void TestGoogleURLTrackerNavigationHelper::SetGoogleURLTracker( + GoogleURLTracker* tracker) { + tracker_ = tracker; +} + +void TestGoogleURLTrackerNavigationHelper::SetListeningForNavigationStart( + bool listen) { + observe_nav_start_ = listen; +} + +bool TestGoogleURLTrackerNavigationHelper::IsListeningForNavigationStart() { + return observe_nav_start_; +} + +void TestGoogleURLTrackerNavigationHelper::SetListeningForNavigationCommit( + const content::NavigationController* nav_controller, + bool listen) { + if (listen) + nav_controller_commit_listeners_.insert(nav_controller); + else + nav_controller_commit_listeners_.erase(nav_controller); +} + +bool TestGoogleURLTrackerNavigationHelper::IsListeningForNavigationCommit( + const content::NavigationController* nav_controller) { + return nav_controller_commit_listeners_.count(nav_controller) > 0; +} + +void TestGoogleURLTrackerNavigationHelper::SetListeningForTabDestruction( + const content::NavigationController* nav_controller, + bool listen) { + if (listen) + nav_controller_tab_close_listeners_.insert(nav_controller); + else + nav_controller_tab_close_listeners_.erase(nav_controller); +} + +bool TestGoogleURLTrackerNavigationHelper::IsListeningForTabDestruction( + const content::NavigationController* nav_controller) { + return nav_controller_tab_close_listeners_.count(nav_controller) > 0; +} + } // namespace @@ -101,14 +185,13 @@ void TestNotificationObserver::Observe( // Ths class exercises GoogleURLTracker. In order to avoid instantiating more // of the Chrome infrastructure than necessary, the GoogleURLTracker functions -// are carefully written so that many of the functions which take WebContents*, -// NavigationController*, InfoBarService*, or objects containing such pointers -// (e.g. NotificationSource) do not actually dereference the objects, merely use -// them for comparisons and lookups, e.g. in |entry_map_|. This then allows the -// test code here to not create any of these objects, and instead supply -// "pointers" that are actually reinterpret_cast<>()ed magic numbers. Then we -// write the necessary stubs/hooks, here and in TestInfoBarDelegate above, to -// make everything continue to work. +// are carefully written so that many of the functions which take +// NavigationController* or InfoBarService* do not actually dereference the +// objects, merely use them for comparisons and lookups, e.g. in |entry_map_|. +// This then allows the test code here to not create any of these objects, and +// instead supply "pointers" that are actually reinterpret_cast<>()ed magic +// numbers. Then we write the necessary stubs/hooks, here and in +// TestInfoBarDelegate above, to make everything continue to work. // // Technically, the C++98 spec defines the result of casting // T* -> intptr_t -> T* to be an identity, but intptr_t -> T* -> intptr_t (what @@ -146,7 +229,6 @@ class GoogleURLTrackerTest : public testing::Test { void SetNavigationPending(intptr_t unique_id, bool is_search); void CommitNonSearch(intptr_t unique_id); void CommitSearch(intptr_t unique_id, const GURL& search_url); - void DoInstantNavigation(intptr_t unique_id, const GURL& search_url); void CloseTab(intptr_t unique_id); GoogleURLTrackerMapEntry* GetMapEntry(intptr_t unique_id); GoogleURLTrackerInfoBarDelegate* GetInfoBar(intptr_t unique_id); @@ -175,6 +257,7 @@ class GoogleURLTrackerTest : public testing::Test { net::TestURLFetcherFactory fetcher_factory_; content::NotificationRegistrar registrar_; TestNotificationObserver observer_; + GoogleURLTrackerNavigationHelper* nav_helper_; TestingProfile profile_; scoped_ptr<GoogleURLTracker> google_url_tracker_; // This tracks the different "tabs" a test has "opened", so we can close them @@ -210,8 +293,13 @@ GoogleURLTrackerTest::~GoogleURLTrackerTest() { void GoogleURLTrackerTest::SetUp() { network_change_notifier_.reset(net::NetworkChangeNotifier::CreateMock()); + // Ownership is passed to google_url_tracker_, but a weak pointer is kept; + // this is safe since GoogleURLTracker keeps the observer for its lifetime. + nav_helper_ = new TestGoogleURLTrackerNavigationHelper(); + scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper(nav_helper_); google_url_tracker_.reset( - new GoogleURLTracker(&profile_, GoogleURLTracker::UNIT_TEST_MODE)); + new GoogleURLTracker(&profile_, nav_helper.Pass(), + GoogleURLTracker::UNIT_TEST_MODE)); google_url_tracker_->infobar_creator_ = base::Bind( &GoogleURLTrackerTest::CreateTestInfoBar, base::Unretained(this)); } @@ -220,6 +308,7 @@ void GoogleURLTrackerTest::TearDown() { while (!unique_ids_seen_.empty()) CloseTab(*unique_ids_seen_.begin()); + nav_helper_ = NULL; google_url_tracker_.reset(); network_change_notifier_.reset(); } @@ -277,18 +366,12 @@ void GoogleURLTrackerTest::SetNavigationPending(intptr_t unique_id, 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. + // for navigation starts 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, - content::NotificationService::AllBrowserContextsAndSources())) { + if (nav_helper_->IsListeningForNavigationStart()) { google_url_tracker_->OnNavigationPending( - content::Source<content::NavigationController>( - reinterpret_cast<content::NavigationController*>(unique_id)), - content::Source<content::WebContents>( - reinterpret_cast<content::WebContents*>(unique_id)), + reinterpret_cast<content::NavigationController*>(unique_id), reinterpret_cast<InfoBarService*>(unique_id), unique_id); } } @@ -315,45 +398,20 @@ void GoogleURLTrackerTest::CommitNonSearch(intptr_t unique_id) { void GoogleURLTrackerTest::CommitSearch(intptr_t unique_id, const GURL& search_url) { DCHECK(search_url.is_valid()); - if (google_url_tracker_->registrar_.IsRegistered(google_url_tracker_.get(), - content::NOTIFICATION_NAV_ENTRY_COMMITTED, - content::Source<content::NavigationController>( - reinterpret_cast<content::NavigationController*>(unique_id)))) { + if (nav_helper_->IsListeningForNavigationCommit( + reinterpret_cast<content::NavigationController*>(unique_id))) { google_url_tracker_->OnNavigationCommitted( reinterpret_cast<InfoBarService*>(unique_id), search_url); } } -void GoogleURLTrackerTest::DoInstantNavigation(intptr_t 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<content::WebContents>( - reinterpret_cast<content::WebContents*>(unique_id)), - reinterpret_cast<InfoBarService*>(unique_id), search_url); - } -} - void GoogleURLTrackerTest::CloseTab(intptr_t unique_id) { unique_ids_seen_.erase(unique_id); - content::Source<content::WebContents> source( - reinterpret_cast<content::WebContents*>(unique_id)); - if (google_url_tracker_->registrar_.IsRegistered( - google_url_tracker_.get(), content::NOTIFICATION_WEB_CONTENTS_DESTROYED, - source)) { - google_url_tracker_->OnTabClosed(source); + content::NavigationController* nav_controller = + reinterpret_cast<content::NavigationController*>(unique_id); + if (nav_helper_->IsListeningForTabDestruction(nav_controller)) { + google_url_tracker_->OnTabClosed(nav_controller); } else { // Closing a tab with an infobar showing would close the infobar. GoogleURLTrackerInfoBarDelegate* infobar = GetInfoBar(unique_id); @@ -385,9 +443,8 @@ void GoogleURLTrackerTest::ExpectListeningForCommit(intptr_t unique_id, bool listening) { GoogleURLTrackerMapEntry* map_entry = GetMapEntry(unique_id); if (map_entry) { - EXPECT_EQ(listening, google_url_tracker_->registrar_.IsRegistered( - google_url_tracker_.get(), content::NOTIFICATION_NAV_ENTRY_COMMITTED, - map_entry->navigation_controller_source())); + EXPECT_EQ(listening, nav_helper_->IsListeningForNavigationCommit( + map_entry->navigation_controller())); } else { EXPECT_FALSE(listening); } @@ -764,16 +821,6 @@ 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")); - EXPECT_FALSE(GetInfoBar(1) == NULL); -} - TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfoBars) { RequestServerCheck(); FinishSleep(); @@ -1016,26 +1063,6 @@ TEST_F(GoogleURLTrackerTest, MultipleMapEntries) { 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); - EXPECT_FALSE(GetMapEntry(1) == NULL); - ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true)); - ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(2, false)); - - DoInstantNavigation(2, GURL()); - EXPECT_TRUE(GetMapEntry(2) == 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(); diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 05cdf02..d413929 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -668,6 +668,9 @@ 'browser/google/google_url_tracker_infobar_delegate.h', 'browser/google/google_url_tracker_map_entry.cc', 'browser/google/google_url_tracker_map_entry.h', + 'browser/google/google_url_tracker_navigation_helper.h', + 'browser/google/google_url_tracker_navigation_helper_impl.cc', + 'browser/google/google_url_tracker_navigation_helper_impl.h', 'browser/google/google_util.cc', 'browser/google/google_util.h', 'browser/google/google_util_chromeos.cc', |