summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorblundell@chromium.org <blundell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-05-23 12:10:05 +0000
committerblundell@chromium.org <blundell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-05-23 12:10:05 +0000
commited295ecf6dd36dda6b34c88ae84e487130749fd0 (patch)
tree2bb202d73f10d73a37cd4919234f099f83f61456
parent2b1a592a168970a3aa5908df022b2d8966e2dcd4 (diff)
downloadchromium_src-ed295ecf6dd36dda6b34c88ae84e487130749fd0.zip
chromium_src-ed295ecf6dd36dda6b34c88ae84e487130749fd0.tar.gz
chromium_src-ed295ecf6dd36dda6b34c88ae84e487130749fd0.tar.bz2
Revert of Turn GoogleURLTrackerNavigationHelper(Impl) into a per-tab object. (https://codereview.chromium.org/283413002/)
Reason for revert: Introduced flake on Mac: crbug.com/376550 Original issue's description: > 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 TBR=pkasting@chromium.org NOTREECHECKS=true NOTRY=true BUG=373230 Review URL: https://codereview.chromium.org/294193005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272503 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.cc107
-rw-r--r--chrome/chrome_browser.gypi1
12 files changed, 232 insertions, 187 deletions
diff --git a/chrome/browser/google/chrome_google_url_tracker_client.cc b/chrome/browser/google/chrome_google_url_tracker_client.cc
index 5416669..2d8a1e5 100644
--- a/chrome/browser/google/chrome_google_url_tracker_client.cc
+++ b/chrome/browser/google/chrome_google_url_tracker_client.cc
@@ -6,7 +6,6 @@
#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"
@@ -49,13 +48,12 @@ 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(
- scoped_ptr<GoogleURLTrackerNavigationHelper>(
- new GoogleURLTrackerNavigationHelperImpl(
- controller->GetWebContents(), google_url_tracker())),
+ controller,
infobar_service,
controller->GetPendingEntry()->GetUniqueID());
}
diff --git a/chrome/browser/google/google_url_tracker.cc b/chrome/browser/google/google_url_tracker.cc
index 4983560..62daab0 100644
--- a/chrome/browser/google/google_url_tracker.cc
+++ b/chrome/browser/google/google_url_tracker.cc
@@ -33,11 +33,14 @@ 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,
- Mode mode)
+GoogleURLTracker::GoogleURLTracker(
+ Profile* profile,
+ scoped_ptr<GoogleURLTrackerClient> client,
+ scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper,
+ 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 :
@@ -51,6 +54,7 @@ GoogleURLTracker::GoogleURLTracker(Profile* profile,
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
@@ -201,6 +205,7 @@ void GoogleURLTracker::OnIPAddressChanged() {
void GoogleURLTracker::Shutdown() {
client_.reset();
+ nav_helper_.reset();
fetcher_.reset();
weak_ptr_factory_.InvalidateWeakPtrs();
net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
@@ -214,7 +219,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;
}
@@ -276,41 +281,39 @@ void GoogleURLTracker::SearchCommitted() {
}
void GoogleURLTracker::OnNavigationPending(
- scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper,
+ content::NavigationController* navigation_controller,
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;
- if (!map_entry) {
+ // 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()) {
// 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.
- 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()) {
+ 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()) {
// This is a new search on a tab where we already have an infobar.
- map_entry->infobar_delegate()->set_pending_id(pending_id);
+ i->second->infobar_delegate()->set_pending_id(pending_id);
}
-
- // 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()) {
+ } else if (i != entry_map_.end()){
+ if (i->second->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
@@ -320,13 +323,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(map_entry, false);
- map_entry->infobar_delegate()->set_pending_id(0);
+ UnregisterForEntrySpecificNotifications(*i->second, false);
+ i->second->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.
- map_entry->Close(false);
+ i->second->Close(false);
}
} else {
// Non-search navigation on a tab without an infobars. This is irrelevant
@@ -341,7 +344,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 {
@@ -357,7 +360,7 @@ void GoogleURLTracker::OnNavigationCommitted(InfoBarService* infobar_service,
}
void GoogleURLTracker::OnTabClosed(
- GoogleURLTrackerNavigationHelper* nav_helper) {
+ content::NavigationController* navigation_controller) {
// 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
@@ -367,7 +370,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_helper() == nav_helper) {
+ if (i->second->navigation_controller() == navigation_controller) {
i->second->Close(false);
return;
}
@@ -387,22 +390,26 @@ void GoogleURLTracker::CloseAllEntries(bool redo_searches) {
}
void GoogleURLTracker::UnregisterForEntrySpecificNotifications(
- GoogleURLTrackerMapEntry* map_entry,
+ const 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 (map_entry->navigation_helper()->IsListeningForNavigationCommit()) {
- map_entry->navigation_helper()->SetListeningForNavigationCommit(false);
+ 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_delegate());
+ DCHECK(map_entry.has_infobar_delegate());
}
const bool registered_for_tab_destruction =
- map_entry->navigation_helper()->IsListeningForTabDestruction();
- DCHECK_NE(registered_for_tab_destruction, map_entry->has_infobar_delegate());
+ nav_helper_->IsListeningForTabDestruction(
+ map_entry.navigation_controller());
+ DCHECK_NE(registered_for_tab_destruction, map_entry.has_infobar_delegate());
if (registered_for_tab_destruction) {
- map_entry->navigation_helper()->SetListeningForTabDestruction(false);
+ nav_helper_->SetListeningForTabDestruction(
+ map_entry.navigation_controller(), false);
}
// Our global listeners for these other notifications should be in place iff
@@ -412,7 +419,8 @@ void GoogleURLTracker::UnregisterForEntrySpecificNotifications(
// See the various cases inside OnNavigationPending().
for (EntryMap::const_iterator i(entry_map_.begin()); i != entry_map_.end();
++i) {
- if (i->second->navigation_helper()->IsListeningForNavigationCommit()) {
+ if (nav_helper_->IsListeningForNavigationCommit(
+ i->second->navigation_controller())) {
DCHECK(client_->IsListeningForNavigationStart());
return;
}
diff --git a/chrome/browser/google/google_url_tracker.h b/chrome/browser/google/google_url_tracker.h
index 5d1d612..5e8ba1a 100644
--- a/chrome/browser/google/google_url_tracker.h
+++ b/chrome/browser/google/google_url_tracker.h
@@ -26,6 +26,10 @@ class GoogleURLTrackerNavigationHelper;
class PrefService;
class Profile;
+namespace content {
+class NavigationController;
+}
+
namespace infobars {
class InfoBar;
}
@@ -68,6 +72,7 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
// GoogleURLTrackerFactory::GetForProfile().
GoogleURLTracker(Profile* profile,
scoped_ptr<GoogleURLTrackerClient> client,
+ scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper,
Mode mode);
virtual ~GoogleURLTracker();
@@ -108,14 +113,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.
- // |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
+ // |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 map entry for the associated tab.
virtual void OnNavigationPending(
- scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper,
+ content::NavigationController* navigation_controller,
InfoBarService* infobar_service,
int pending_id);
@@ -126,7 +131,8 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
const GURL& search_url);
// Called by the navigation observer when a tab closes.
- virtual void OnTabClosed(GoogleURLTrackerNavigationHelper* nav_helper);
+ virtual void OnTabClosed(
+ content::NavigationController* navigation_controller);
scoped_ptr<Subscription> RegisterCallback(
const OnGoogleURLUpdatedCallback& cb);
@@ -168,14 +174,14 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
// Google TLD.
void CloseAllEntries(bool redo_searches);
- // Unregisters any listeners for the navigation helper 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 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(
- GoogleURLTrackerMapEntry* map_entry,
+ const GoogleURLTrackerMapEntry& map_entry,
bool must_be_listening_for_commit);
void NotifyGoogleURLUpdated(GURL old_url, GURL new_url);
@@ -186,6 +192,8 @@ 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 6c45c85..239eabc 100644
--- a/chrome/browser/google/google_url_tracker_factory.cc
+++ b/chrome/browser/google/google_url_tracker_factory.cc
@@ -7,6 +7,7 @@
#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"
@@ -37,8 +38,11 @@ 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 ee1ee0f..e0798fd 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,
- scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper)
+ const content::NavigationController* navigation_controller)
: google_url_tracker_(google_url_tracker),
infobar_service_(infobar_service),
infobar_delegate_(NULL),
- navigation_helper_(navigation_helper.Pass()) {
+ 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 5f087fb..2e035a7 100644
--- a/chrome/browser/google/google_url_tracker_map_entry.h
+++ b/chrome/browser/google/google_url_tracker_map_entry.h
@@ -5,8 +5,6 @@
#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"
@@ -14,12 +12,16 @@ class GoogleURLTracker;
class GoogleURLTrackerInfoBarDelegate;
class InfoBarService;
+namespace content {
+class NavigationController;
+}
+
class GoogleURLTrackerMapEntry : public content::NotificationObserver {
public:
GoogleURLTrackerMapEntry(
GoogleURLTracker* google_url_tracker,
InfoBarService* infobar_service,
- scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper);
+ const content::NavigationController* navigation_controller);
virtual ~GoogleURLTrackerMapEntry();
bool has_infobar_delegate() const { return !!infobar_delegate_; }
@@ -28,8 +30,8 @@ class GoogleURLTrackerMapEntry : public content::NotificationObserver {
}
void SetInfoBarDelegate(GoogleURLTrackerInfoBarDelegate* infobar_delegate);
- GoogleURLTrackerNavigationHelper* navigation_helper() {
- return navigation_helper_.get();
+ const content::NavigationController* navigation_controller() const {
+ return navigation_controller_;
}
void Close(bool redo_search);
@@ -46,7 +48,7 @@ class GoogleURLTrackerMapEntry : public content::NotificationObserver {
GoogleURLTracker* const google_url_tracker_;
const InfoBarService* const infobar_service_;
GoogleURLTrackerInfoBarDelegate* infobar_delegate_;
- scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper_;
+ const content::NavigationController* const navigation_controller_;
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
deleted file mode 100644
index 84a2651..0000000
--- a/chrome/browser/google/google_url_tracker_navigation_helper.cc
+++ /dev/null
@@ -1,13 +0,0 @@
-// 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 98804ce..26b7a53 100644
--- a/chrome/browser/google/google_url_tracker_navigation_helper.h
+++ b/chrome/browser/google/google_url_tracker_navigation_helper.h
@@ -5,42 +5,46 @@
#ifndef CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_NAVIGATION_HELPER_H_
#define CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_NAVIGATION_HELPER_H_
-#include "base/macros.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:
- explicit GoogleURLTrackerNavigationHelper(
- GoogleURLTracker* google_url_tracker);
- virtual ~GoogleURLTrackerNavigationHelper();
-
- // 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;
+ virtual ~GoogleURLTrackerNavigationHelper() {}
- // Returns whether or not this object is currently listening for navigation
- // commits.
- virtual bool IsListeningForNavigationCommit() = 0;
+ // Sets the GoogleURLTracker that is associated with this object.
+ virtual void SetGoogleURLTracker(GoogleURLTracker* tracker) = 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 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 tab
- // destruction.
- virtual bool IsListeningForTabDestruction() = 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;
- protected:
- GoogleURLTracker* google_url_tracker() { return google_url_tracker_; }
-
- private:
- GoogleURLTracker* google_url_tracker_;
-
- DISALLOW_COPY_AND_ASSIGN(GoogleURLTrackerNavigationHelper);
+ // 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
index 99420a8..64e4c5c 100644
--- a/chrome/browser/google/google_url_tracker_navigation_helper_impl.cc
+++ b/chrome/browser/google/google_url_tracker_navigation_helper_impl.cc
@@ -12,21 +12,25 @@
#include "content/public/browser/notification_service.h"
#include "content/public/browser/web_contents.h"
-GoogleURLTrackerNavigationHelperImpl::GoogleURLTrackerNavigationHelperImpl(
- content::WebContents* web_contents,
- GoogleURLTracker* tracker)
- : GoogleURLTrackerNavigationHelper(tracker),
- web_contents_(web_contents) {
+GoogleURLTrackerNavigationHelperImpl::
+ GoogleURLTrackerNavigationHelperImpl() : tracker_(NULL) {
}
-GoogleURLTrackerNavigationHelperImpl::~GoogleURLTrackerNavigationHelperImpl() {
+GoogleURLTrackerNavigationHelperImpl::
+ ~GoogleURLTrackerNavigationHelperImpl() {
+}
+
+void GoogleURLTrackerNavigationHelperImpl::SetGoogleURLTracker(
+ GoogleURLTracker* tracker) {
+ DCHECK(tracker);
+ tracker_ = tracker;
}
void GoogleURLTrackerNavigationHelperImpl::SetListeningForNavigationCommit(
+ const content::NavigationController* nav_controller,
bool listen) {
content::NotificationSource navigation_controller_source =
- content::Source<content::NavigationController>(
- &web_contents_->GetController());
+ content::Source<content::NavigationController>(nav_controller);
if (listen) {
registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
navigation_controller_source);
@@ -36,35 +40,47 @@ void GoogleURLTrackerNavigationHelperImpl::SetListeningForNavigationCommit(
}
}
-bool GoogleURLTrackerNavigationHelperImpl::IsListeningForNavigationCommit() {
+bool GoogleURLTrackerNavigationHelperImpl::IsListeningForNavigationCommit(
+ const content::NavigationController* nav_controller) {
content::NotificationSource navigation_controller_source =
- content::Source<content::NavigationController>(
- &web_contents_->GetController());
+ 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 web_contents_source =
- content::Source<content::WebContents>(web_contents_);
+ content::NotificationSource navigation_controller_source =
+ content::Source<content::NavigationController>(nav_controller);
if (listen) {
- registrar_.Add(this,
- content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
- web_contents_source);
+ registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
+ GetWebContentsSource(navigation_controller_source));
} else {
- registrar_.Remove(this,
- content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
- web_contents_source);
+ registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
+ GetWebContentsSource(navigation_controller_source));
}
}
-bool GoogleURLTrackerNavigationHelperImpl::IsListeningForTabDestruction() {
+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,
- content::Source<content::WebContents>(web_contents_));
+ 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(
@@ -75,24 +91,23 @@ 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.
- google_url_tracker()->OnTabClosed(this);
- google_url_tracker()->OnNavigationCommitted(infobar_service, search_url);
+ tracker_->OnTabClosed(controller);
+ tracker_->OnNavigationCommitted(infobar_service, search_url);
break;
}
case content::NOTIFICATION_WEB_CONTENTS_DESTROYED: {
- DCHECK_EQ(web_contents_,
- content::Source<content::WebContents>(source).ptr());
- google_url_tracker()->OnTabClosed(this);
+ content::WebContents* web_contents =
+ content::Source<content::WebContents>(source).ptr();
+ tracker_->OnTabClosed(&web_contents->GetController());
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 1d81e54..b024744 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:
- GoogleURLTrackerNavigationHelperImpl(content::WebContents* web_contents,
- GoogleURLTracker* tracker);
+ explicit GoogleURLTrackerNavigationHelperImpl();
virtual ~GoogleURLTrackerNavigationHelperImpl();
- // GoogleURLTrackerNavigationHelper:
+ // GoogleURLTrackerNavigationHelper.
+ virtual void SetGoogleURLTracker(GoogleURLTracker* tracker) OVERRIDE;
virtual void SetListeningForNavigationCommit(
+ const content::NavigationController* nav_controller,
bool listen) OVERRIDE;
- virtual bool IsListeningForNavigationCommit() OVERRIDE;
+ virtual bool IsListeningForNavigationCommit(
+ const content::NavigationController* nav_controller) OVERRIDE;
virtual void SetListeningForTabDestruction(
+ const content::NavigationController* nav_controller,
bool listen) OVERRIDE;
- virtual bool IsListeningForTabDestruction() OVERRIDE;
+ virtual bool IsListeningForTabDestruction(
+ const content::NavigationController* nav_controller) OVERRIDE;
private:
// content::NotificationObserver:
@@ -36,10 +36,13 @@ class GoogleURLTrackerNavigationHelperImpl
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;
- content::WebContents* web_contents_;
- content::NotificationRegistrar registrar_;
+ // Returns a WebContents NavigationSource for the WebContents corresponding to
+ // the given NavigationController NotificationSource.
+ virtual content::NotificationSource GetWebContentsSource(
+ const content::NotificationSource& nav_controller_source);
- DISALLOW_COPY_AND_ASSIGN(GoogleURLTrackerNavigationHelperImpl);
+ 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 d62eaa6..6b2fedf 100644
--- a/chrome/browser/google/google_url_tracker_unittest.cc
+++ b/chrome/browser/google/google_url_tracker_unittest.cc
@@ -137,53 +137,73 @@ bool TestGoogleURLTrackerClient::IsListeningForNavigationStart() {
return observe_nav_start_;
}
-
// TestGoogleURLTrackerNavigationHelper ---------------------------------------
class TestGoogleURLTrackerNavigationHelper
: public GoogleURLTrackerNavigationHelper {
public:
- explicit TestGoogleURLTrackerNavigationHelper(GoogleURLTracker* tracker);
+ TestGoogleURLTrackerNavigationHelper();
virtual ~TestGoogleURLTrackerNavigationHelper();
- virtual void SetListeningForNavigationCommit(bool listen) OVERRIDE;
- virtual bool IsListeningForNavigationCommit() OVERRIDE;
- virtual void SetListeningForTabDestruction(bool listen) OVERRIDE;
- virtual bool IsListeningForTabDestruction() OVERRIDE;
+ 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;
private:
- bool listening_for_nav_commit_;
- bool listening_for_tab_destruction_;
-
- DISALLOW_COPY_AND_ASSIGN(TestGoogleURLTrackerNavigationHelper);
+ GoogleURLTracker* tracker_;
+ std::set<const content::NavigationController*>
+ nav_controller_commit_listeners_;
+ std::set<const content::NavigationController*>
+ nav_controller_tab_close_listeners_;
};
-TestGoogleURLTrackerNavigationHelper::TestGoogleURLTrackerNavigationHelper(
- GoogleURLTracker* tracker)
- : GoogleURLTrackerNavigationHelper(tracker),
- listening_for_nav_commit_(false),
- listening_for_tab_destruction_(false) {
+TestGoogleURLTrackerNavigationHelper::TestGoogleURLTrackerNavigationHelper()
+ : tracker_(NULL) {
+}
+
+TestGoogleURLTrackerNavigationHelper::
+ ~TestGoogleURLTrackerNavigationHelper() {
}
-TestGoogleURLTrackerNavigationHelper::~TestGoogleURLTrackerNavigationHelper() {
+void TestGoogleURLTrackerNavigationHelper::SetGoogleURLTracker(
+ GoogleURLTracker* tracker) {
+ tracker_ = tracker;
}
void TestGoogleURLTrackerNavigationHelper::SetListeningForNavigationCommit(
+ const content::NavigationController* nav_controller,
bool listen) {
- listening_for_nav_commit_ = listen;
+ if (listen)
+ nav_controller_commit_listeners_.insert(nav_controller);
+ else
+ nav_controller_commit_listeners_.erase(nav_controller);
}
-bool TestGoogleURLTrackerNavigationHelper::IsListeningForNavigationCommit() {
- return listening_for_nav_commit_;
+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) {
- listening_for_tab_destruction_ = listen;
+ if (listen)
+ nav_controller_tab_close_listeners_.insert(nav_controller);
+ else
+ nav_controller_tab_close_listeners_.erase(nav_controller);
}
-bool TestGoogleURLTrackerNavigationHelper::IsListeningForTabDestruction() {
- return listening_for_tab_destruction_;
+bool TestGoogleURLTrackerNavigationHelper::IsListeningForTabDestruction(
+ const content::NavigationController* nav_controller) {
+ return nav_controller_tab_close_listeners_.count(nav_controller) > 0;
}
} // namespace
@@ -240,7 +260,6 @@ 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(); }
@@ -263,6 +282,7 @@ 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_;
@@ -301,12 +321,17 @@ 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 client for its lifetime.
+ // Ownership is passed to google_url_tracker_, but weak pointers are kept;
+ // this is safe since GoogleURLTracker keeps these objects for its lifetime.
client_ = new TestGoogleURLTrackerClient();
+ nav_helper_ = new TestGoogleURLTrackerNavigationHelper();
scoped_ptr<GoogleURLTrackerClient> client(client_);
- google_url_tracker_.reset(new GoogleURLTracker(
- &profile_, client.Pass(), GoogleURLTracker::UNIT_TEST_MODE));
+ 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_->infobar_creator_ = base::Bind(
&GoogleURLTrackerTest::CreateTestInfoBar, base::Unretained(this));
}
@@ -370,11 +395,8 @@ void GoogleURLTrackerTest::SetNavigationPending(intptr_t unique_id,
unique_ids_seen_.insert(unique_id);
if (client_->IsListeningForNavigationStart()) {
google_url_tracker_->OnNavigationPending(
- scoped_ptr<GoogleURLTrackerNavigationHelper>(
- new TestGoogleURLTrackerNavigationHelper(
- google_url_tracker_.get())),
- reinterpret_cast<InfoBarService*>(unique_id),
- unique_id);
+ reinterpret_cast<content::NavigationController*>(unique_id),
+ reinterpret_cast<InfoBarService*>(unique_id), unique_id);
}
}
@@ -400,8 +422,8 @@ void GoogleURLTrackerTest::CommitNonSearch(intptr_t unique_id) {
void GoogleURLTrackerTest::CommitSearch(intptr_t unique_id,
const GURL& search_url) {
DCHECK(search_url.is_valid());
- GoogleURLTrackerNavigationHelper* nav_helper = GetNavigationHelper(unique_id);
- if (nav_helper && nav_helper->IsListeningForNavigationCommit()) {
+ if (nav_helper_->IsListeningForNavigationCommit(
+ reinterpret_cast<content::NavigationController*>(unique_id))) {
google_url_tracker_->OnNavigationCommitted(
reinterpret_cast<InfoBarService*>(unique_id), search_url);
}
@@ -409,9 +431,10 @@ void GoogleURLTrackerTest::CommitSearch(intptr_t unique_id,
void GoogleURLTrackerTest::CloseTab(intptr_t unique_id) {
unique_ids_seen_.erase(unique_id);
- GoogleURLTrackerNavigationHelper* nav_helper = GetNavigationHelper(unique_id);
- if (nav_helper && nav_helper->IsListeningForTabDestruction()) {
- google_url_tracker_->OnTabClosed(nav_helper);
+ 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* delegate = GetInfoBarDelegate(unique_id);
@@ -434,12 +457,6 @@ 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());
@@ -449,8 +466,8 @@ void GoogleURLTrackerTest::ExpectListeningForCommit(intptr_t unique_id,
bool listening) {
GoogleURLTrackerMapEntry* map_entry = GetMapEntry(unique_id);
if (map_entry) {
- EXPECT_EQ(listening,
- map_entry->navigation_helper()->IsListeningForNavigationCommit());
+ EXPECT_EQ(listening, nav_helper_->IsListeningForNavigationCommit(
+ map_entry->navigation_controller()));
} else {
EXPECT_FALSE(listening);
}
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 7d99444..a6bcc17 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -696,7 +696,6 @@
'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',