summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorkaren@chromium.org <karen@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-07-10 00:28:11 +0000
committerkaren@chromium.org <karen@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-07-10 00:28:11 +0000
commit5bfd2b9da50ef216cf207a8c19042884ca6a4783 (patch)
treec7fd1fe8e4e7485144f870ed6715c370b861d657
parent48b13139f347ce0b2d924ec49b36c3e094227bc8 (diff)
downloadchromium_src-5bfd2b9da50ef216cf207a8c19042884ca6a4783.zip
chromium_src-5bfd2b9da50ef216cf207a8c19042884ca6a4783.tar.gz
chromium_src-5bfd2b9da50ef216cf207a8c19042884ca6a4783.tar.bz2
Revert 145760 - Merge 144201 - More comprehensive handling of NOTIFICATION_NAV_ENTRY_PENDING for GoogleURLTracker.
In particular, there are three cases that are improved: (1) We didn't deal well with further searches in the same tab that was already showing an infobar. Depending on ordering, various kinds of badness could occur, in particular trying to deref infobar_map_.end(). (2) If we caught a PENDING search, but before the load committed the user performed some other navigation, we'd show the infobar on the commit for the later navigation, leading to infobars on random pages. (3) In a generalization of (2), we now listen for PENDING in many more cases so that we know if the user is starting a new navigation, which may be the only way we get signalled that a previous navigation has been cancelled. (Note that if a pending navigation is simply stopped, and we were waiting for commit, we'll keep waiting, but this is harmless since eventually either the tab will be closed or the user will start another navigation.) These changes should hopefully make GoogleURLTracker as robust about navigation-handling as AlternateNavURLFetcher already was. Additionally, I reverted a previous change where a pending search would change the "search_url" of a visible infobar in that tab. Because we don't get notified when a load is stopped, we can't distinguish between "started a new search that hasn't committed" and "started but then cancelled a new search", and thus it's potentially wrong to search for the new URL (yet). We now once again wait until a search actually commits. In doing this I simplified things so that we only even pass in the search URL when a load commits. BUG=133108 TEST=Edit your preferences file so that you get prompted about changing Google search URLs. Do a Google search in one tab, wait for the infobar to appear, then do another search in the same tab. The infobar should stick around and interacting with it shouldn't cause crashes. Review URL: https://chromiumcodereview.appspot.com/10630022 TBR=pkasting@chromium.org TBR=pkasting@chromium.org Review URL: https://chromiumcodereview.appspot.com/10749017 git-svn-id: svn://svn.chromium.org/chrome/branches/1180/src@145808 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/google/google_url_tracker.cc295
-rw-r--r--chrome/browser/google/google_url_tracker.h62
-rw-r--r--chrome/browser/google/google_url_tracker_unittest.cc332
3 files changed, 222 insertions, 467 deletions
diff --git a/chrome/browser/google/google_url_tracker.cc b/chrome/browser/google/google_url_tracker.cc
index 750b3f5..5124f15 100644
--- a/chrome/browser/google/google_url_tracker.cc
+++ b/chrome/browser/google/google_url_tracker.cc
@@ -23,7 +23,6 @@
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "content/public/browser/navigation_controller.h"
-#include "content/public/browser/navigation_details.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/web_contents.h"
@@ -39,10 +38,11 @@ namespace {
GoogleURLTrackerInfoBarDelegate* CreateInfoBar(
InfoBarTabHelper* infobar_helper,
+ const GURL& search_url,
GoogleURLTracker* google_url_tracker,
const GURL& new_google_url) {
- return new GoogleURLTrackerInfoBarDelegate(infobar_helper, google_url_tracker,
- new_google_url);
+ return new GoogleURLTrackerInfoBarDelegate(infobar_helper, search_url,
+ google_url_tracker, new_google_url);
}
string16 GetHost(const GURL& url) {
@@ -56,14 +56,15 @@ string16 GetHost(const GURL& url) {
GoogleURLTrackerInfoBarDelegate::GoogleURLTrackerInfoBarDelegate(
InfoBarTabHelper* infobar_helper,
+ const GURL& search_url,
GoogleURLTracker* google_url_tracker,
const GURL& new_google_url)
: ConfirmInfoBarDelegate(infobar_helper),
map_key_(infobar_helper),
+ search_url_(search_url),
google_url_tracker_(google_url_tracker),
new_google_url_(new_google_url),
- showing_(false),
- pending_id_(0) {
+ showing_(false) {
}
bool GoogleURLTrackerInfoBarDelegate::Accept() {
@@ -91,30 +92,30 @@ bool GoogleURLTrackerInfoBarDelegate::LinkClicked(
return false;
}
-bool GoogleURLTrackerInfoBarDelegate::ShouldExpireInternal(
- const content::LoadCommittedDetails& details) const {
- int unique_id = details.entry->GetUniqueID();
- return (unique_id != contents_unique_id()) && (unique_id != pending_id_);
-}
-
void GoogleURLTrackerInfoBarDelegate::SetGoogleURL(const GURL& new_google_url) {
DCHECK_EQ(GetHost(new_google_url_), GetHost(new_google_url));
new_google_url_ = new_google_url;
}
-void GoogleURLTrackerInfoBarDelegate::Show(const GURL& search_url) {
- if (!owner())
- return;
- StoreActiveEntryUniqueID(owner());
- search_url_ = search_url;
- pending_id_ = 0;
- if (!showing_) {
- showing_ = true;
- owner()->AddInfoBar(this); // May delete |this| on failure!
- }
+void GoogleURLTrackerInfoBarDelegate::Show() {
+ showing_ = true;
+ owner()->AddInfoBar(this); // May delete |this| on failure!
}
void GoogleURLTrackerInfoBarDelegate::Close(bool redo_search) {
+ if (redo_search && owner()) {
+ // Re-do the user's search on the new domain.
+ url_canon::Replacements<char> replacements;
+ const std::string& host(new_google_url_.host());
+ replacements.SetHost(host.data(), url_parse::Component(0, host.length()));
+ GURL new_search_url(search_url_.ReplaceComponents(replacements));
+ if (new_search_url.is_valid()) {
+ content::OpenURLParams params(new_search_url, content::Referrer(),
+ CURRENT_TAB, content::PAGE_TRANSITION_GENERATED, false);
+ owner()->web_contents()->OpenURL(params);
+ }
+ }
+
if (!showing_) {
// We haven't been added to a tab, so just delete ourselves.
delete this;
@@ -135,24 +136,8 @@ void GoogleURLTrackerInfoBarDelegate::Close(bool redo_search) {
// subsequently reached here due to GoogleURLTracker::CloseAllInfoBars().
// This case will no longer happen once infobars are refactored to own their
// delegates.
- if (!owner())
- return;
-
- if (redo_search) {
- // Re-do the user's search on the new domain.
- DCHECK(search_url_.is_valid());
- url_canon::Replacements<char> replacements;
- const std::string& host(new_google_url_.host());
- replacements.SetHost(host.data(), url_parse::Component(0, host.length()));
- GURL new_search_url(search_url_.ReplaceComponents(replacements));
- if (new_search_url.is_valid()) {
- content::OpenURLParams params(new_search_url, content::Referrer(),
- CURRENT_TAB, content::PAGE_TRANSITION_GENERATED, false);
- owner()->web_contents()->OpenURL(params);
- }
- }
-
- owner()->RemoveInfoBar(this);
+ if (owner())
+ owner()->RemoveInfoBar(this);
}
GoogleURLTrackerInfoBarDelegate::~GoogleURLTrackerInfoBarDelegate() {
@@ -175,30 +160,6 @@ string16 GoogleURLTrackerInfoBarDelegate::GetButtonLabel(
}
-// GoogleURLTracker::MapEntry -------------------------------------------------
-
-// Note that we have to initialize at least the NotificationSources explicitly
-// lest this not compile, because NotificationSource has no null constructor.
-GoogleURLTracker::MapEntry::MapEntry()
- : navigation_controller_source(
- content::Source<content::NavigationController>(NULL)),
- tab_contents_source(content::Source<TabContents>(NULL)) {
- NOTREACHED();
-}
-
-GoogleURLTracker::MapEntry::MapEntry(
- GoogleURLTrackerInfoBarDelegate* infobar,
- const content::NotificationSource& navigation_controller_source,
- const content::NotificationSource& tab_contents_source)
- : infobar(infobar),
- navigation_controller_source(navigation_controller_source),
- tab_contents_source(tab_contents_source) {
-}
-
-GoogleURLTracker::MapEntry::~MapEntry() {
-}
-
-
// GoogleURLTracker -----------------------------------------------------------
const char GoogleURLTracker::kDefaultGoogleHomepage[] =
@@ -216,8 +177,7 @@ GoogleURLTracker::GoogleURLTracker(Profile* profile, Mode mode)
in_startup_sleep_(true),
already_fetched_(false),
need_to_fetch_(false),
- need_to_prompt_(false),
- search_committed_(false) {
+ need_to_prompt_(false) {
net::NetworkChangeNotifier::AddIPAddressObserver(this);
// Because this function can be called during startup, when kicking off a URL
@@ -339,7 +299,7 @@ void GoogleURLTracker::OnURLFetchComplete(const net::URLFetcher* source) {
} else if (fetched_google_url_ != url) {
for (InfoBarMap::iterator i(infobar_map_.begin());
i != infobar_map_.end(); ++i)
- i->second.infobar->SetGoogleURL(fetched_google_url_);
+ i->second->SetGoogleURL(fetched_google_url_);
}
}
}
@@ -353,25 +313,29 @@ void GoogleURLTracker::Observe(int type,
content::Source<content::NavigationController>(source).ptr();
TabContents* tab_contents =
TabContents::FromWebContents(controller->GetWebContents());
- OnNavigationPending(source, content::Source<TabContents>(tab_contents),
+ OnNavigationPending(source,
+ content::Source<TabContents>(tab_contents),
tab_contents->infobar_tab_helper(),
- controller->GetPendingEntry()->GetUniqueID());
+ controller->GetPendingEntry()->GetURL());
break;
}
case content::NOTIFICATION_NAV_ENTRY_COMMITTED: {
- content::NavigationController* controller =
- content::Source<content::NavigationController>(source).ptr();
- OnNavigationCommittedOrTabClosed(
- TabContents::FromWebContents(controller->GetWebContents())->
- infobar_tab_helper(),
- controller->GetActiveEntry()->GetURL());
+ TabContents* tab_contents = TabContents::FromWebContents(
+ content::Source<content::NavigationController>(source)->
+ GetWebContents());
+ OnNavigationCommittedOrTabClosed(source,
+ content::Source<TabContents>(tab_contents),
+ tab_contents->infobar_tab_helper(), true);
break;
}
case chrome::NOTIFICATION_TAB_CONTENTS_DESTROYED: {
+ TabContents* tab_contents = content::Source<TabContents>(source).ptr();
OnNavigationCommittedOrTabClosed(
- content::Source<TabContents>(source)->infobar_tab_helper(), GURL());
+ content::Source<content::NavigationController>(
+ &tab_contents->web_contents()->GetController()), source,
+ tab_contents->infobar_tab_helper(), false);
break;
}
@@ -380,10 +344,12 @@ void GoogleURLTracker::Observe(int type,
content::WebContents* web_contents = tab_contents->web_contents();
content::Source<content::NavigationController> source(
&web_contents->GetController());
+ content::Source<TabContents> tab_contents_source(tab_contents);
InfoBarTabHelper* infobar_helper = tab_contents->infobar_tab_helper();
- OnNavigationPending(source, content::Source<TabContents>(tab_contents),
- infobar_helper, 0);
- OnNavigationCommittedOrTabClosed(infobar_helper, web_contents->GetURL());
+ OnNavigationPending(source, tab_contents_source, infobar_helper,
+ web_contents->GetURL());
+ OnNavigationCommittedOrTabClosed(source, tab_contents_source,
+ infobar_helper, true);
break;
}
@@ -427,32 +393,9 @@ void GoogleURLTracker::CancelGoogleURL(const GURL& new_google_url) {
}
void GoogleURLTracker::InfoBarClosed(const InfoBarTabHelper* infobar_helper) {
- DCHECK(!search_committed_);
InfoBarMap::iterator i(infobar_map_.find(infobar_helper));
DCHECK(i != infobar_map_.end());
- const MapEntry& map_entry = i->second;
-
- UnregisterForEntrySpecificNotifications(map_entry, false);
infobar_map_.erase(i);
-
- // Our global listeners for these other notifications should be in place iff
- // we have any non-showing infobars.
- for (InfoBarMap::const_iterator i(infobar_map_.begin());
- i != infobar_map_.end(); ++i) {
- if (!i->second.infobar->showing()) {
- DCHECK(registrar_.IsRegistered(this,
- content::NOTIFICATION_NAV_ENTRY_PENDING,
- content::NotificationService::AllBrowserContextsAndSources()));
- return;
- }
- }
- if (registrar_.IsRegistered(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
- content::NotificationService::AllBrowserContextsAndSources())) {
- registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
- content::NotificationService::AllBrowserContextsAndSources());
- registrar_.Remove(this, chrome::NOTIFICATION_INSTANT_COMMITTED,
- content::NotificationService::AllBrowserContextsAndSources());
- }
}
void GoogleURLTracker::SetNeedToFetch() {
@@ -503,16 +446,12 @@ void GoogleURLTracker::StartFetchIfDesirable() {
void GoogleURLTracker::SearchCommitted() {
if (need_to_prompt_) {
- search_committed_ = true;
- // These notifications will fire a bit later in the same call chain we're
+ // This notification will fire a bit later in the same call chain we're
// currently in.
- if (!registrar_.IsRegistered(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
- content::NotificationService::AllBrowserContextsAndSources())) {
- registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
- content::NotificationService::AllBrowserContextsAndSources());
- registrar_.Add(this, chrome::NOTIFICATION_INSTANT_COMMITTED,
- content::NotificationService::AllBrowserContextsAndSources());
- }
+ registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
+ content::NotificationService::AllBrowserContextsAndSources());
+ registrar_.Add(this, chrome::NOTIFICATION_INSTANT_COMMITTED,
+ content::NotificationService::AllBrowserContextsAndSources());
}
}
@@ -520,110 +459,60 @@ void GoogleURLTracker::OnNavigationPending(
const content::NotificationSource& navigation_controller_source,
const content::NotificationSource& tab_contents_source,
InfoBarTabHelper* infobar_helper,
- int pending_id) {
- InfoBarMap::iterator i(infobar_map_.find(infobar_helper));
-
- 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 (!registrar_.IsRegistered(this,
- content::NOTIFICATION_NAV_ENTRY_COMMITTED,
- navigation_controller_source)) {
- registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
- navigation_controller_source);
- }
- if (i == infobar_map_.end()) {
- // This is a search on a tab that doesn't have one of our infobars, so add
- // one. Note that we only listen for the tab's destruction on this path;
- // if there was already an infobar, then either it's not yet showing and
- // we're already registered for this, or it is showing and its owner will
- // handle tearing it down when the tab is destroyed.
- registrar_.Add(this, chrome::NOTIFICATION_TAB_CONTENTS_DESTROYED,
- tab_contents_source);
- infobar_map_.insert(std::make_pair(infobar_helper, MapEntry(
- (*infobar_creator_)(infobar_helper, this, fetched_google_url_),
- navigation_controller_source, tab_contents_source)));
- } else {
- // This is a new search on a tab where we already have an infobar (which
- // may or may not be showing).
- i->second.infobar->set_pending_id(pending_id);
- }
- } else if (i != infobar_map_.end()){
- if (i->second.infobar->showing()) {
- // This is a non-search navigation on a tab with a visible 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 was no pending search on this tab, these statements are
- // effectively a no-op.
- //
- // 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->set_pending_id(0);
- } else {
- // Non-search navigation on a tab with a not-yet-shown infobar. This
- // means the original search won't commit, so close the infobar.
- i->second.infobar->Close(false);
- }
+ const GURL& search_url) {
+ registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
+ content::NotificationService::AllBrowserContextsAndSources());
+ registrar_.Remove(this, chrome::NOTIFICATION_INSTANT_COMMITTED,
+ content::NotificationService::AllBrowserContextsAndSources());
+
+ if (registrar_.IsRegistered(this,
+ chrome::NOTIFICATION_TAB_CONTENTS_DESTROYED, tab_contents_source)) {
+ // If the previous load hasn't committed and the user triggers a new load,
+ // we don't need to re-register our listeners; just kill the old,
+ // never-shown infobar (to be replaced by a new one below).
+ InfoBarMap::iterator i(infobar_map_.find(infobar_helper));
+ DCHECK(i != infobar_map_.end());
+ i->second->Close(false);
} else {
- // Non-search navigation on a tab without one of our infobars. This is
- // irrelevant to us.
+ // Start listening for the commit notification. We also need to listen for
+ // the tab close command since that means the load will never commit. Note
+ // that in this case we don't need to close any previous infobar for this
+ // tab since this navigation will close it.
+ registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+ navigation_controller_source);
+ registrar_.Add(this, chrome::NOTIFICATION_TAB_CONTENTS_DESTROYED,
+ tab_contents_source);
}
+
+ infobar_map_[infobar_helper] = (*infobar_creator_)(infobar_helper, search_url,
+ this, fetched_google_url_);
}
void GoogleURLTracker::OnNavigationCommittedOrTabClosed(
+ const content::NotificationSource& navigation_controller_source,
+ const content::NotificationSource& tab_contents_source,
const InfoBarTabHelper* infobar_helper,
- const GURL& search_url) {
+ bool navigated) {
+ registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+ navigation_controller_source);
+ registrar_.Remove(this, chrome::NOTIFICATION_TAB_CONTENTS_DESTROYED,
+ tab_contents_source);
+
InfoBarMap::iterator i(infobar_map_.find(infobar_helper));
DCHECK(i != infobar_map_.end());
- const MapEntry& map_entry = i->second;
-
- if (!search_url.is_valid()) {
- // Tab closed, or we somehow tried to navigate to an invalid URL (?!).
- // InfoBarClosed() will take care of unregistering the notifications for
- // this tab.
- map_entry.infobar->Close(false);
- return;
- }
-
- // We're getting called because of a commit notification, so pass true for
- // |must_be_listening_for_commit|.
- UnregisterForEntrySpecificNotifications(map_entry, true);
- map_entry.infobar->Show(search_url);
+ DCHECK(need_to_prompt_);
+ if (navigated)
+ i->second->Show();
+ else
+ i->second->Close(false); // Close manually since it's not added to a tab.
}
void GoogleURLTracker::CloseAllInfoBars(bool redo_searches) {
// Close all infobars, whether they've been added to tabs or not.
while (!infobar_map_.empty())
- infobar_map_.begin()->second.infobar->Close(redo_searches);
-}
-
-void GoogleURLTracker::UnregisterForEntrySpecificNotifications(
- const MapEntry& map_entry,
- bool must_be_listening_for_commit) {
- // For tabs with non-showing infobars, we should always be listening for both
- // these notifications. For tabs with showing infobars, we may be listening
- // for NOTIFICATION_NAV_ENTRY_COMMITTED if the user has performed a new search
- // on this tab.
- if (registrar_.IsRegistered(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
- map_entry.navigation_controller_source)) {
- registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
- map_entry.navigation_controller_source);
- } else {
- DCHECK(!must_be_listening_for_commit);
- DCHECK(map_entry.infobar->showing());
- }
- const bool registered_for_tab_contents_destroyed =
- registrar_.IsRegistered(this, chrome::NOTIFICATION_TAB_CONTENTS_DESTROYED,
- map_entry.tab_contents_source);
- DCHECK_NE(registered_for_tab_contents_destroyed,
- map_entry.infobar->showing());
- if (registered_for_tab_contents_destroyed) {
- registrar_.Remove(this, chrome::NOTIFICATION_TAB_CONTENTS_DESTROYED,
- map_entry.tab_contents_source);
- }
+ infobar_map_.begin()->second->Close(redo_searches);
+
+ // Any registered listeners for NAV_ENTRY_COMMITTED and TAB_CLOSED are now
+ // irrelevant as the associated infobars are gone.
+ registrar_.RemoveAll();
}
diff --git a/chrome/browser/google/google_url_tracker.h b/chrome/browser/google/google_url_tracker.h
index 38cda80c..96b192a 100644
--- a/chrome/browser/google/google_url_tracker.h
+++ b/chrome/browser/google/google_url_tracker.h
@@ -17,7 +17,6 @@
#include "chrome/browser/tab_contents/confirm_infobar_delegate.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
-#include "content/public/browser/notification_source.h"
#include "content/public/common/url_fetcher.h"
#include "googleurl/src/gurl.h"
#include "net/base/network_change_notifier.h"
@@ -97,21 +96,11 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
friend class GoogleURLTrackerInfoBarDelegate;
friend class GoogleURLTrackerTest;
- struct MapEntry {
- MapEntry(); // Required by STL.
- MapEntry(GoogleURLTrackerInfoBarDelegate* infobar,
- const content::NotificationSource& navigation_controller_source,
- const content::NotificationSource& tab_contents_source);
- ~MapEntry();
-
- GoogleURLTrackerInfoBarDelegate* infobar;
- content::NotificationSource navigation_controller_source;
- content::NotificationSource tab_contents_source;
- };
-
- typedef std::map<const InfoBarTabHelper*, MapEntry> InfoBarMap;
+ typedef std::map<const InfoBarTabHelper*,
+ GoogleURLTrackerInfoBarDelegate*> InfoBarMap;
typedef GoogleURLTrackerInfoBarDelegate* (*InfoBarCreator)(
InfoBarTabHelper* infobar_helper,
+ const GURL& search_url,
GoogleURLTracker* google_url_tracker,
const GURL& new_google_url);
@@ -159,36 +148,30 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
// TabContents, respectively, for this load; |infobar_helper| is the
// InfoBarTabHelper of the associated tab; and |search_url| is the actual
// search performed by the user, which if necessary we'll re-do on a new
- // domain later. If there is already a visible GoogleURLTracker infobar for
- // this tab, this function resets its associated navigation entry to point at
- // the new pending entry. Otherwise this function creates a (still-invisible)
- // InfoBarDelegate for the associated tab.
+ // domain later. This function creates a (still-invisible) InfoBarDelegate
+ // for the associated tab and begins listening for the "load committed"
+ // notification that will tell us it's safe to show the infobar.
void OnNavigationPending(
const content::NotificationSource& navigation_controller_source,
const content::NotificationSource& tab_contents_source,
InfoBarTabHelper* infobar_helper,
- int pending_id);
+ const GURL& search_url);
// Called by Observe() once a load we're watching commits, or the associated
- // tab is closed. |infobar_helper| is the same as for OnNavigationPending();
- // |search_url| is valid when this call is due to a successful navigation
- // (indicating that we should show or update the relevant infobar) as opposed
- // to tab closure (which means we should delete the infobar).
- void OnNavigationCommittedOrTabClosed(const InfoBarTabHelper* infobar_helper,
- const GURL& search_url);
+ // tab is closed. The first three args are the same as for
+ // OnNavigationPending(); |navigated| is true when this call is due to a
+ // successful navigation (indicating that we should show our infobar) as
+ // opposed to tab closue (which means we should delete the infobar).
+ void OnNavigationCommittedOrTabClosed(
+ const content::NotificationSource& navigation_controller_source,
+ const content::NotificationSource& tab_contents_source,
+ const InfoBarTabHelper* infobar_helper,
+ bool navigated);
// Closes all open infobars. If |redo_searches| is true, this also triggers
// each tab to re-perform the user's search, but on the new Google TLD.
void CloseAllInfoBars(bool redo_searches);
- // Unregisters any listeners for the notification sources in |map_entry|.
- // This also 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.)
- void UnregisterForEntrySpecificNotifications(
- const MapEntry& map_entry,
- bool must_be_listening_for_commit);
-
Profile* profile_;
content::NotificationRegistrar registrar_;
InfoBarCreator infobar_creator_;
@@ -212,8 +195,6 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
bool need_to_prompt_; // True if the last fetched Google URL is not
// matched with current user's default Google URL
// nor the last prompted Google URL.
- bool search_committed_; // True when we're expecting a notification of a new
- // pending search navigation.
InfoBarMap infobar_map_;
DISALLOW_COPY_AND_ASSIGN(GoogleURLTracker);
@@ -225,6 +206,7 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
class GoogleURLTrackerInfoBarDelegate : public ConfirmInfoBarDelegate {
public:
GoogleURLTrackerInfoBarDelegate(InfoBarTabHelper* infobar_helper,
+ const GURL& search_url,
GoogleURLTracker* google_url_tracker,
const GURL& new_google_url);
@@ -233,8 +215,6 @@ class GoogleURLTrackerInfoBarDelegate : public ConfirmInfoBarDelegate {
virtual bool Cancel() OVERRIDE;
virtual string16 GetLinkText() const OVERRIDE;
virtual bool LinkClicked(WindowOpenDisposition disposition) OVERRIDE;
- virtual bool ShouldExpireInternal(
- const content::LoadCommittedDetails& details) const OVERRIDE;
// Allows GoogleURLTracker to change the Google base URL after the infobar has
// been instantiated. This should only be called with an URL with the same
@@ -242,22 +222,18 @@ class GoogleURLTrackerInfoBarDelegate : public ConfirmInfoBarDelegate {
// correct.
void SetGoogleURL(const GURL& new_google_url);
- bool showing() const { return showing_; }
- void set_pending_id(int pending_id) { pending_id_ = pending_id; }
-
// These are virtual so test code can override them in a subclass.
- virtual void Show(const GURL& search_url);
+ virtual void Show();
virtual void Close(bool redo_search);
protected:
virtual ~GoogleURLTrackerInfoBarDelegate();
InfoBarTabHelper* map_key_; // What |google_url_tracker_| uses to track us.
- GURL search_url_;
+ const GURL search_url_;
GoogleURLTracker* google_url_tracker_;
GURL new_google_url_;
bool showing_; // True if this delegate has been added to a TabContents.
- int pending_id_;
private:
// ConfirmInfoBarDelegate:
diff --git a/chrome/browser/google/google_url_tracker_unittest.cc b/chrome/browser/google/google_url_tracker_unittest.cc
index 506e089..dd52008 100644
--- a/chrome/browser/google/google_url_tracker_unittest.cc
+++ b/chrome/browser/google/google_url_tracker_unittest.cc
@@ -60,33 +60,34 @@ void TestNotificationObserver::Observe(
class TestInfoBarDelegate : public GoogleURLTrackerInfoBarDelegate {
public:
TestInfoBarDelegate(InfoBarTabHelper* infobar_helper,
+ const GURL& search_url,
GoogleURLTracker* google_url_tracker,
const GURL& new_google_url);
virtual ~TestInfoBarDelegate();
GURL search_url() const { return search_url_; }
GURL new_google_url() const { return new_google_url_; }
- int pending_id() const { return pending_id_; }
+ bool showing() const { return showing_; }
private:
// GoogleURLTrackerInfoBarDelegate:
- virtual void Show(const GURL& search_url) OVERRIDE;
+ virtual void Show() OVERRIDE;
virtual void Close(bool redo_search) OVERRIDE;
};
TestInfoBarDelegate::TestInfoBarDelegate(InfoBarTabHelper* infobar_helper,
+ const GURL& search_url,
GoogleURLTracker* google_url_tracker,
const GURL& new_google_url)
- : GoogleURLTrackerInfoBarDelegate(NULL, google_url_tracker, new_google_url) {
+ : GoogleURLTrackerInfoBarDelegate(NULL, search_url, google_url_tracker,
+ new_google_url) {
// We set |map_key_| here instead of in the superclass constructor so that the
// InfoBarDelegate base class will not try to dereference it, which would fail
// since this is really a magic number and not an actual pointer.
map_key_ = infobar_helper;
}
-void TestInfoBarDelegate::Show(const GURL& search_url) {
- search_url_ = search_url;
- pending_id_ = 0;
+void TestInfoBarDelegate::Show() {
showing_ = true;
}
@@ -99,9 +100,10 @@ TestInfoBarDelegate::~TestInfoBarDelegate() {
GoogleURLTrackerInfoBarDelegate* CreateTestInfobar(
InfoBarTabHelper* infobar_helper,
+ const GURL& search_url,
GoogleURLTracker* google_url_tracker,
const GURL& new_google_url) {
- return new TestInfoBarDelegate(infobar_helper, google_url_tracker,
+ return new TestInfoBarDelegate(infobar_helper, search_url, google_url_tracker,
new_google_url);
}
@@ -133,14 +135,11 @@ class GoogleURLTrackerTest : public testing::Test {
GURL google_url() const { return google_url_tracker_->google_url_; }
void SetLastPromptedGoogleURL(const GURL& url);
GURL GetLastPromptedGoogleURL();
- void SetNonSearchPending(int unique_id);
- void SetSearchPending(int unique_id);
- void CommitNonSearch(int unique_id);
- void CommitSearch(const GURL& search_url, int unique_id);
+ void SetSearchPending(const GURL& search_url, int unique_id);
+ void CommitSearch(int unique_id);
void CloseTab(int unique_id);
TestInfoBarDelegate* GetInfoBar(int unique_id);
- void ExpectDefaultURLs() const;
- void ExpectListeningForCommit(int unique_id, bool listening) const;
+ void ExpectDefaultURLs();
scoped_ptr<TestNotificationObserver> observer_;
@@ -234,8 +233,10 @@ GURL GoogleURLTrackerTest::GetLastPromptedGoogleURL() {
return GURL(profile_.GetPrefs()->GetString(prefs::kLastPromptedGoogleURL));
}
-void GoogleURLTrackerTest::SetNonSearchPending(int unique_id) {
+void GoogleURLTrackerTest::SetSearchPending(const GURL& search_url,
+ int unique_id) {
unique_ids_seen_.insert(unique_id);
+ google_url_tracker_->SearchCommitted();
if (google_url_tracker_->registrar_.IsRegistered(google_url_tracker_.get(),
content::NOTIFICATION_NAV_ENTRY_PENDING,
content::NotificationService::AllBrowserContextsAndSources())) {
@@ -243,55 +244,33 @@ void GoogleURLTrackerTest::SetNonSearchPending(int unique_id) {
content::Source<content::NavigationController>(
reinterpret_cast<content::NavigationController*>(unique_id)),
content::Source<TabContents>(reinterpret_cast<TabContents*>(unique_id)),
- reinterpret_cast<InfoBarTabHelper*>(unique_id), unique_id);
- }
-}
-
-void GoogleURLTrackerTest::SetSearchPending(int unique_id) {
- google_url_tracker_->SearchCommitted();
- // Note that the call above might not have actually registered a listener for
- // NOTIFICATION_NAV_ENTRY_PENDING if the searchdomaincheck response was bogus.
- SetNonSearchPending(unique_id);
-}
-
-void GoogleURLTrackerTest::CommitNonSearch(int unique_id) {
- GoogleURLTracker::InfoBarMap::iterator i =
- google_url_tracker_->infobar_map_.find(
- reinterpret_cast<InfoBarTabHelper*>(unique_id));
- if (i != google_url_tracker_->infobar_map_.end()) {
- ExpectListeningForCommit(unique_id, false);
- TestInfoBarDelegate* infobar =
- static_cast<TestInfoBarDelegate*>(i->second.infobar);
- // The infobar should be showing; otherwise the pending non-search should
- // have closed it.
- EXPECT_TRUE(infobar->showing());
- // The pending_id should have been reset to 0 when the non-search became
- // pending.
- EXPECT_EQ(0, infobar->pending_id());
- infobar->InfoBarClosed();
+ reinterpret_cast<InfoBarTabHelper*>(unique_id), search_url);
}
}
-void GoogleURLTrackerTest::CommitSearch(const GURL& search_url, int unique_id) {
+void GoogleURLTrackerTest::CommitSearch(int unique_id) {
+ content::Source<content::NavigationController> source(
+ reinterpret_cast<content::NavigationController*>(unique_id));
if (google_url_tracker_->registrar_.IsRegistered(google_url_tracker_.get(),
- content::NOTIFICATION_NAV_ENTRY_COMMITTED,
- content::Source<content::NavigationController>(
- reinterpret_cast<content::NavigationController*>(unique_id)))) {
- google_url_tracker_->OnNavigationCommittedOrTabClosed(
- reinterpret_cast<InfoBarTabHelper*>(unique_id),
- search_url);
+ content::NOTIFICATION_NAV_ENTRY_COMMITTED, source)) {
+ google_url_tracker_->OnNavigationCommittedOrTabClosed(source,
+ content::Source<TabContents>(reinterpret_cast<TabContents*>(unique_id)),
+ reinterpret_cast<InfoBarTabHelper*>(unique_id), true);
}
}
void GoogleURLTrackerTest::CloseTab(int unique_id) {
unique_ids_seen_.erase(unique_id);
+ content::Source<TabContents> source(
+ reinterpret_cast<TabContents*>(unique_id));
InfoBarTabHelper* infobar_helper =
reinterpret_cast<InfoBarTabHelper*>(unique_id);
if (google_url_tracker_->registrar_.IsRegistered(google_url_tracker_.get(),
- chrome::NOTIFICATION_TAB_CONTENTS_DESTROYED, content::Source<TabContents>(
- reinterpret_cast<TabContents*>(unique_id)))) {
- google_url_tracker_->OnNavigationCommittedOrTabClosed(infobar_helper,
- GURL());
+ chrome::NOTIFICATION_TAB_CONTENTS_DESTROYED, source)) {
+ google_url_tracker_->OnNavigationCommittedOrTabClosed(
+ content::Source<content::NavigationController>(
+ reinterpret_cast<content::NavigationController*>(unique_id)),
+ source, infobar_helper, false);
} else {
// Normally, closing a tab with an infobar showing will close the infobar.
// Since we don't have real tabs and are just faking things with magic
@@ -300,8 +279,8 @@ void GoogleURLTrackerTest::CloseTab(int unique_id) {
google_url_tracker_->infobar_map_.find(infobar_helper);
if (i != google_url_tracker_->infobar_map_.end()) {
TestInfoBarDelegate* infobar =
- static_cast<TestInfoBarDelegate*>(i->second.infobar);
- EXPECT_TRUE(infobar->showing());
+ static_cast<TestInfoBarDelegate*>(i->second);
+ DCHECK(infobar->showing());
infobar->InfoBarClosed();
}
}
@@ -312,25 +291,14 @@ TestInfoBarDelegate* GoogleURLTrackerTest::GetInfoBar(int unique_id) {
google_url_tracker_->infobar_map_.find(
reinterpret_cast<InfoBarTabHelper*>(unique_id));
return (i == google_url_tracker_->infobar_map_.end()) ?
- NULL : static_cast<TestInfoBarDelegate*>(i->second.infobar);
+ NULL : static_cast<TestInfoBarDelegate*>(i->second);
}
-void GoogleURLTrackerTest::ExpectDefaultURLs() const {
+void GoogleURLTrackerTest::ExpectDefaultURLs() {
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL(), fetched_google_url());
}
-void GoogleURLTrackerTest::ExpectListeningForCommit(int unique_id,
- bool listening) const {
- GoogleURLTracker::InfoBarMap::iterator i =
- google_url_tracker_->infobar_map_.find(
- reinterpret_cast<InfoBarTabHelper*>(unique_id));
- ASSERT_FALSE(i == google_url_tracker_->infobar_map_.end());
- EXPECT_EQ(listening, google_url_tracker_->registrar_.IsRegistered(
- google_url_tracker_.get(), content::NOTIFICATION_NAV_ENTRY_COMMITTED,
- i->second.navigation_controller_source));
-}
-
// Tests ----------------------------------------------------------------------
@@ -389,8 +357,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_->notified());
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_TRUE(GetInfoBar(1) == NULL);
// Bad subdomain.
@@ -399,8 +367,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_->notified());
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_TRUE(GetInfoBar(1) == NULL);
// Non-empty path.
@@ -409,8 +377,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_->notified());
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_TRUE(GetInfoBar(1) == NULL);
// Non-empty query.
@@ -419,8 +387,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_->notified());
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_TRUE(GetInfoBar(1) == NULL);
// Non-empty ref.
@@ -429,8 +397,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_->notified());
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_TRUE(GetInfoBar(1) == NULL);
// Complete garbage.
@@ -439,8 +407,8 @@ TEST_F(GoogleURLTrackerTest, DontPromptOnBadReplies) {
EXPECT_EQ(GURL(), fetched_google_url());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_FALSE(observer_->notified());
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_TRUE(GetInfoBar(1) == NULL);
}
@@ -551,8 +519,8 @@ TEST_F(GoogleURLTrackerTest, SearchingDoesNothingIfNoNeedToPrompt) {
EXPECT_TRUE(observer_->notified());
observer_->clear_notified();
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
TestInfoBarDelegate* infobar = GetInfoBar(1);
EXPECT_TRUE(infobar == NULL);
EXPECT_EQ(GURL("http://www.google.co.uk/"), fetched_google_url());
@@ -571,11 +539,13 @@ TEST_F(GoogleURLTrackerTest, TabClosedOnPendingSearch) {
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_->notified());
- SetSearchPending(1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
TestInfoBarDelegate* infobar = GetInfoBar(1);
ASSERT_FALSE(infobar == NULL);
EXPECT_FALSE(infobar->showing());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
+ EXPECT_EQ(GURL("http://www.google.co.uk/search?q=test"),
+ infobar->search_url());
EXPECT_EQ(GURL("http://www.google.co.jp/"), infobar->new_google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_->notified());
@@ -593,8 +563,8 @@ TEST_F(GoogleURLTrackerTest, TabClosedOnCommittedSearch) {
FinishSleep();
MockSearchDomainCheckResponse("http://www.google.co.jp/");
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
TestInfoBarDelegate* infobar = GetInfoBar(1);
ASSERT_FALSE(infobar == NULL);
EXPECT_TRUE(infobar->showing());
@@ -612,8 +582,8 @@ TEST_F(GoogleURLTrackerTest, InfobarClosed) {
FinishSleep();
MockSearchDomainCheckResponse("http://www.google.co.jp/");
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
TestInfoBarDelegate* infobar = GetInfoBar(1);
ASSERT_FALSE(infobar == NULL);
@@ -630,8 +600,8 @@ TEST_F(GoogleURLTrackerTest, InfobarRefused) {
FinishSleep();
MockSearchDomainCheckResponse("http://www.google.co.jp/");
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
TestInfoBarDelegate* infobar = GetInfoBar(1);
ASSERT_FALSE(infobar == NULL);
@@ -648,8 +618,8 @@ TEST_F(GoogleURLTrackerTest, InfobarAccepted) {
FinishSleep();
MockSearchDomainCheckResponse("http://www.google.co.jp/");
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
TestInfoBarDelegate* infobar = GetInfoBar(1);
ASSERT_FALSE(infobar == NULL);
@@ -669,8 +639,8 @@ TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfobars) {
// should close the infobar.
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("http://www.google.co.uk/");
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.com/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_FALSE(GetInfoBar(1) == NULL);
NotifyIPAddressChanged();
MockSearchDomainCheckResponse(google_url().spec());
@@ -680,8 +650,8 @@ TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfobars) {
// As should fetching a URL that differs from the accepted only by the scheme.
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("http://www.google.co.uk/");
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.com/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_FALSE(GetInfoBar(1) == NULL);
NotifyIPAddressChanged();
url_canon::Replacements<char> replacements;
@@ -697,8 +667,8 @@ TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfobars) {
SetLastPromptedGoogleURL(GURL("http://www.google.co.uk/"));
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("http://www.google.co.jp/");
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.com/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_FALSE(GetInfoBar(1) == NULL);
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("http://www.google.co.uk/");
@@ -709,8 +679,8 @@ TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfobars) {
// And one that differs from the last prompted URL only by the scheme.
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("http://www.google.co.jp/");
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.com/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_FALSE(GetInfoBar(1) == NULL);
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("https://www.google.co.uk/");
@@ -721,8 +691,8 @@ TEST_F(GoogleURLTrackerTest, FetchesCanAutomaticallyCloseInfobars) {
// And fetching a different URL entirely.
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("http://www.google.co.jp/");
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
+ SetSearchPending(GURL("http://www.google.com/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_FALSE(GetInfoBar(1) == NULL);
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("https://www.google.co.in/");
@@ -738,158 +708,74 @@ TEST_F(GoogleURLTrackerTest, ResetInfobarGoogleURLs) {
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("http://www.google.co.uk/");
- SetSearchPending(1);
+ SetSearchPending(GURL("http://www.google.com/search?q=test"), 1);
+ CommitSearch(1);
TestInfoBarDelegate* infobar = GetInfoBar(1);
ASSERT_FALSE(infobar == NULL);
EXPECT_EQ(GURL("http://www.google.co.uk/"), infobar->new_google_url());
- // If while an infobar is pending we fetch a new URL that differs from the
- // infobar's only by scheme, the infobar should stay pending but have its
- // Google URL reset.
+ // If while an infobar is showing we fetch a new URL that differs from the
+ // infobar's only by scheme, the infobar should stay open but have its Google
+ // URL reset.
NotifyIPAddressChanged();
MockSearchDomainCheckResponse("https://www.google.co.uk/");
TestInfoBarDelegate* new_infobar = GetInfoBar(1);
ASSERT_FALSE(new_infobar == NULL);
EXPECT_EQ(infobar, new_infobar);
EXPECT_EQ(GURL("https://www.google.co.uk/"), new_infobar->new_google_url());
-
- // Same with an infobar that is showing.
- CommitSearch(GURL("http://www.google.com/search?q=test"), 1);
- EXPECT_TRUE(infobar->showing());
- NotifyIPAddressChanged();
- MockSearchDomainCheckResponse("http://www.google.co.uk/");
- new_infobar = GetInfoBar(1);
- ASSERT_FALSE(new_infobar == NULL);
- EXPECT_EQ(infobar, new_infobar);
- EXPECT_EQ(GURL("http://www.google.co.uk/"), infobar->new_google_url());
- EXPECT_TRUE(infobar->showing());
}
-TEST_F(GoogleURLTrackerTest, NavigationsAfterPendingSearch) {
+TEST_F(GoogleURLTrackerTest, InfobarShownAgainOnSearchAfterPendingSearch) {
SetLastPromptedGoogleURL(GURL("http://www.google.co.uk/"));
RequestServerCheck();
FinishSleep();
MockSearchDomainCheckResponse("http://www.google.co.jp/");
- // A pending non-search after a pending search should close the infobar.
- SetSearchPending(1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
TestInfoBarDelegate* infobar = GetInfoBar(1);
ASSERT_FALSE(infobar == NULL);
EXPECT_FALSE(infobar->showing());
- SetNonSearchPending(1);
- infobar = GetInfoBar(1);
- EXPECT_TRUE(infobar == NULL);
+ EXPECT_EQ(GURL("http://www.google.co.uk/search?q=test"),
+ infobar->search_url());
- // A pending search after a pending search should leave the infobar alive.
- SetSearchPending(1);
- infobar = GetInfoBar(1);
- ASSERT_FALSE(infobar == NULL);
- EXPECT_FALSE(infobar->showing());
- SetSearchPending(1);
- TestInfoBarDelegate* new_infobar = GetInfoBar(1);
- ASSERT_FALSE(new_infobar == NULL);
- EXPECT_EQ(infobar, new_infobar);
- EXPECT_FALSE(infobar->showing());
- ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test2"), 1);
+ TestInfoBarDelegate* infobar2 = GetInfoBar(1);
+ ASSERT_FALSE(infobar2 == NULL);
+ EXPECT_FALSE(infobar2->showing());
+ EXPECT_EQ(GURL("http://www.google.co.uk/search?q=test2"),
+ infobar2->search_url());
- // Committing this search should show the infobar.
- CommitSearch(GURL("http://www.google.co.uk/search?q=test2"), 1);
- EXPECT_TRUE(infobar->showing());
+ CommitSearch(1);
+ EXPECT_TRUE(infobar2->showing());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_->notified());
- ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
}
-TEST_F(GoogleURLTrackerTest, NavigationsAfterCommittedSearch) {
+TEST_F(GoogleURLTrackerTest, InfobarShownAgainOnSearchAfterCommittedSearch) {
SetLastPromptedGoogleURL(GURL("http://www.google.co.uk/"));
RequestServerCheck();
FinishSleep();
MockSearchDomainCheckResponse("http://www.google.co.jp/");
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
- TestInfoBarDelegate* infobar = GetInfoBar(1);
- ASSERT_FALSE(infobar == NULL);
- EXPECT_TRUE(infobar->showing());
- ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
- // A pending non-search on a visible infobar should basically do nothing.
- SetNonSearchPending(1);
- TestInfoBarDelegate* new_infobar = GetInfoBar(1);
- ASSERT_FALSE(new_infobar == NULL);
- EXPECT_EQ(infobar, new_infobar);
- EXPECT_TRUE(infobar->showing());
- EXPECT_EQ(0, infobar->pending_id());
- ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
-
- // As should another pending non-search after the first.
- SetNonSearchPending(1);
- new_infobar = GetInfoBar(1);
- ASSERT_FALSE(new_infobar == NULL);
- EXPECT_EQ(infobar, new_infobar);
- EXPECT_TRUE(infobar->showing());
- EXPECT_EQ(0, infobar->pending_id());
- ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
-
- // Committing this non-search should close the infobar. The control flow in
- // these tests is not really comparable to in the real browser, but at least a
- // few sanity-checks will be performed.
- ASSERT_NO_FATAL_FAILURE(CommitNonSearch(1));
- new_infobar = GetInfoBar(1);
- EXPECT_TRUE(new_infobar == NULL);
-
- // A pending search on a visible infobar should cause the infobar to listen
- // for the search to commit.
- SetSearchPending(1);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
- infobar = GetInfoBar(1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
+ TestInfoBarDelegate* infobar = GetInfoBar(1);
ASSERT_FALSE(infobar == NULL);
EXPECT_TRUE(infobar->showing());
- SetSearchPending(1);
- new_infobar = GetInfoBar(1);
- ASSERT_FALSE(new_infobar == NULL);
- EXPECT_EQ(infobar, new_infobar);
- EXPECT_TRUE(infobar->showing());
- EXPECT_EQ(1, infobar->pending_id());
- ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
- // But a non-search after this should cancel that state.
- SetNonSearchPending(1);
- new_infobar = GetInfoBar(1);
- ASSERT_FALSE(new_infobar == NULL);
- EXPECT_EQ(infobar, new_infobar);
- EXPECT_TRUE(infobar->showing());
- EXPECT_EQ(0, infobar->pending_id());
- ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
-
- // Another pending search after the non-search should put us back into
- // "waiting for commit" mode.
- SetSearchPending(1);
- new_infobar = GetInfoBar(1);
- ASSERT_FALSE(new_infobar == NULL);
- EXPECT_EQ(infobar, new_infobar);
- EXPECT_TRUE(infobar->showing());
- EXPECT_EQ(1, infobar->pending_id());
- ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
+ // In real usage, the upcoming pending navigation for "test2" would close
+ // |infobar|. Since we're not actually doing navigations, we need to clean it
+ // up manually to prevent leaks.
+ delete infobar;
- // A second pending search after the first should not really change anything.
- SetSearchPending(1);
- new_infobar = GetInfoBar(1);
- ASSERT_FALSE(new_infobar == NULL);
- EXPECT_EQ(infobar, new_infobar);
- EXPECT_TRUE(infobar->showing());
- EXPECT_EQ(1, infobar->pending_id());
- ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test2"), 1);
+ TestInfoBarDelegate* infobar2 = GetInfoBar(1);
+ ASSERT_FALSE(infobar2 == NULL);
+ EXPECT_FALSE(infobar2->showing());
- // Committing this search should change the visible infobar's search_url.
- CommitSearch(GURL("http://www.google.co.uk/search?q=test2"), 1);
- new_infobar = GetInfoBar(1);
- ASSERT_FALSE(new_infobar == NULL);
- EXPECT_EQ(infobar, new_infobar);
- EXPECT_TRUE(infobar->showing());
- EXPECT_EQ(GURL("http://www.google.co.uk/search?q=test2"), infobar->search_url());
- EXPECT_EQ(0, infobar->pending_id());
- ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
+ CommitSearch(1);
+ EXPECT_TRUE(infobar2->showing());
EXPECT_EQ(GURL(GoogleURLTracker::kDefaultGoogleHomepage), google_url());
EXPECT_EQ(GURL("http://www.google.co.uk/"), GetLastPromptedGoogleURL());
EXPECT_FALSE(observer_->notified());
@@ -901,33 +787,37 @@ TEST_F(GoogleURLTrackerTest, MultipleInfobars) {
FinishSleep();
MockSearchDomainCheckResponse("http://www.google.co.jp/");
- SetSearchPending(1);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test"), 1);
TestInfoBarDelegate* infobar = GetInfoBar(1);
ASSERT_FALSE(infobar == NULL);
EXPECT_FALSE(infobar->showing());
+ EXPECT_EQ(GURL("http://www.google.co.uk/search?q=test"),
+ infobar->search_url());
- SetSearchPending(2);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test2"), 2);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test2"), 2);
+ CommitSearch(2);
TestInfoBarDelegate* infobar2 = GetInfoBar(2);
ASSERT_FALSE(infobar2 == NULL);
EXPECT_TRUE(infobar2->showing());
EXPECT_EQ(GURL("http://www.google.co.uk/search?q=test2"),
infobar2->search_url());
- SetSearchPending(3);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test3"), 3);
TestInfoBarDelegate* infobar3 = GetInfoBar(3);
ASSERT_FALSE(infobar3 == NULL);
EXPECT_FALSE(infobar3->showing());
+ EXPECT_EQ(GURL("http://www.google.co.uk/search?q=test3"),
+ infobar3->search_url());
- SetSearchPending(4);
- CommitSearch(GURL("http://www.google.co.uk/search?q=test4"), 4);
+ SetSearchPending(GURL("http://www.google.co.uk/search?q=test4"), 4);
+ CommitSearch(4);
TestInfoBarDelegate* infobar4 = GetInfoBar(4);
ASSERT_FALSE(infobar4 == NULL);
EXPECT_TRUE(infobar4->showing());
EXPECT_EQ(GURL("http://www.google.co.uk/search?q=test4"),
infobar4->search_url());
- CommitSearch(GURL("http://www.google.co.uk/search?q=test"), 1);
+ CommitSearch(1);
EXPECT_TRUE(infobar->showing());
infobar2->InfoBarClosed();