summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorblundell@chromium.org <blundell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-05-29 16:32:10 +0000
committerblundell@chromium.org <blundell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-05-29 16:32:10 +0000
commit57f12776e5aa51ab0b60cd31b12f0acbe3d97207 (patch)
treef59b17eca74e9542f64aa079952ecd9f65ce0706
parent8f783d09bc5d77822300ddad9297d0acd3461835 (diff)
downloadchromium_src-57f12776e5aa51ab0b60cd31b12f0acbe3d97207.zip
chromium_src-57f12776e5aa51ab0b60cd31b12f0acbe3d97207.tar.gz
chromium_src-57f12776e5aa51ab0b60cd31b12f0acbe3d97207.tar.bz2
Turn GoogleURLTrackerNavigationHelper(Impl) into a per-tab object.
The goal of this CL is to eliminate the dependence of GoogleURLTracker(MapEntry) on NavigationController. To accomplish this goal, GoogleURLTrackerNavigationHelper is turned into a conceptually per-tab interface. GoogleURLTracker::OnNavigationPending() now takes in the GoogleURLTrackerNavigationHelper with which the navigation is associated rather than the associated NavigationController; GoogleURLTracker performs per-tab actions by calling the navigation helper associated with the tab. A followup CL will turn GoogleURLTrackerNavigationHelper(Impl) into the GoogleURLTrackerDriver interface and ContentURLTrackerDriver implementation. BUG=373230 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272094 Review URL: https://codereview.chromium.org/283413002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273490 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/google/chrome_google_url_tracker_client.cc10
-rw-r--r--chrome/browser/google/google_url_tracker.cc86
-rw-r--r--chrome/browser/google/google_url_tracker.h24
-rw-r--r--chrome/browser/google/google_url_tracker_factory.cc4
-rw-r--r--chrome/browser/google/google_url_tracker_map_entry.cc4
-rw-r--r--chrome/browser/google/google_url_tracker_map_entry.h14
-rw-r--r--chrome/browser/google/google_url_tracker_navigation_helper.cc13
-rw-r--r--chrome/browser/google/google_url_tracker_navigation_helper.h56
-rw-r--r--chrome/browser/google/google_url_tracker_navigation_helper_impl.cc73
-rw-r--r--chrome/browser/google/google_url_tracker_navigation_helper_impl.h27
-rw-r--r--chrome/browser/google/google_url_tracker_unittest.cc108
-rw-r--r--chrome/chrome_browser.gypi1
12 files changed, 188 insertions, 232 deletions
diff --git a/chrome/browser/google/chrome_google_url_tracker_client.cc b/chrome/browser/google/chrome_google_url_tracker_client.cc
index 2d8a1e5..5416669 100644
--- a/chrome/browser/google/chrome_google_url_tracker_client.cc
+++ b/chrome/browser/google/chrome_google_url_tracker_client.cc
@@ -6,6 +6,7 @@
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/google/google_url_tracker.h"
+#include "chrome/browser/google/google_url_tracker_navigation_helper_impl.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
@@ -48,12 +49,13 @@ void ChromeGoogleURLTrackerClient::Observe(
content::Source<content::NavigationController>(source).ptr();
InfoBarService* infobar_service =
InfoBarService::FromWebContents(controller->GetWebContents());
- // Because we're listening to all sources, there may be no
- // InfoBarService for some notifications, e.g. navigations in
- // bubbles/balloons etc.
+ // 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) {
google_url_tracker()->OnNavigationPending(
- controller,
+ scoped_ptr<GoogleURLTrackerNavigationHelper>(
+ new GoogleURLTrackerNavigationHelperImpl(
+ controller->GetWebContents(), google_url_tracker())),
infobar_service,
controller->GetPendingEntry()->GetUniqueID());
}
diff --git a/chrome/browser/google/google_url_tracker.cc b/chrome/browser/google/google_url_tracker.cc
index 62daab0..4983560 100644
--- a/chrome/browser/google/google_url_tracker.cc
+++ b/chrome/browser/google/google_url_tracker.cc
@@ -33,14 +33,11 @@ const char GoogleURLTracker::kDefaultGoogleHomepage[] =
const char GoogleURLTracker::kSearchDomainCheckURL[] =
"https://www.google.com/searchdomaincheck?format=url&type=chrome";
-GoogleURLTracker::GoogleURLTracker(
- Profile* profile,
- scoped_ptr<GoogleURLTrackerClient> client,
- scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper,
- Mode mode)
+GoogleURLTracker::GoogleURLTracker(Profile* profile,
+ scoped_ptr<GoogleURLTrackerClient> client,
+ Mode mode)
: profile_(profile),
client_(client.Pass()),
- nav_helper_(nav_helper.Pass()),
infobar_creator_(base::Bind(&GoogleURLTrackerInfoBarDelegate::Create)),
google_url_(mode == UNIT_TEST_MODE ?
kDefaultGoogleHomepage :
@@ -54,7 +51,6 @@ GoogleURLTracker::GoogleURLTracker(
weak_ptr_factory_(this) {
net::NetworkChangeNotifier::AddIPAddressObserver(this);
client_->set_google_url_tracker(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
@@ -205,7 +201,6 @@ void GoogleURLTracker::OnIPAddressChanged() {
void GoogleURLTracker::Shutdown() {
client_.reset();
- nav_helper_.reset();
fetcher_.reset();
weak_ptr_factory_.InvalidateWeakPtrs();
net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
@@ -219,7 +214,7 @@ void GoogleURLTracker::DeleteMapEntryForService(
DCHECK(i != entry_map_.end());
GoogleURLTrackerMapEntry* map_entry = i->second;
- UnregisterForEntrySpecificNotifications(*map_entry, false);
+ UnregisterForEntrySpecificNotifications(map_entry, false);
entry_map_.erase(i);
delete map_entry;
}
@@ -281,39 +276,41 @@ void GoogleURLTracker::SearchCommitted() {
}
void GoogleURLTracker::OnNavigationPending(
- content::NavigationController* navigation_controller,
+ scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper,
InfoBarService* infobar_service,
int pending_id) {
+ GoogleURLTrackerMapEntry* map_entry = NULL;
+
EntryMap::iterator i(entry_map_.find(infobar_service));
+ if (i != entry_map_.end())
+ map_entry = i->second;
if (search_committed_) {
search_committed_ = false;
- // Whether there's an existing infobar or not, we need to listen for the
- // 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 (!nav_helper_->IsListeningForNavigationCommit(navigation_controller)) {
- nav_helper_->SetListeningForNavigationCommit(navigation_controller,
- true);
- }
- if (i == entry_map_.end()) {
+ if (!map_entry) {
// This is a search on a tab that doesn't have one of our infobars, so
// prepare to add one. Note that we only listen for the tab's destruction
// on this path; if there was already a map entry, then either it doesn't
// 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.
- nav_helper_->SetListeningForTabDestruction(navigation_controller, true);
- entry_map_.insert(std::make_pair(
- infobar_service,
- new GoogleURLTrackerMapEntry(this, infobar_service,
- navigation_controller)));
- } else if (i->second->has_infobar_delegate()) {
+ map_entry = new GoogleURLTrackerMapEntry(
+ this, infobar_service, nav_helper.Pass());
+ map_entry->navigation_helper()->SetListeningForTabDestruction(true);
+ entry_map_.insert(std::make_pair(infobar_service, map_entry));
+ } else if (map_entry->infobar_delegate()) {
// This is a new search on a tab where we already have an infobar.
- i->second->infobar_delegate()->set_pending_id(pending_id);
+ map_entry->infobar_delegate()->set_pending_id(pending_id);
}
- } else if (i != entry_map_.end()){
- if (i->second->has_infobar_delegate()) {
+
+ // Whether there's an existing infobar or not, we need to listen for the
+ // 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 (!map_entry->navigation_helper()->IsListeningForNavigationCommit())
+ map_entry->navigation_helper()->SetListeningForNavigationCommit(true);
+ } else if (map_entry) {
+ if (map_entry->has_infobar_delegate()) {
// This is a non-search navigation on a tab with an infobar. If there was
// a previous pending search on this tab, this means it won't commit, so
// undo anything we did in response to seeing that. Note that if there
@@ -323,13 +320,13 @@ void GoogleURLTracker::OnNavigationPending(
// If this navigation actually commits, that will trigger the infobar's
// owner to expire the infobar if need be. If it doesn't commit, then
// simply leaving the infobar as-is will have been the right thing.
- UnregisterForEntrySpecificNotifications(*i->second, false);
- i->second->infobar_delegate()->set_pending_id(0);
+ UnregisterForEntrySpecificNotifications(map_entry, false);
+ map_entry->infobar_delegate()->set_pending_id(0);
} else {
// Non-search navigation on a tab with an entry that has not yet created
// an infobar. This means the original search won't commit, so delete the
// entry.
- i->second->Close(false);
+ map_entry->Close(false);
}
} else {
// Non-search navigation on a tab without an infobars. This is irrelevant
@@ -344,7 +341,7 @@ void GoogleURLTracker::OnNavigationCommitted(InfoBarService* infobar_service,
GoogleURLTrackerMapEntry* map_entry = i->second;
DCHECK(search_url.is_valid());
- UnregisterForEntrySpecificNotifications(*map_entry, true);
+ UnregisterForEntrySpecificNotifications(map_entry, true);
if (map_entry->has_infobar_delegate()) {
map_entry->infobar_delegate()->Update(search_url);
} else {
@@ -360,7 +357,7 @@ void GoogleURLTracker::OnNavigationCommitted(InfoBarService* infobar_service,
}
void GoogleURLTracker::OnTabClosed(
- content::NavigationController* navigation_controller) {
+ GoogleURLTrackerNavigationHelper* nav_helper) {
// Because InfoBarService tears itself down on tab destruction, it's 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
@@ -370,7 +367,7 @@ void GoogleURLTracker::OnTabClosed(
// 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->navigation_controller() == navigation_controller) {
+ if (i->second->navigation_helper() == nav_helper) {
i->second->Close(false);
return;
}
@@ -390,26 +387,22 @@ void GoogleURLTracker::CloseAllEntries(bool redo_searches) {
}
void GoogleURLTracker::UnregisterForEntrySpecificNotifications(
- const GoogleURLTrackerMapEntry& map_entry,
+ GoogleURLTrackerMapEntry* map_entry,
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 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);
+ if (map_entry->navigation_helper()->IsListeningForNavigationCommit()) {
+ map_entry->navigation_helper()->SetListeningForNavigationCommit(false);
} else {
DCHECK(!must_be_listening_for_commit);
- DCHECK(map_entry.has_infobar_delegate());
+ DCHECK(map_entry->has_infobar_delegate());
}
const bool registered_for_tab_destruction =
- nav_helper_->IsListeningForTabDestruction(
- map_entry.navigation_controller());
- DCHECK_NE(registered_for_tab_destruction, map_entry.has_infobar_delegate());
+ map_entry->navigation_helper()->IsListeningForTabDestruction();
+ DCHECK_NE(registered_for_tab_destruction, map_entry->has_infobar_delegate());
if (registered_for_tab_destruction) {
- nav_helper_->SetListeningForTabDestruction(
- map_entry.navigation_controller(), false);
+ map_entry->navigation_helper()->SetListeningForTabDestruction(false);
}
// Our global listeners for these other notifications should be in place iff
@@ -419,8 +412,7 @@ void GoogleURLTracker::UnregisterForEntrySpecificNotifications(
// See the various cases inside OnNavigationPending().
for (EntryMap::const_iterator i(entry_map_.begin()); i != entry_map_.end();
++i) {
- if (nav_helper_->IsListeningForNavigationCommit(
- i->second->navigation_controller())) {
+ if (i->second->navigation_helper()->IsListeningForNavigationCommit()) {
DCHECK(client_->IsListeningForNavigationStart());
return;
}
diff --git a/chrome/browser/google/google_url_tracker.h b/chrome/browser/google/google_url_tracker.h
index 5e8ba1a..5d1d612 100644
--- a/chrome/browser/google/google_url_tracker.h
+++ b/chrome/browser/google/google_url_tracker.h
@@ -26,10 +26,6 @@ class GoogleURLTrackerNavigationHelper;
class PrefService;
class Profile;
-namespace content {
-class NavigationController;
-}
-
namespace infobars {
class InfoBar;
}
@@ -72,7 +68,6 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
// GoogleURLTrackerFactory::GetForProfile().
GoogleURLTracker(Profile* profile,
scoped_ptr<GoogleURLTrackerClient> client,
- scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper,
Mode mode);
virtual ~GoogleURLTracker();
@@ -113,14 +108,14 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
// Called by the client 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
+ // |nav_helper| is the GoogleURLTrackerNavigationHelper associated with this
+ // navigation; |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 map entry for the associated tab.
virtual void OnNavigationPending(
- content::NavigationController* navigation_controller,
+ scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper,
InfoBarService* infobar_service,
int pending_id);
@@ -131,8 +126,7 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
const GURL& search_url);
// Called by the navigation observer when a tab closes.
- virtual void OnTabClosed(
- content::NavigationController* navigation_controller);
+ virtual void OnTabClosed(GoogleURLTrackerNavigationHelper* nav_helper);
scoped_ptr<Subscription> RegisterCallback(
const OnGoogleURLUpdatedCallback& cb);
@@ -174,14 +168,14 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
// Google TLD.
void CloseAllEntries(bool redo_searches);
- // Unregisters any listeners for the navigation controller in |map_entry|.
+ // Unregisters any listeners for the navigation helper 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 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,
+ GoogleURLTrackerMapEntry* map_entry,
bool must_be_listening_for_commit);
void NotifyGoogleURLUpdated(GURL old_url, GURL new_url);
@@ -192,8 +186,6 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
scoped_ptr<GoogleURLTrackerClient> client_;
- scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper_;
-
// Creates an infobar and adds it to the provided InfoBarService. Returns the
// infobar on success or NULL on failure. The caller does not own the
// returned object, the InfoBarService does.
diff --git a/chrome/browser/google/google_url_tracker_factory.cc b/chrome/browser/google/google_url_tracker_factory.cc
index 239eabc..6c45c85 100644
--- a/chrome/browser/google/google_url_tracker_factory.cc
+++ b/chrome/browser/google/google_url_tracker_factory.cc
@@ -7,7 +7,6 @@
#include "base/prefs/pref_service.h"
#include "chrome/browser/google/chrome_google_url_tracker_client.h"
#include "chrome/browser/google/google_url_tracker.h"
-#include "chrome/browser/google/google_url_tracker_navigation_helper_impl.h"
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/pref_names.h"
@@ -38,11 +37,8 @@ GoogleURLTrackerFactory::~GoogleURLTrackerFactory() {
KeyedService* GoogleURLTrackerFactory::BuildServiceInstanceFor(
content::BrowserContext* profile) const {
scoped_ptr<GoogleURLTrackerClient> client(new ChromeGoogleURLTrackerClient());
- scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper(
- new GoogleURLTrackerNavigationHelperImpl());
return new GoogleURLTracker(static_cast<Profile*>(profile),
client.Pass(),
- nav_helper.Pass(),
GoogleURLTracker::NORMAL_MODE);
}
diff --git a/chrome/browser/google/google_url_tracker_map_entry.cc b/chrome/browser/google/google_url_tracker_map_entry.cc
index e0798fd..ee1ee0f 100644
--- a/chrome/browser/google/google_url_tracker_map_entry.cc
+++ b/chrome/browser/google/google_url_tracker_map_entry.cc
@@ -15,11 +15,11 @@
GoogleURLTrackerMapEntry::GoogleURLTrackerMapEntry(
GoogleURLTracker* google_url_tracker,
InfoBarService* infobar_service,
- const content::NavigationController* navigation_controller)
+ scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper)
: google_url_tracker_(google_url_tracker),
infobar_service_(infobar_service),
infobar_delegate_(NULL),
- navigation_controller_(navigation_controller) {
+ navigation_helper_(navigation_helper.Pass()) {
}
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 2e035a7..5f087fb 100644
--- a/chrome/browser/google/google_url_tracker_map_entry.h
+++ b/chrome/browser/google/google_url_tracker_map_entry.h
@@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_MAP_ENTRY_H_
#define CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_MAP_ENTRY_H_
+#include "base/memory/scoped_ptr.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"
@@ -12,16 +14,12 @@ 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::NavigationController* navigation_controller);
+ scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper);
virtual ~GoogleURLTrackerMapEntry();
bool has_infobar_delegate() const { return !!infobar_delegate_; }
@@ -30,8 +28,8 @@ class GoogleURLTrackerMapEntry : public content::NotificationObserver {
}
void SetInfoBarDelegate(GoogleURLTrackerInfoBarDelegate* infobar_delegate);
- const content::NavigationController* navigation_controller() const {
- return navigation_controller_;
+ GoogleURLTrackerNavigationHelper* navigation_helper() {
+ return navigation_helper_.get();
}
void Close(bool redo_search);
@@ -48,7 +46,7 @@ class GoogleURLTrackerMapEntry : public content::NotificationObserver {
GoogleURLTracker* const google_url_tracker_;
const InfoBarService* const infobar_service_;
GoogleURLTrackerInfoBarDelegate* infobar_delegate_;
- const content::NavigationController* const navigation_controller_;
+ scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper_;
DISALLOW_COPY_AND_ASSIGN(GoogleURLTrackerMapEntry);
};
diff --git a/chrome/browser/google/google_url_tracker_navigation_helper.cc b/chrome/browser/google/google_url_tracker_navigation_helper.cc
new file mode 100644
index 0000000..84a2651
--- /dev/null
+++ b/chrome/browser/google/google_url_tracker_navigation_helper.cc
@@ -0,0 +1,13 @@
+// Copyright 2014 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.h"
+
+GoogleURLTrackerNavigationHelper::GoogleURLTrackerNavigationHelper(
+ GoogleURLTracker* google_url_tracker)
+ : google_url_tracker_(google_url_tracker) {
+}
+
+GoogleURLTrackerNavigationHelper::~GoogleURLTrackerNavigationHelper() {
+}
diff --git a/chrome/browser/google/google_url_tracker_navigation_helper.h b/chrome/browser/google/google_url_tracker_navigation_helper.h
index 26b7a53..98804ce 100644
--- a/chrome/browser/google/google_url_tracker_navigation_helper.h
+++ b/chrome/browser/google/google_url_tracker_navigation_helper.h
@@ -5,46 +5,42 @@
#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;
+#include "base/macros.h"
-namespace content {
-class NavigationController;
-}
+class GoogleURLTracker;
// A helper class for GoogleURLTracker that abstracts the details of listening
// for various navigation events.
class GoogleURLTrackerNavigationHelper {
public:
- virtual ~GoogleURLTrackerNavigationHelper() {}
+ explicit GoogleURLTrackerNavigationHelper(
+ GoogleURLTracker* google_url_tracker);
+ virtual ~GoogleURLTrackerNavigationHelper();
- // Sets the GoogleURLTracker that is associated with this object.
- virtual void SetGoogleURLTracker(GoogleURLTracker* tracker) = 0;
+ // Enables or disables listening for navigation commits.
+ // OnNavigationCommitted will be called for each navigation commit if
+ // listening is enabled.
+ virtual void SetListeningForNavigationCommit(bool listen) = 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 this object is currently listening for navigation
+ // commits.
+ virtual bool IsListeningForNavigationCommit() = 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. OnTabClosed will be
+ // called on tab destruction if listening is enabled.
+ virtual void SetListeningForTabDestruction(bool listen) = 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;
+ // Returns whether or not this object is currently listening for tab
+ // destruction.
+ virtual bool IsListeningForTabDestruction() = 0;
+
+ protected:
+ GoogleURLTracker* google_url_tracker() { return google_url_tracker_; }
+
+ private:
+ GoogleURLTracker* google_url_tracker_;
+
+ DISALLOW_COPY_AND_ASSIGN(GoogleURLTrackerNavigationHelper);
};
#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
index 64e4c5c..99420a8 100644
--- a/chrome/browser/google/google_url_tracker_navigation_helper_impl.cc
+++ b/chrome/browser/google/google_url_tracker_navigation_helper_impl.cc
@@ -12,25 +12,21 @@
#include "content/public/browser/notification_service.h"
#include "content/public/browser/web_contents.h"
-GoogleURLTrackerNavigationHelperImpl::
- GoogleURLTrackerNavigationHelperImpl() : tracker_(NULL) {
+GoogleURLTrackerNavigationHelperImpl::GoogleURLTrackerNavigationHelperImpl(
+ content::WebContents* web_contents,
+ GoogleURLTracker* tracker)
+ : GoogleURLTrackerNavigationHelper(tracker),
+ web_contents_(web_contents) {
}
-GoogleURLTrackerNavigationHelperImpl::
- ~GoogleURLTrackerNavigationHelperImpl() {
-}
-
-void GoogleURLTrackerNavigationHelperImpl::SetGoogleURLTracker(
- GoogleURLTracker* tracker) {
- DCHECK(tracker);
- tracker_ = tracker;
+GoogleURLTrackerNavigationHelperImpl::~GoogleURLTrackerNavigationHelperImpl() {
}
void GoogleURLTrackerNavigationHelperImpl::SetListeningForNavigationCommit(
- const content::NavigationController* nav_controller,
bool listen) {
content::NotificationSource navigation_controller_source =
- content::Source<content::NavigationController>(nav_controller);
+ content::Source<content::NavigationController>(
+ &web_contents_->GetController());
if (listen) {
registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
navigation_controller_source);
@@ -40,47 +36,35 @@ void GoogleURLTrackerNavigationHelperImpl::SetListeningForNavigationCommit(
}
}
-bool GoogleURLTrackerNavigationHelperImpl::IsListeningForNavigationCommit(
- const content::NavigationController* nav_controller) {
+bool GoogleURLTrackerNavigationHelperImpl::IsListeningForNavigationCommit() {
content::NotificationSource navigation_controller_source =
- content::Source<content::NavigationController>(nav_controller);
+ content::Source<content::NavigationController>(
+ &web_contents_->GetController());
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);
+ content::NotificationSource web_contents_source =
+ content::Source<content::WebContents>(web_contents_);
if (listen) {
- registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
- GetWebContentsSource(navigation_controller_source));
+ registrar_.Add(this,
+ content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
+ web_contents_source);
} else {
- registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
- GetWebContentsSource(navigation_controller_source));
+ registrar_.Remove(this,
+ content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
+ web_contents_source);
}
}
-bool GoogleURLTrackerNavigationHelperImpl::IsListeningForTabDestruction(
- const content::NavigationController* nav_controller) {
- content::NotificationSource navigation_controller_source =
- content::Source<content::NavigationController>(nav_controller);
+bool GoogleURLTrackerNavigationHelperImpl::IsListeningForTabDestruction() {
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);
+ content::Source<content::WebContents>(web_contents_));
}
void GoogleURLTrackerNavigationHelperImpl::Observe(
@@ -91,23 +75,24 @@ void GoogleURLTrackerNavigationHelperImpl::Observe(
case content::NOTIFICATION_NAV_ENTRY_COMMITTED: {
content::NavigationController* controller =
content::Source<content::NavigationController>(source).ptr();
+ DCHECK_EQ(web_contents_, controller->GetWebContents());
+
// 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);
+ 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);
+ google_url_tracker()->OnTabClosed(this);
+ google_url_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());
+ DCHECK_EQ(web_contents_,
+ content::Source<content::WebContents>(source).ptr());
+ google_url_tracker()->OnTabClosed(this);
break;
}
diff --git a/chrome/browser/google/google_url_tracker_navigation_helper_impl.h b/chrome/browser/google/google_url_tracker_navigation_helper_impl.h
index b024744..1d81e54 100644
--- a/chrome/browser/google/google_url_tracker_navigation_helper_impl.h
+++ b/chrome/browser/google/google_url_tracker_navigation_helper_impl.h
@@ -10,25 +10,25 @@
#include "content/public/browser/notification_registrar.h"
#include "url/gurl.h"
+namespace content {
+class WebContents;
+}
+
class GoogleURLTrackerNavigationHelperImpl
: public GoogleURLTrackerNavigationHelper,
public content::NotificationObserver {
public:
- explicit GoogleURLTrackerNavigationHelperImpl();
+ GoogleURLTrackerNavigationHelperImpl(content::WebContents* web_contents,
+ GoogleURLTracker* tracker);
virtual ~GoogleURLTrackerNavigationHelperImpl();
- // GoogleURLTrackerNavigationHelper.
- virtual void SetGoogleURLTracker(GoogleURLTracker* tracker) OVERRIDE;
+ // GoogleURLTrackerNavigationHelper:
virtual void SetListeningForNavigationCommit(
- const content::NavigationController* nav_controller,
bool listen) OVERRIDE;
- virtual bool IsListeningForNavigationCommit(
- const content::NavigationController* nav_controller) OVERRIDE;
+ virtual bool IsListeningForNavigationCommit() OVERRIDE;
virtual void SetListeningForTabDestruction(
- const content::NavigationController* nav_controller,
bool listen) OVERRIDE;
- virtual bool IsListeningForTabDestruction(
- const content::NavigationController* nav_controller) OVERRIDE;
+ virtual bool IsListeningForTabDestruction() OVERRIDE;
private:
// content::NotificationObserver:
@@ -36,13 +36,10 @@ class GoogleURLTrackerNavigationHelperImpl
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;
- // 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::WebContents* web_contents_;
content::NotificationRegistrar registrar_;
+
+ DISALLOW_COPY_AND_ASSIGN(GoogleURLTrackerNavigationHelperImpl);
};
#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 6b2fedf..0fb975a 100644
--- a/chrome/browser/google/google_url_tracker_unittest.cc
+++ b/chrome/browser/google/google_url_tracker_unittest.cc
@@ -137,73 +137,53 @@ bool TestGoogleURLTrackerClient::IsListeningForNavigationStart() {
return observe_nav_start_;
}
+
// TestGoogleURLTrackerNavigationHelper ---------------------------------------
class TestGoogleURLTrackerNavigationHelper
: public GoogleURLTrackerNavigationHelper {
public:
- TestGoogleURLTrackerNavigationHelper();
+ explicit TestGoogleURLTrackerNavigationHelper(GoogleURLTracker* tracker);
virtual ~TestGoogleURLTrackerNavigationHelper();
- virtual void SetGoogleURLTracker(GoogleURLTracker* tracker) 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;
+ virtual void SetListeningForNavigationCommit(bool listen) OVERRIDE;
+ virtual bool IsListeningForNavigationCommit() OVERRIDE;
+ virtual void SetListeningForTabDestruction(bool listen) OVERRIDE;
+ virtual bool IsListeningForTabDestruction() OVERRIDE;
private:
- GoogleURLTracker* tracker_;
- std::set<const content::NavigationController*>
- nav_controller_commit_listeners_;
- std::set<const content::NavigationController*>
- nav_controller_tab_close_listeners_;
-};
+ bool listening_for_nav_commit_;
+ bool listening_for_tab_destruction_;
-TestGoogleURLTrackerNavigationHelper::TestGoogleURLTrackerNavigationHelper()
- : tracker_(NULL) {
-}
+ DISALLOW_COPY_AND_ASSIGN(TestGoogleURLTrackerNavigationHelper);
+};
-TestGoogleURLTrackerNavigationHelper::
- ~TestGoogleURLTrackerNavigationHelper() {
+TestGoogleURLTrackerNavigationHelper::TestGoogleURLTrackerNavigationHelper(
+ GoogleURLTracker* tracker)
+ : GoogleURLTrackerNavigationHelper(tracker),
+ listening_for_nav_commit_(false),
+ listening_for_tab_destruction_(false) {
}
-void TestGoogleURLTrackerNavigationHelper::SetGoogleURLTracker(
- GoogleURLTracker* tracker) {
- tracker_ = tracker;
+TestGoogleURLTrackerNavigationHelper::~TestGoogleURLTrackerNavigationHelper() {
}
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);
+ listening_for_nav_commit_ = listen;
}
-bool TestGoogleURLTrackerNavigationHelper::IsListeningForNavigationCommit(
- const content::NavigationController* nav_controller) {
- return nav_controller_commit_listeners_.count(nav_controller) > 0;
+bool TestGoogleURLTrackerNavigationHelper::IsListeningForNavigationCommit() {
+ return listening_for_nav_commit_;
}
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);
+ listening_for_tab_destruction_ = listen;
}
-bool TestGoogleURLTrackerNavigationHelper::IsListeningForTabDestruction(
- const content::NavigationController* nav_controller) {
- return nav_controller_tab_close_listeners_.count(nav_controller) > 0;
+bool TestGoogleURLTrackerNavigationHelper::IsListeningForTabDestruction() {
+ return listening_for_tab_destruction_;
}
} // namespace
@@ -260,6 +240,7 @@ class GoogleURLTrackerTest : public testing::Test {
void CloseTab(intptr_t unique_id);
GoogleURLTrackerMapEntry* GetMapEntry(intptr_t unique_id);
GoogleURLTrackerInfoBarDelegate* GetInfoBarDelegate(intptr_t unique_id);
+ GoogleURLTrackerNavigationHelper* GetNavigationHelper(intptr_t unique_id);
void ExpectDefaultURLs() const;
void ExpectListeningForCommit(intptr_t unique_id, bool listening);
bool listener_notified() const { return listener_.notified(); }
@@ -282,7 +263,6 @@ class GoogleURLTrackerTest : public testing::Test {
scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_;
net::TestURLFetcherFactory fetcher_factory_;
GoogleURLTrackerClient* client_;
- GoogleURLTrackerNavigationHelper* nav_helper_;
TestingProfile profile_;
scoped_ptr<GoogleURLTracker> google_url_tracker_;
TestCallbackListener listener_;
@@ -321,17 +301,12 @@ GoogleURLTrackerTest::~GoogleURLTrackerTest() {
void GoogleURLTrackerTest::SetUp() {
network_change_notifier_.reset(net::NetworkChangeNotifier::CreateMock());
- // Ownership is passed to google_url_tracker_, but weak pointers are kept;
- // this is safe since GoogleURLTracker keeps these objects for its lifetime.
+ // Ownership is passed to google_url_tracker_, but a weak pointer is kept;
+ // this is safe since GoogleURLTracker keeps the client for its lifetime.
client_ = new TestGoogleURLTrackerClient();
- nav_helper_ = new TestGoogleURLTrackerNavigationHelper();
scoped_ptr<GoogleURLTrackerClient> client(client_);
- scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper(nav_helper_);
- google_url_tracker_.reset(
- new GoogleURLTracker(&profile_,
- client.Pass(),
- nav_helper.Pass(),
- GoogleURLTracker::UNIT_TEST_MODE));
+ google_url_tracker_.reset(new GoogleURLTracker(
+ &profile_, client.Pass(), GoogleURLTracker::UNIT_TEST_MODE));
google_url_tracker_->infobar_creator_ = base::Bind(
&GoogleURLTrackerTest::CreateTestInfoBar, base::Unretained(this));
}
@@ -339,6 +314,7 @@ void GoogleURLTrackerTest::SetUp() {
void GoogleURLTrackerTest::TearDown() {
while (!unique_ids_seen_.empty())
CloseTab(*unique_ids_seen_.begin());
+ google_url_tracker_->Shutdown();
}
net::TestURLFetcher* GoogleURLTrackerTest::GetFetcher() {
@@ -395,8 +371,11 @@ void GoogleURLTrackerTest::SetNavigationPending(intptr_t unique_id,
unique_ids_seen_.insert(unique_id);
if (client_->IsListeningForNavigationStart()) {
google_url_tracker_->OnNavigationPending(
- reinterpret_cast<content::NavigationController*>(unique_id),
- reinterpret_cast<InfoBarService*>(unique_id), unique_id);
+ scoped_ptr<GoogleURLTrackerNavigationHelper>(
+ new TestGoogleURLTrackerNavigationHelper(
+ google_url_tracker_.get())),
+ reinterpret_cast<InfoBarService*>(unique_id),
+ unique_id);
}
}
@@ -422,8 +401,8 @@ void GoogleURLTrackerTest::CommitNonSearch(intptr_t unique_id) {
void GoogleURLTrackerTest::CommitSearch(intptr_t unique_id,
const GURL& search_url) {
DCHECK(search_url.is_valid());
- if (nav_helper_->IsListeningForNavigationCommit(
- reinterpret_cast<content::NavigationController*>(unique_id))) {
+ GoogleURLTrackerNavigationHelper* nav_helper = GetNavigationHelper(unique_id);
+ if (nav_helper && nav_helper->IsListeningForNavigationCommit()) {
google_url_tracker_->OnNavigationCommitted(
reinterpret_cast<InfoBarService*>(unique_id), search_url);
}
@@ -431,10 +410,9 @@ void GoogleURLTrackerTest::CommitSearch(intptr_t unique_id,
void GoogleURLTrackerTest::CloseTab(intptr_t unique_id) {
unique_ids_seen_.erase(unique_id);
- content::NavigationController* nav_controller =
- reinterpret_cast<content::NavigationController*>(unique_id);
- if (nav_helper_->IsListeningForTabDestruction(nav_controller)) {
- google_url_tracker_->OnTabClosed(nav_controller);
+ GoogleURLTrackerNavigationHelper* nav_helper = GetNavigationHelper(unique_id);
+ if (nav_helper && nav_helper->IsListeningForTabDestruction()) {
+ google_url_tracker_->OnTabClosed(nav_helper);
} else {
// Closing a tab with an infobar showing would close the infobar.
GoogleURLTrackerInfoBarDelegate* delegate = GetInfoBarDelegate(unique_id);
@@ -457,6 +435,12 @@ GoogleURLTrackerInfoBarDelegate* GoogleURLTrackerTest::GetInfoBarDelegate(
return map_entry ? map_entry->infobar_delegate() : NULL;
}
+GoogleURLTrackerNavigationHelper* GoogleURLTrackerTest::GetNavigationHelper(
+ intptr_t unique_id) {
+ GoogleURLTrackerMapEntry* map_entry = GetMapEntry(unique_id);
+ return map_entry ? map_entry->navigation_helper() : NULL;
+}
+
void GoogleURLTrackerTest::ExpectDefaultURLs() const {
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL(), fetched_google_url());
@@ -466,8 +450,8 @@ void GoogleURLTrackerTest::ExpectListeningForCommit(intptr_t unique_id,
bool listening) {
GoogleURLTrackerMapEntry* map_entry = GetMapEntry(unique_id);
if (map_entry) {
- EXPECT_EQ(listening, nav_helper_->IsListeningForNavigationCommit(
- map_entry->navigation_controller()));
+ EXPECT_EQ(listening,
+ map_entry->navigation_helper()->IsListeningForNavigationCommit());
} else {
EXPECT_FALSE(listening);
}
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 16c92aa..7f2a0ae 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -702,6 +702,7 @@
'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.cc',
'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',