Skip to content

GitLab

T
tangled
Commit 184c2367 authored by blundell@chromium.org's avatar blundell@chromium.org
Browse files
Options

Elimate NOTIFICATION_GOOGLE_URL_UPDATED 

This CL moves the remaining client of the NOTIFICATION_GOOGLE_URL_UPDATED
notification to instead register a callback with GoogleURLTracker and
eliminates the notification. It also migrate the GoogleURLTracker unittest from
listening for the notification to listening for the callback.

BUG=373261,373237
TBR=thakis

Review URL: https://codereview.chromium.org/284343003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271313 0039d316-1c4b-4281-b951-d872f2087c98
parent 49d02cd2
Hide whitespace changes
Inline Side-by-side
Showing with 89 additions and 111 deletions
chrome/browser/chrome_notification_types.h
View file @ 184c2367
......@@ -353,18 +353,6 @@ enum NotificationType {
// This is sent from Instant when the omnibox focus state changes.
NOTIFICATION_OMNIBOX_FOCUS_CHANGED,
// Sent when the Google URL for a profile has been updated. Some services
// cache this value and need to update themselves when it changes. See
// google_util::GetGoogleURLAndUpdateIfNecessary(). The source is the
// Profile, the details a GoogleURLTracker::UpdatedDetails containing the old
// and new URLs.
//
// Note that because incognito mode requests for the GoogleURLTracker are
// redirected to the non-incognito profile's copy, this notification will only
// ever fire on non-incognito profiles; thus listeners should use
// GetOriginalProfile() when constructing a Source to filter against.
NOTIFICATION_GOOGLE_URL_UPDATED,
// Printing ----------------------------------------------------------------
// Notification from PrintJob that an event occurred. It can be that a page
......
chrome/browser/google/google_url_tracker.cc
View file @ 184c2367
......@@ -102,19 +102,13 @@ void GoogleURLTracker::GoogleURLSearchCommitted(Profile* profile) {
}
void GoogleURLTracker::AcceptGoogleURL(bool redo_searches) {
UpdatedDetails urls(google_url_, fetched_google_url_);
GURL old_google_url = google_url_;
google_url_ = fetched_google_url_;
PrefService* prefs = profile_->GetPrefs();
prefs->SetString(prefs::kLastKnownGoogleURL, google_url_.spec());
prefs->SetString(prefs::kLastPromptedGoogleURL, google_url_.spec());
NotifyGoogleURLUpdated(urls.first, urls.second);
// TODO(blundell): Convert all clients to use the callback interface and
// eliminate this notification. crbug.com/373237
content::NotificationService::current()->Notify(
chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
content::Source<Profile>(profile_),
content::Details<UpdatedDetails>(&urls));
NotifyGoogleURLUpdated(old_google_url, google_url_);
need_to_prompt_ = false;
CloseAllEntries(redo_searches);
}
......
chrome/browser/google/google_url_tracker.h
View file @ 184c2367
......@@ -39,9 +39,9 @@ class InfoBar;
//
// Most consumers should only call GoogleURL(), which is guaranteed to
// synchronously return a value at all times (even during startup or in unittest
// mode). Consumers who need to be notified when things change should listen to
// the notification service for NOTIFICATION_GOOGLE_URL_UPDATED, which provides
// the original and updated values.
// mode). Consumers who need to be notified when things change should register
// a callback that provides the original and updated values via
// RegisterCallback().
//
// To protect users' privacy and reduce server load, no updates will be
// performed (ever) unless at least one consumer registers interest by calling
......@@ -56,9 +56,6 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
typedef base::CallbackList<void(GURL, GURL)> CallbackList;
typedef CallbackList::Subscription Subscription;
// The contents of the Details for a NOTIFICATION_GOOGLE_URL_UPDATED.
typedef std::pair<GURL, GURL> UpdatedDetails;
// The constructor does different things depending on which of these values
// you pass it. Hopefully these are self-explanatory.
enum Mode {
......@@ -211,8 +208,8 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
bool need_to_fetch_; // True if a consumer actually wants us to fetch an
// updated URL. If this is never set, we won't
// bother to fetch anything.
// Consumers should observe
// chrome::NOTIFICATION_GOOGLE_URL_UPDATED.
// Consumers should register a callback via
// RegisterCallback().
bool need_to_prompt_; // True if the last fetched Google URL is not
// matched with current user's default Google URL
// nor the last prompted Google URL.
......
chrome/browser/google/google_url_tracker_unittest.cc
View file @ 184c2367
......@@ -63,36 +63,46 @@ class TestInfoBarDelegate : public GoogleURLTrackerInfoBarDelegate {
// GoogleURLTrackerTest, so they can call members on it.
// TestNotificationObserver ---------------------------------------------------
// TestCallbackListener ---------------------------------------------------
class TestNotificationObserver : public content::NotificationObserver {
class TestCallbackListener {
public:
TestNotificationObserver();
virtual ~TestNotificationObserver();
TestCallbackListener();
virtual ~TestCallbackListener();
bool HasRegisteredCallback();
void RegisterCallback(GoogleURLTracker* google_url_tracker);
virtual void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;
bool notified() const { return notified_; }
void clear_notified() { notified_ = false; }
private:
void OnGoogleURLUpdated(GURL old_url, GURL new_url);
bool notified_;
scoped_ptr<GoogleURLTracker::Subscription> google_url_updated_subscription_;
};
TestNotificationObserver::TestNotificationObserver() : notified_(false) {
TestCallbackListener::TestCallbackListener() : notified_(false) {
}
TestNotificationObserver::~TestNotificationObserver() {
TestCallbackListener::~TestCallbackListener() {
}
void TestNotificationObserver::Observe(
int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
void TestCallbackListener::OnGoogleURLUpdated(GURL old_url, GURL new_url) {
notified_ = true;
}
bool TestCallbackListener::HasRegisteredCallback() {
return google_url_updated_subscription_.get();
}
void TestCallbackListener::RegisterCallback(
GoogleURLTracker* google_url_tracker) {
google_url_updated_subscription_ =
google_url_tracker->RegisterCallback(base::Bind(
&TestCallbackListener::OnGoogleURLUpdated, base::Unretained(this)));
}
// TestGoogleURLTrackerNavigationHelper -------------------------------------
......@@ -232,8 +242,8 @@ class GoogleURLTrackerTest : public testing::Test {
GoogleURLTrackerInfoBarDelegate* GetInfoBarDelegate(intptr_t unique_id);
void ExpectDefaultURLs() const;
void ExpectListeningForCommit(intptr_t unique_id, bool listening);
bool observer_notified() const { return observer_.notified(); }
void clear_observer_notified() { observer_.clear_notified(); }
bool listener_notified() const { return listener_.notified(); }
void clear_listener_notified() { listener_.clear_notified(); }
private:
// Since |infobar_service| is really a magic number rather than an actual
......@@ -251,11 +261,10 @@ class GoogleURLTrackerTest : public testing::Test {
// net::NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests().
scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_;
net::TestURLFetcherFactory fetcher_factory_;
content::NotificationRegistrar registrar_;
TestNotificationObserver observer_;
GoogleURLTrackerNavigationHelper* nav_helper_;
TestingProfile profile_;
scoped_ptr<GoogleURLTracker> google_url_tracker_;
TestCallbackListener listener_;
// This tracks the different "tabs" a test has "opened", so we can close them
// properly before shutting down |google_url_tracker_|, which expects that.
std::set<int> unique_ids_seen_;
......@@ -307,7 +316,6 @@ void GoogleURLTrackerTest::TearDown() {
CloseTab(*unique_ids_seen_.begin());
nav_helper_ = NULL;
google_url_tracker_.reset();
network_change_notifier_.reset();
}
......@@ -331,12 +339,8 @@ void GoogleURLTrackerTest::MockSearchDomainCheckResponse(
}
void GoogleURLTrackerTest::RequestServerCheck() {
if (!registrar_.IsRegistered(&observer_,
chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
content::Source<Profile>(&profile_))) {
registrar_.Add(&observer_, chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
content::Source<Profile>(&profile_));
}
if (!listener_.HasRegisteredCallback())
listener_.RegisterCallback(google_url_tracker_.get());
google_url_tracker_->SetNeedToFetch();
}
......@@ -507,21 +511,21 @@ TEST_F(GoogleURLTrackerTest, DontFetchWhenNoOneRequestsCheck) {
EXPECT_FALSE(GetFetcher());
MockSearchDomainCheckResponse("http://www.google.co.uk/");
ExpectDefaultURLs();
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, UpdateOnFirstRun) {
RequestServerCheck();
EXPECT_FALSE(GetFetcher());
ExpectDefaultURLs();
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
FinishSleep();
MockSearchDomainCheckResponse("http://www.google.co.uk/");
EXPECT_EQ(GURL("http://www.google.co.uk/"), fetched_google_url());
// GoogleURL should be updated, becase there was no last prompted URL.
EXPECT_EQ(GURL("http://www.google.co.uk/"), google_url());
EXPECT_TRUE(observer_notified());
EXPECT_TRUE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, DontUpdateWhenUnchanged) {
......@@ -530,7 +534,7 @@ TEST_F(GoogleURLTrackerTest, DontUpdateWhenUnchanged) {
RequestServerCheck();
EXPECT_FALSE(GetFetcher());
ExpectDefaultURLs();
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
FinishSleep();
MockSearchDomainCheckResponse("http://www.google.co.uk/");
......@@ -538,7 +542,7 @@ TEST_F(GoogleURLTrackerTest, DontUpdateWhenUnchanged) {
// GoogleURL should not be updated, because the fetched and prompted URLs
// match.
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
......@@ -547,14 +551,14 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
RequestServerCheck();
EXPECT_FALSE(GetFetcher());
ExpectDefaultURLs();
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
// Old-style domain string.
FinishSleep();
MockSearchDomainCheckResponse(".google.co.in");
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
SetNavigationPending(1, true);
CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
EXPECT_TRUE(GetMapEntry(1) == NULL);
......@@ -564,7 +568,7 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
MockSearchDomainCheckResponse("http://mail.google.com/");
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
SetNavigationPending(1, true);
CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
EXPECT_TRUE(GetMapEntry(1) == NULL);
......@@ -574,7 +578,7 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
MockSearchDomainCheckResponse("http://www.google.com/search");
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
SetNavigationPending(1, true);
CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
EXPECT_TRUE(GetMapEntry(1) == NULL);
......@@ -584,7 +588,7 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
MockSearchDomainCheckResponse("http://www.google.com/?q=foo");
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
SetNavigationPending(1, true);
CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
EXPECT_TRUE(GetMapEntry(1) == NULL);
......@@ -594,7 +598,7 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
MockSearchDomainCheckResponse("http://www.google.com/#anchor");
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
SetNavigationPending(1, true);
CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
EXPECT_TRUE(GetMapEntry(1) == NULL);
......@@ -604,7 +608,7 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
MockSearchDomainCheckResponse("HJ)*qF)_*&@f1");
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
SetNavigationPending(1, true);
CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
EXPECT_TRUE(GetMapEntry(1) == NULL);
......@@ -619,7 +623,7 @@ TEST_F(GoogleURLTrackerTest, UpdatePromptedURLOnReturnToPreviousLocation) {
EXPECT_EQ(GURL("http://www.google.co.uk/"), fetched_google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, SilentlyAcceptSchemeChange) {
......@@ -633,14 +637,14 @@ TEST_F(GoogleURLTrackerTest, SilentlyAcceptSchemeChange) {
EXPECT_EQ(GURL("https://www.google.co.uk/"), fetched_google_url());
EXPECT_EQ(GURL("https://www.google.co.uk/"), google_url());
EXPECT_EQ(GURL("https://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_TRUE(observer_notified());
EXPECT_TRUE(listener_notified());
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("http://www.google.co.uk/");
EXPECT_EQ(GURL("http://www.google.co.uk/"), fetched_google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_TRUE(observer_notified());
EXPECT_TRUE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, RefetchOnIPAddressChange) {
......@@ -649,15 +653,15 @@ TEST_F(GoogleURLTrackerTest, RefetchOnIPAddressChange) {
MockSearchDomainCheckResponse("http://www.google.co.uk/");
EXPECT_EQ(GURL("http://www.google.co.uk/"), fetched_google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), google_url());
EXPECT_TRUE(observer_notified());
clear_observer_notified();
EXPECT_TRUE(listener_notified());
clear_listener_notified();
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("http://www.google.co.in/");
EXPECT_EQ(GURL("http://www.google.co.in/"), fetched_google_url());
// Just fetching a new URL shouldn't reset things without a prompt.
EXPECT_EQ(GURL("http://www.google.co.uk/"), google_url());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, DontRefetchWhenNoOneRequestsCheck) {
......@@ -667,7 +671,7 @@ TEST_F(GoogleURLTrackerTest, DontRefetchWhenNoOneRequestsCheck) {
EXPECT_FALSE(GetFetcher());
MockSearchDomainCheckResponse("http://www.google.co.uk/");
ExpectDefaultURLs();
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, FetchOnLateRequest) {
......@@ -681,7 +685,7 @@ TEST_F(GoogleURLTrackerTest, FetchOnLateRequest) {
MockSearchDomainCheckResponse("http://www.google.co.uk/");
EXPECT_EQ(GURL("http://www.google.co.uk/"), fetched_google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), google_url());
EXPECT_TRUE(observer_notified());
EXPECT_TRUE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, DontFetchTwiceOnLateRequests) {
......@@ -695,8 +699,8 @@ TEST_F(GoogleURLTrackerTest, DontFetchTwiceOnLateRequests) {
MockSearchDomainCheckResponse("http://www.google.co.uk/");
EXPECT_EQ(GURL("http://www.google.co.uk/"), fetched_google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), google_url());
EXPECT_TRUE(observer_notified());
clear_observer_notified();
EXPECT_TRUE(listener_notified());
clear_listener_notified();
RequestServerCheck();
// The second request should be ignored.
......@@ -704,7 +708,7 @@ TEST_F(GoogleURLTrackerTest, DontFetchTwiceOnLateRequests) {
MockSearchDomainCheckResponse("http://www.google.co.in/");
EXPECT_EQ(GURL("http://www.google.co.uk/"), fetched_google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), google_url());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, SearchingDoesNothingIfNoNeedToPrompt) {
......@@ -714,8 +718,8 @@ TEST_F(GoogleURLTrackerTest, SearchingDoesNothingIfNoNeedToPrompt) {
EXPECT_EQ(GURL("http://www.google.co.uk/"), fetched_google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_TRUE(observer_notified());
clear_observer_notified();
EXPECT_TRUE(listener_notified());
clear_listener_notified();
SetNavigationPending(1, true);
CommitSearch(1, GURL("http://www.google.co.uk/search?q=test"));
......@@ -723,7 +727,7 @@ TEST_F(GoogleURLTrackerTest, SearchingDoesNothingIfNoNeedToPrompt) {
EXPECT_EQ(GURL("http://www.google.co.uk/"), fetched_google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, TabClosedOnPendingSearch) {
......@@ -734,7 +738,7 @@ TEST_F(GoogleURLTrackerTest, TabClosedOnPendingSearch) {
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL("http://www.google.co.jp/"), fetched_google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
SetNavigationPending(1, true);
GoogleURLTrackerMapEntry* map_entry = GetMapEntry(1);
......@@ -742,13 +746,13 @@ TEST_F(GoogleURLTrackerTest, TabClosedOnPendingSearch) {
EXPECT_FALSE(map_entry->has_infobar_delegate());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
CloseTab(1);
EXPECT_TRUE(GetMapEntry(1) == NULL);
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, TabClosedOnCommittedSearch) {
......@@ -765,7 +769,7 @@ TEST_F(GoogleURLTrackerTest, TabClosedOnCommittedSearch) {
EXPECT_TRUE(GetMapEntry(1) == NULL);
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, InfoBarClosed) {
......@@ -783,7 +787,7 @@ TEST_F(GoogleURLTrackerTest, InfoBarClosed) {
EXPECT_TRUE(GetMapEntry(1) == NULL);
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, InfoBarRefused) {
......@@ -801,7 +805,7 @@ TEST_F(GoogleURLTrackerTest, InfoBarRefused) {
EXPECT_TRUE(GetMapEntry(1) == NULL);
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL("http://www.google.co.jp/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, InfoBarAccepted) {
......@@ -819,7 +823,7 @@ TEST_F(GoogleURLTrackerTest, InfoBarAccepted) {
EXPECT_TRUE(GetMapEntry(1) == NULL);
EXPECT_EQ(GURL("http://www.google.co.jp/"), google_url());
EXPECT_EQ(GURL("http://www.google.co.jp/"), GetLastPromptedGoogleURL());
EXPECT_TRUE(observer_notified());
EXPECT_TRUE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfoBars) {
......@@ -942,7 +946,7 @@ TEST_F(GoogleURLTrackerTest, NavigationsAfterPendingSearch) {
EXPECT_TRUE(map_entry->has_infobar_delegate());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
}
......@@ -1014,7 +1018,7 @@ TEST_F(GoogleURLTrackerTest, NavigationsAfterCommittedSearch) {
ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, MultipleMapEntries) {
......@@ -1052,7 +1056,7 @@ TEST_F(GoogleURLTrackerTest, MultipleMapEntries) {
delegate2->Close(false);
EXPECT_TRUE(GetMapEntry(2) == NULL);
EXPECT_FALSE(observer_notified());
EXPECT_FALSE(listener_notified());
delegate4->Accept();
EXPECT_TRUE(GetMapEntry(1) == NULL);
......@@ -1060,7 +1064,7 @@ TEST_F(GoogleURLTrackerTest, MultipleMapEntries) {
EXPECT_TRUE(GetMapEntry(4) == NULL);
EXPECT_EQ(GURL("http://www.google.co.jp/"), google_url());
EXPECT_EQ(GURL("http://www.google.co.jp/"), GetLastPromptedGoogleURL());
EXPECT_TRUE(observer_notified());
EXPECT_TRUE(listener_notified());
}
TEST_F(GoogleURLTrackerTest, IgnoreIrrelevantNavigation) {
......
chrome/browser/ui/navigation_correction_tab_observer.cc
View file @ 184c2367
......@@ -6,12 +6,12 @@
#include "base/prefs/pref_service.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/google/google_url_tracker_factory.h"
#include "chrome/browser/google/google_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/render_messages.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/web_contents.h"
......@@ -36,8 +36,13 @@ NavigationCorrectionTabObserver::NavigationCorrectionTabObserver(
base::Unretained(this)));
}
registrar_.Add(this, chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
content::Source<Profile>(profile_->GetOriginalProfile()));
GoogleURLTracker* google_url_tracker =
GoogleURLTrackerFactory::GetForProfile(profile_);
if (google_url_tracker) {
google_url_updated_subscription_ = google_url_tracker->RegisterCallback(
base::Bind(&NavigationCorrectionTabObserver::OnGoogleURLUpdated,
base::Unretained(this)));
}
}
NavigationCorrectionTabObserver::~NavigationCorrectionTabObserver() {
......@@ -60,19 +65,13 @@ void NavigationCorrectionTabObserver::RenderViewCreated(
}
////////////////////////////////////////////////////////////////////////////////
// content::NotificationObserver overrides
// Internal helpers
void NavigationCorrectionTabObserver::Observe(
int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
DCHECK_EQ(chrome::NOTIFICATION_GOOGLE_URL_UPDATED, type);
void NavigationCorrectionTabObserver::OnGoogleURLUpdated(GURL old_url,
GURL new_url) {
UpdateNavigationCorrectionInfo(web_contents()->GetRenderViewHost());
}
////////////////////////////////////////////////////////////////////////////////
// Internal helpers
GURL NavigationCorrectionTabObserver::GetNavigationCorrectionURL() const {
// Disable navigation corrections when the preference is disabled or when in
// Incognito mode.
......
chrome/browser/ui/navigation_correction_tab_observer.h
View file @ 184c2367
......@@ -6,8 +6,7 @@
#define CHROME_BROWSER_UI_NAVIGATION_CORRECTION_TAB_OBSERVER_H_
#include "base/prefs/pref_change_registrar.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "chrome/browser/google/google_url_tracker.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h"
......@@ -20,7 +19,6 @@ class PrefRegistrySyncable;
// Per-tab class to implement navigation suggestion service functionality.
class NavigationCorrectionTabObserver
: public content::WebContentsObserver,
public content::NotificationObserver,
public content::WebContentsUserData<NavigationCorrectionTabObserver> {
public:
virtual ~NavigationCorrectionTabObserver();
......@@ -35,13 +33,11 @@ class NavigationCorrectionTabObserver
virtual void RenderViewCreated(
content::RenderViewHost* render_view_host) OVERRIDE;
// content::NotificationObserver overrides:
virtual void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;
// Internal helpers ----------------------------------------------------------
// Callback that is called when the Google URL is updated.
void OnGoogleURLUpdated(GURL old_url, GURL new_url);
// Returns the URL for the correction service. If the returned URL
// is empty, the default error pages will be used.
GURL GetNavigationCorrectionURL() const;
......@@ -53,8 +49,8 @@ class NavigationCorrectionTabObserver
void UpdateNavigationCorrectionInfo(content::RenderViewHost* rvh);
Profile* profile_;
content::NotificationRegistrar registrar_;
PrefChangeRegistrar pref_change_registrar_;
scoped_ptr<GoogleURLTracker::Subscription> google_url_updated_subscription_;
DISALLOW_COPY_AND_ASSIGN(NavigationCorrectionTabObserver);
};
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment