Commit 7f0f5c76 authored by blundell@chromium.org's avatar blundell@chromium.org

Eliminate dependence of GoogleURLTracker et al. on InfoBarService

This CL changes the dependence of GoogleURLTracker(InfoBarDelegate, MapEntry)
on InfoBarService to instead be a dependence on infobars::InfoBarManager. The
only reason that InfoBarManager wasn't sufficient was that
GoogleURLTrackerInfoBarDelegate obtained the WebContents from the
InfoBarService. This flow is replaced by introducing an OpenURL() API on
GoogleURLTrackerNavigationHelper (GoogleURLTrackerNavigationHelperImpl
implements this API by calling through to its WebContents instance, which is
the same instance associated with the InfoBarService).

BUG=373231,373233

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273821 0039d316-1c4b-4281-b951-d872f2087c98
parent 51994b2a
......@@ -13,14 +13,12 @@
#include "chrome/browser/google/google_url_tracker_infobar_delegate.h"
#include "chrome/browser/google/google_url_tracker_navigation_helper.h"
#include "chrome/browser/google/google_util.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "components/google/core/browser/google_url_tracker_client.h"
#include "components/infobars/core/infobar.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
#include "components/infobars/core/infobar_manager.h"
#include "content/public/browser/notification_service.h"
#include "net/base/load_flags.h"
#include "net/base/net_util.h"
......@@ -206,11 +204,11 @@ void GoogleURLTracker::Shutdown() {
net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
}
void GoogleURLTracker::DeleteMapEntryForService(
const InfoBarService* infobar_service) {
// WARNING: |infobar_service| may point to a deleted object. Do not
void GoogleURLTracker::DeleteMapEntryForManager(
const infobars::InfoBarManager* infobar_manager) {
// WARNING: |infobar_manager| may point to a deleted object. Do not
// dereference it! See OnTabClosed().
EntryMap::iterator i(entry_map_.find(infobar_service));
EntryMap::iterator i(entry_map_.find(infobar_manager));
DCHECK(i != entry_map_.end());
GoogleURLTrackerMapEntry* map_entry = i->second;
......@@ -277,11 +275,11 @@ void GoogleURLTracker::SearchCommitted() {
void GoogleURLTracker::OnNavigationPending(
scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper,
InfoBarService* infobar_service,
infobars::InfoBarManager* infobar_manager,
int pending_id) {
GoogleURLTrackerMapEntry* map_entry = NULL;
EntryMap::iterator i(entry_map_.find(infobar_service));
EntryMap::iterator i(entry_map_.find(infobar_manager));
if (i != entry_map_.end())
map_entry = i->second;
......@@ -295,9 +293,9 @@ void GoogleURLTracker::OnNavigationPending(
// 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());
this, infobar_manager, nav_helper.Pass());
map_entry->navigation_helper()->SetListeningForTabDestruction(true);
entry_map_.insert(std::make_pair(infobar_service, map_entry));
entry_map_.insert(std::make_pair(infobar_manager, map_entry));
} else if (map_entry->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);
......@@ -334,9 +332,10 @@ void GoogleURLTracker::OnNavigationPending(
}
}
void GoogleURLTracker::OnNavigationCommitted(InfoBarService* infobar_service,
const GURL& search_url) {
EntryMap::iterator i(entry_map_.find(infobar_service));
void GoogleURLTracker::OnNavigationCommitted(
infobars::InfoBarManager* infobar_manager,
const GURL& search_url) {
EntryMap::iterator i(entry_map_.find(infobar_manager));
DCHECK(i != entry_map_.end());
GoogleURLTrackerMapEntry* map_entry = i->second;
DCHECK(search_url.is_valid());
......@@ -345,8 +344,8 @@ void GoogleURLTracker::OnNavigationCommitted(InfoBarService* infobar_service,
if (map_entry->has_infobar_delegate()) {
map_entry->infobar_delegate()->Update(search_url);
} else {
infobars::InfoBar* infobar =
infobar_creator_.Run(infobar_service, this, search_url);
infobars::InfoBar* infobar = infobar_creator_.Run(
infobar_manager, this, search_url);
if (infobar) {
map_entry->SetInfoBarDelegate(
static_cast<GoogleURLTrackerInfoBarDelegate*>(infobar->delegate()));
......@@ -358,11 +357,11 @@ void GoogleURLTracker::OnNavigationCommitted(InfoBarService* infobar_service,
void GoogleURLTracker::OnTabClosed(
GoogleURLTrackerNavigationHelper* nav_helper) {
// Because InfoBarService tears itself down on tab destruction, it's possible
// to get a non-NULL InfoBarService pointer here, depending on which order
// Because InfoBarManager tears itself down on tab destruction, it's possible
// to get a non-NULL InfoBarManager pointer here, depending on which order
// notifications fired in. Likewise, the pointer in |entry_map_| (and in its
// associated MapEntry) may point to deleted memory. Therefore, if we were to
// access the InfoBarService* we have for this tab, we'd need to ensure we
// associated MapEntry) may point to deleted memory. Therefore, if we were
// to access the InfoBarManager* we have for this tab, we'd need to ensure we
// just looked at the raw pointer value, and never dereferenced it. This
// function doesn't need to do even that, but others in the call chain from
// here might (and have comments pointing back here).
......
......@@ -104,25 +104,26 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
const GURL& fetched_google_url() const { return fetched_google_url_; }
// No one but GoogleURLTrackerMapEntry should call this.
void DeleteMapEntryForService(const InfoBarService* infobar_service);
void DeleteMapEntryForManager(
const infobars::InfoBarManager* infobar_manager);
// 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;
// navigation; |infobar_manager| is the InfoBarManager 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,
InfoBarService* infobar_service,
infobars::InfoBarManager* infobar_manager,
int pending_id);
// Called by the navigation observer once a load we're watching commits.
// |infobar_service| is the same as for OnNavigationPending();
// |infobar_manager| is the same as for OnNavigationPending();
// |search_url| is guaranteed to be valid.
virtual void OnNavigationCommitted(InfoBarService* infobar_service,
virtual void OnNavigationCommitted(infobars::InfoBarManager* infobar_manager,
const GURL& search_url);
// Called by the navigation observer when a tab closes.
......@@ -134,7 +135,8 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
private:
friend class GoogleURLTrackerTest;
typedef std::map<const InfoBarService*, GoogleURLTrackerMapEntry*> EntryMap;
typedef std::map<const infobars::InfoBarManager*, GoogleURLTrackerMapEntry*>
EntryMap;
// net::URLFetcherDelegate:
virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
......@@ -186,12 +188,12 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
scoped_ptr<GoogleURLTrackerClient> client_;
// 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.
base::Callback<
infobars::InfoBar*(InfoBarService*, GoogleURLTracker*, const GURL&)>
infobar_creator_;
// Creates an infobar and adds it to the provided InfoBarManager. Returns
// the infobar on success or NULL on failure. The caller does not own the
// returned object, the InfoBarManager does.
base::Callback<infobars::InfoBar*(infobars::InfoBarManager*,
GoogleURLTracker*,
const GURL&)> infobar_creator_;
GURL google_url_;
GURL fetched_google_url_;
......
......@@ -5,12 +5,10 @@
#include "chrome/browser/google/google_url_tracker_infobar_delegate.h"
#include "chrome/browser/google/google_url_tracker.h"
#include "chrome/browser/google/google_url_tracker_navigation_helper.h"
#include "chrome/browser/google/google_util.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "components/infobars/core/infobar.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/page_navigator.h"
#include "content/public/browser/web_contents.h"
#include "components/infobars/core/infobar_manager.h"
#include "grit/generated_resources.h"
#include "net/base/net_util.h"
#include "ui/base/l10n/l10n_util.h"
......@@ -18,10 +16,10 @@
// static
infobars::InfoBar* GoogleURLTrackerInfoBarDelegate::Create(
InfoBarService* infobar_service,
infobars::InfoBarManager* infobar_manager,
GoogleURLTracker* google_url_tracker,
const GURL& search_url) {
return infobar_service->AddInfoBar(ConfirmInfoBarDelegate::CreateInfoBar(
return infobar_manager->AddInfoBar(ConfirmInfoBarDelegate::CreateInfoBar(
scoped_ptr<ConfirmInfoBarDelegate>(new GoogleURLTrackerInfoBarDelegate(
google_url_tracker, search_url))));
}
......@@ -58,16 +56,15 @@ void GoogleURLTrackerInfoBarDelegate::Close(bool redo_search) {
new_search_url = search_url_.ReplaceComponents(replacements);
}
content::WebContents* contents =
InfoBarService::WebContentsFromInfoBar(infobar());
// Take ownership of |navigation_helper_| in order to ensure that it stays
// alive for the duration of this method.
scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper =
navigation_helper_.Pass();
infobar()->RemoveSelf();
// WARNING: |this| may be deleted at this point! Do not access any members!
if (new_search_url.is_valid()) {
contents->OpenURL(content::OpenURLParams(
new_search_url, content::Referrer(), CURRENT_TAB,
content::PAGE_TRANSITION_GENERATED, false));
}
if (new_search_url.is_valid())
navigation_helper->OpenURL(new_search_url, CURRENT_TAB, false);
}
GoogleURLTrackerInfoBarDelegate::GoogleURLTrackerInfoBarDelegate(
......@@ -107,14 +104,12 @@ base::string16 GoogleURLTrackerInfoBarDelegate::GetLinkText() const {
bool GoogleURLTrackerInfoBarDelegate::LinkClicked(
WindowOpenDisposition disposition) {
InfoBarService::WebContentsFromInfoBar(infobar())->OpenURL(
content::OpenURLParams(
google_util::AppendGoogleLocaleParam(GURL(
"https://www.google.com/support/chrome/bin/answer.py?"
"answer=1618699")),
content::Referrer(),
(disposition == CURRENT_TAB) ? NEW_FOREGROUND_TAB : disposition,
content::PAGE_TRANSITION_LINK, false));
navigation_helper_->OpenURL(
google_util::AppendGoogleLocaleParam(GURL(
"https://www.google.com/support/chrome/bin/answer.py?"
"answer=1618699")),
(disposition == CURRENT_TAB) ? NEW_FOREGROUND_TAB : disposition,
true);
return false;
}
......
......@@ -9,22 +9,37 @@
#include "url/gurl.h"
class GoogleURLTracker;
class InfoBarService;
class GoogleURLTrackerNavigationHelper;
namespace infobars {
class InfoBarManager;
}
// This infobar is shown by the GoogleURLTracker when the Google base URL has
// changed.
class GoogleURLTrackerInfoBarDelegate : public ConfirmInfoBarDelegate {
public:
// Creates a Google URL tracker infobar and delegate and adds the infobar to
// |infobar_service|. Returns the infobar if it was successfully added.
static infobars::InfoBar* Create(InfoBarService* infobar_service,
GoogleURLTracker* google_url_tracker,
const GURL& search_url);
// |infobar_manager|. Returns the infobar if it was successfully added.
static infobars::InfoBar* Create(
infobars::InfoBarManager* infobar_manager,
GoogleURLTracker* google_url_tracker,
const GURL& search_url);
// ConfirmInfoBarDelegate:
virtual bool Accept() OVERRIDE;
virtual bool Cancel() OVERRIDE;
GoogleURLTrackerNavigationHelper* navigation_helper() {
return navigation_helper_weak_ptr_;
}
void set_navigation_helper(
scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper) {
navigation_helper_ = navigation_helper.Pass();
navigation_helper_weak_ptr_ = navigation_helper_.get();
}
// Other than set_pending_id(), these accessors are only used by test code.
const GURL& search_url() const { return search_url_; }
void set_search_url(const GURL& search_url) { search_url_ = search_url; }
......@@ -36,8 +51,9 @@ class GoogleURLTrackerInfoBarDelegate : public ConfirmInfoBarDelegate {
virtual void Close(bool redo_search);
protected:
GoogleURLTrackerInfoBarDelegate(GoogleURLTracker* google_url_tracker,
const GURL& search_url);
GoogleURLTrackerInfoBarDelegate(
GoogleURLTracker* google_url_tracker,
const GURL& search_url);
virtual ~GoogleURLTrackerInfoBarDelegate();
private:
......@@ -50,6 +66,14 @@ class GoogleURLTrackerInfoBarDelegate : public ConfirmInfoBarDelegate {
const NavigationDetails& details) const OVERRIDE;
GoogleURLTracker* google_url_tracker_;
scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper_;
// During Close(), this object gives up ownership of |navigation_helper_|,
// which then outlives this object. Sometimes after this point, other classes
// still attempt to call navigation_helper() to access the (still-valid)
// instance. The NavigationHelper instance is stored as a weak pointer in
// addition to a strong pointer to facilitate this case.
GoogleURLTrackerNavigationHelper* navigation_helper_weak_ptr_;
GURL search_url_;
int pending_id_;
......
......@@ -6,18 +6,16 @@
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/google/google_url_tracker.h"
#include "chrome/browser/google/google_url_tracker_infobar_delegate.h"
#include "components/infobars/core/infobar.h"
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_source.h"
GoogleURLTrackerMapEntry::GoogleURLTrackerMapEntry(
GoogleURLTracker* google_url_tracker,
InfoBarService* infobar_service,
infobars::InfoBarManager* infobar_manager,
scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper)
: google_url_tracker_(google_url_tracker),
infobar_service_(infobar_service),
infobar_manager_(infobar_manager),
infobar_delegate_(NULL),
navigation_helper_(navigation_helper.Pass()) {
}
......@@ -31,10 +29,11 @@ void GoogleURLTrackerMapEntry::Observe(
const content::NotificationDetails& details) {
DCHECK(infobar_delegate_);
DCHECK_EQ(chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED, type);
DCHECK_EQ(infobar_service_, content::Source<InfoBarService>(source).ptr());
DCHECK_EQ(infobar_manager_,
content::Source<infobars::InfoBarManager>(source).ptr());
if (content::Details<infobars::InfoBar::RemovedDetails>(
details)->first->delegate() == infobar_delegate_) {
google_url_tracker_->DeleteMapEntryForService(infobar_service_);
google_url_tracker_->DeleteMapEntryForManager(infobar_manager_);
// WARNING: At this point |this| has been deleted!
}
}
......@@ -42,18 +41,24 @@ void GoogleURLTrackerMapEntry::Observe(
void GoogleURLTrackerMapEntry::SetInfoBarDelegate(
GoogleURLTrackerInfoBarDelegate* infobar_delegate) {
DCHECK(!infobar_delegate_);
// Transfer ownership of |navigation_helper_| to the infobar delegate as the
// infobar delegate has need of it after this object has been destroyed in
// the case where the user accepts the new Google URL.
infobar_delegate->set_navigation_helper(navigation_helper_.Pass());
infobar_delegate_ = infobar_delegate;
registrar_.Add(this, chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
content::Source<InfoBarService>(infobar_service_));
registrar_.Add(this,
chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
content::Source<infobars::InfoBarManager>(infobar_manager_));
}
void GoogleURLTrackerMapEntry::Close(bool redo_search) {
if (infobar_delegate_) {
infobar_delegate_->Close(redo_search);
} else {
// WARNING: |infobar_service_| may point to a deleted object. Do not
// WARNING: |infobar_manager_| may point to a deleted object. Do not
// dereference it! See GoogleURLTracker::OnTabClosed().
google_url_tracker_->DeleteMapEntryForService(infobar_service_);
google_url_tracker_->DeleteMapEntryForManager(infobar_manager_);
}
// WARNING: At this point |this| has been deleted!
}
......@@ -6,19 +6,22 @@
#define CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_MAP_ENTRY_H_
#include "base/memory/scoped_ptr.h"
#include "chrome/browser/google/google_url_tracker_infobar_delegate.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"
class GoogleURLTracker;
class GoogleURLTrackerInfoBarDelegate;
class InfoBarService;
namespace infobars {
class InfoBarManager;
}
class GoogleURLTrackerMapEntry : public content::NotificationObserver {
public:
GoogleURLTrackerMapEntry(
GoogleURLTracker* google_url_tracker,
InfoBarService* infobar_service,
infobars::InfoBarManager* infobar_manager,
scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper);
virtual ~GoogleURLTrackerMapEntry();
......@@ -29,7 +32,10 @@ class GoogleURLTrackerMapEntry : public content::NotificationObserver {
void SetInfoBarDelegate(GoogleURLTrackerInfoBarDelegate* infobar_delegate);
GoogleURLTrackerNavigationHelper* navigation_helper() {
return navigation_helper_.get();
// This object gives ownership of |navigation_helper_| to the infobar
// delegate in SetInfoBarDelegate().
return has_infobar_delegate() ?
infobar_delegate_->navigation_helper() : navigation_helper_.get();
}
void Close(bool redo_search);
......@@ -44,7 +50,7 @@ class GoogleURLTrackerMapEntry : public content::NotificationObserver {
content::NotificationRegistrar registrar_;
GoogleURLTracker* const google_url_tracker_;
const InfoBarService* const infobar_service_;
const infobars::InfoBarManager* const infobar_manager_;
GoogleURLTrackerInfoBarDelegate* infobar_delegate_;
scoped_ptr<GoogleURLTrackerNavigationHelper> navigation_helper_;
......
......@@ -6,11 +6,14 @@
#define CHROME_BROWSER_GOOGLE_GOOGLE_URL_TRACKER_NAVIGATION_HELPER_H_
#include "base/macros.h"
#include "ui/base/window_open_disposition.h"
class GoogleURLTracker;
class GURL;
// A helper class for GoogleURLTracker that abstracts the details of listening
// for various navigation events.
// Interface via which GoogleURLTracker communicates with its driver.
// TODO(blundell): Rename this class to GoogleURLTrackerDriver.
// crbug.com/373221
class GoogleURLTrackerNavigationHelper {
public:
explicit GoogleURLTrackerNavigationHelper(
......@@ -34,6 +37,11 @@ class GoogleURLTrackerNavigationHelper {
// destruction.
virtual bool IsListeningForTabDestruction() = 0;
// Opens |url| with the given window disposition.
virtual void OpenURL(GURL url,
WindowOpenDisposition disposition,
bool user_clicked_on_link) = 0;
protected:
GoogleURLTracker* google_url_tracker() { return google_url_tracker_; }
......
......@@ -20,6 +20,7 @@ GoogleURLTrackerNavigationHelperImpl::GoogleURLTrackerNavigationHelperImpl(
}
GoogleURLTrackerNavigationHelperImpl::~GoogleURLTrackerNavigationHelperImpl() {
web_contents_ = NULL;
}
void GoogleURLTrackerNavigationHelperImpl::SetListeningForNavigationCommit(
......@@ -67,6 +68,16 @@ bool GoogleURLTrackerNavigationHelperImpl::IsListeningForTabDestruction() {
content::Source<content::WebContents>(web_contents_));
}
void GoogleURLTrackerNavigationHelperImpl::OpenURL(
GURL url,
WindowOpenDisposition disposition,
bool user_clicked_on_link) {
content::PageTransition transition_type = user_clicked_on_link ?
content::PAGE_TRANSITION_LINK : content::PAGE_TRANSITION_GENERATED;
web_contents_->OpenURL(content::OpenURLParams(
url, content::Referrer(), disposition, transition_type, false));
}
void GoogleURLTrackerNavigationHelperImpl::Observe(
int type,
const content::NotificationSource& source,
......
......@@ -29,6 +29,9 @@ class GoogleURLTrackerNavigationHelperImpl
virtual void SetListeningForTabDestruction(
bool listen) OVERRIDE;
virtual bool IsListeningForTabDestruction() OVERRIDE;
virtual void OpenURL(GURL url,
WindowOpenDisposition disposition,
bool user_clicked_on_link) OVERRIDE;
private:
// content::NotificationObserver:
......
......@@ -32,20 +32,21 @@ namespace {
class TestInfoBarDelegate : public GoogleURLTrackerInfoBarDelegate {
public:
// Creates a test infobar and delegate and returns the infobar. Unlike the
// parent class, this does not add the infobar to |infobar_service|, since
// that "pointer" is really just a magic number. Thus there is no
// InfoBarService ownership of the returned object; and since the caller
// doesn't own the returned object, we rely on |test_harness| cleaning this up
// eventually in GoogleURLTrackerTest::OnInfoBarClosed() to avoid leaks.
static infobars::InfoBar* Create(GoogleURLTrackerTest* test_harness,
InfoBarService* infobar_service,
GoogleURLTracker* google_url_tracker,
const GURL& search_url);
// Creates a test infobar and delegate and returns the infobar. Unlike the
// parent class, this does not add the infobar to |infobar_manager|, since
// that "pointer" is really just a magic number. Thus there is no
// InfoBarManager ownership of the returned object; and since the caller
// doesn't own the returned object, we rely on |test_harness| cleaning this
// up eventually in GoogleURLTrackerTest::OnInfoBarClosed() to avoid leaks.
static infobars::InfoBar* Create(
GoogleURLTrackerTest* test_harness,
infobars::InfoBarManager* infobar_manager,
GoogleURLTracker* google_url_tracker,
const GURL& search_url);
private:
TestInfoBarDelegate(GoogleURLTrackerTest* test_harness,
InfoBarService* infobar_service,
infobars::InfoBarManager* infobar_manager,
GoogleURLTracker* google_url_tracker,
const GURL& search_url);
virtual ~TestInfoBarDelegate();
......@@ -55,7 +56,7 @@ class TestInfoBarDelegate : public GoogleURLTrackerInfoBarDelegate {
virtual void Close(bool redo_search) OVERRIDE;
GoogleURLTrackerTest* test_harness_;
InfoBarService* infobar_service_;
infobars::InfoBarManager* infobar_manager_;
DISALLOW_COPY_AND_ASSIGN(TestInfoBarDelegate);
};
......@@ -150,6 +151,9 @@ class TestGoogleURLTrackerNavigationHelper
virtual bool IsListeningForNavigationCommit() OVERRIDE;
virtual void SetListeningForTabDestruction(bool listen) OVERRIDE;
virtual bool IsListeningForTabDestruction() OVERRIDE;
virtual void OpenURL(GURL url,
WindowOpenDisposition disposition,
bool user_clicked_on_link) OVERRIDE;
private:
bool listening_for_nav_commit_;
......@@ -186,6 +190,12 @@ bool TestGoogleURLTrackerNavigationHelper::IsListeningForTabDestruction() {
return listening_for_tab_destruction_;
}
void TestGoogleURLTrackerNavigationHelper::OpenURL(
GURL url,
WindowOpenDisposition disposition,
bool user_clicked_on_link) {
}
} // namespace
......@@ -194,7 +204,8 @@ bool TestGoogleURLTrackerNavigationHelper::IsListeningForTabDestruction() {
// Ths class exercises GoogleURLTracker. In order to avoid instantiating more
// of the Chrome infrastructure than necessary, the GoogleURLTracker functions
// are carefully written so that many of the functions which take
// NavigationController* or InfoBarService* do not actually dereference the
// NavigationController* or infobars::InfoBarManager* do not actually
// dereference the
// objects, merely use them for comparisons and lookups, e.g. in |entry_map_|.
// This then allows the test code here to not create any of these objects, and
// instead supply "pointers" that are actually reinterpret_cast<>()ed magic
......@@ -210,7 +221,7 @@ class GoogleURLTrackerTest : public testing::Test {
public:
// Called by TestInfoBarDelegate::Close().
void OnInfoBarClosed(scoped_ptr<infobars::InfoBar> infobar,
InfoBarService* infobar_service);
infobars::InfoBarManager* infobar_manager);
protected:
GoogleURLTrackerTest();
......@@ -247,13 +258,14 @@ class GoogleURLTrackerTest : public testing::Test {
void clear_listener_notified() { listener_.clear_notified(); }
private:
// Since |infobar_service| is really a magic number rather than an actual
// Since |infobar_manager| is really a magic number rather than an actual
// object, we don't add the created infobar to it. Instead we will simulate
// any helper<->infobar interaction necessary. The returned object will be
// cleaned up in OnInfoBarClosed().
infobars::InfoBar* CreateTestInfoBar(InfoBarService* infobar_service,
GoogleURLTracker* google_url_tracker,
const GURL& search_url);
infobars::InfoBar* CreateTestInfoBar(
infobars::InfoBarManager* infobar_manager,
GoogleURLTracker* google_url_tracker,
const GURL& search_url);
// These are required by the TestURLFetchers GoogleURLTracker will create (see
// test_url_fetcher_factory.h).
......@@ -273,17 +285,19 @@ class GoogleURLTrackerTest : public testing::Test {
void GoogleURLTrackerTest::OnInfoBarClosed(
scoped_ptr<infobars::InfoBar> infobar,
InfoBarService* infobar_service) {
// First, simulate the InfoBarService firing INFOBAR_REMOVED.
infobars::InfoBarManager* infobar_manager) {
// First, simulate the InfoBarManager firing INFOBAR_REMOVED.
// TODO(droger): Replace this flow with a call to the observer method once
// the map entry is observing InfoBarManager. crbug.com/373243
infobars::InfoBar::RemovedDetails removed_details(infobar.get(), false);
GoogleURLTracker::EntryMap::const_iterator i =
google_url_tracker_->entry_map_.find(infobar_service);
google_url_tracker_->entry_map_.find(infobar_manager);
ASSERT_FALSE(i == google_url_tracker_->entry_map_.end());
GoogleURLTrackerMapEntry* map_entry = i->second;
ASSERT_EQ(infobar->delegate(), map_entry->infobar_delegate());
map_entry->Observe(
chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
content::Source<InfoBarService>(infobar_service),
content::Source<infobars::InfoBarManager>(infobar_manager),
content::Details<infobars::InfoBar::RemovedDetails>(&removed_details));
// Second, simulate the infobar container closing the infobar in response.
......@@ -374,7 +388,7 @@ void GoogleURLTrackerTest::SetNavigationPending(intptr_t unique_id,
scoped_ptr<GoogleURLTrackerNavigationHelper>(
new TestGoogleURLTrackerNavigationHelper(
google_url_tracker_.get())),
reinterpret_cast<InfoBarService*>(unique_id),
reinterpret_cast<infobars::InfoBarManager*>(unique_id),
unique_id);
}
}
......@@ -404,7 +418,7 @@ void GoogleURLTrackerTest::CommitSearch(intptr_t unique_id,
GoogleURLTrackerNavigationHelper* nav_helper = GetNavigationHelper(unique_id);
if (nav_helper && nav_helper->IsListeningForNavigationCommit()) {
google_url_tracker_->OnNavigationCommitted(
reinterpret_cast<InfoBarService*>(unique_id), search_url);
reinterpret_cast<infobars::InfoBarManager*>(unique_id), search_url);
}
}
......@@ -425,7 +439,7 @@ GoogleURLTrackerMapEntry* GoogleURLTrackerTest::GetMapEntry(
intptr_t unique_id) {
GoogleURLTracker::EntryMap::const_iterator i =
google_url_tracker_->entry_map_.find(
reinterpret_cast<InfoBarService*>(unique_id));
reinterpret_cast<infobars::InfoBarManager*>(unique_id));
return (i == google_url_tracker_->entry_map_.end()) ? NULL : i->second;
}
......@@ -458,11 +472,11 @@ void GoogleURLTrackerTest::ExpectListeningForCommit(intptr_t unique_id,
}
infobars::InfoBar* GoogleURLTrackerTest::CreateTestInfoBar(
InfoBarService* infobar_service,
infobars::InfoBarManager* infobar_manager,
GoogleURLTracker* google_url_tracker,
const GURL& search_url) {
return TestInfoBarDelegate::Create(this, infobar_service, google_url_tracker,
search_url);
return TestInfoBarDelegate::Create(
this, infobar_manager, google_url_tracker, search_url);
}
......@@ -473,22 +487,26 @@ namespace {
// static
infobars::InfoBar* TestInfoBarDelegate::Create(
GoogleURLTrackerTest* test_harness,
InfoBarService* infobar_service,
infobars::InfoBarManager* infobar_manager,
GoogleURLTracker* google_url_tracker,
const GURL& search_url) {
return ConfirmInfoBarDelegate::CreateInfoBar(
scoped_ptr<ConfirmInfoBarDelegate>(new TestInfoBarDelegate(
test_harness, infobar_service, google_url_tracker,
test_harness,
infobar_manager,
google_url_tracker,
search_url))).release();
}
TestInfoBarDelegate::TestInfoBarDelegate(GoogleURLTrackerTest* test_harness,
InfoBarService* infobar_service,
GoogleURLTracker* google_url_tracker,
const GURL& search_url)
: GoogleURLTrackerInfoBarDelegate(google_url_tracker, search_url),
test_harness_(test_harness),
infobar_service_(infobar_service) {
TestInfoBarDelegate::TestInfoBarDelegate(
GoogleURLTrackerTest* test_harness,
infobars::InfoBarManager* infobar_manager,
GoogleURLTracker* google_url_tracker,
const GURL& search_url)
: GoogleURLTrackerInfoBarDelegate(google_url_tracker,
search_url),
test_harness_(test_harness),
infobar_manager_(infobar_manager) {
}
TestInfoBarDelegate::~TestInfoBarDelegate() {
......@@ -501,7 +519,7 @@ void TestInfoBarDelegate::Update(const GURL& search_url) {
void TestInfoBarDelegate::Close(bool redo_search) {
test_harness_->OnInfoBarClosed(scoped_ptr<infobars::InfoBar>(infobar()),
infobar_service_);
infobar_manager_);
// WARNING: At this point |this| has been deleted!
}
......
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