chrome/browser/google/google_url_tracker_unittest.cc - Issue 290453005: Remove Infobars notifications from GoogleURLTrackerMapEntry

Unified Diff: chrome/browser/google/google_url_tracker_unittest.cc

Issue 290453005: Remove Infobars notifications from GoogleURLTrackerMapEntry (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: review comments Created 6 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: chrome/browser/google/google_url_tracker_unittest.cc

diff --git a/chrome/browser/google/google_url_tracker_unittest.cc b/chrome/browser/google/google_url_tracker_unittest.cc

index 6b2fedf80f4973e100919f6b575e168c3f55f8eb..cb4931019ec4e0d970d7cbc7657e838d80f6deb4 100644

--- a/chrome/browser/google/google_url_tracker_unittest.cc

+++ b/chrome/browser/google/google_url_tracker_unittest.cc

@@ -1,4 +1,4 @@

// Use of this source code is governed by a BSD-style license that can be

// found in the LICENSE file.

@@ -7,19 +7,24 @@

#include <set>

#include <string>

+#include "base/containers/scoped_ptr_hash_map.h"

+#include "base/memory/scoped_ptr.h"

#include "base/message_loop/message_loop.h"

#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_url_tracker_infobar_delegate.h"

#include "chrome/browser/google/google_url_tracker_navigation_helper.h"

+#include "chrome/browser/infobars/infobar_service.h"

#include "chrome/common/pref_names.h"

+#include "chrome/test/base/chrome_render_view_host_test_harness.h"

#include "chrome/test/base/testing_profile.h"

#include "components/google/core/browser/google_url_tracker_client.h"

#include "components/infobars/core/infobar.h"

#include "components/infobars/core/infobar_delegate.h"

+#include "content/public/browser/notification_observer.h"

+#include "content/public/browser/notification_registrar.h"

#include "content/public/browser/notification_service.h"

-#include "content/public/test/test_browser_thread_bundle.h"

#include "net/url_request/test_url_fetcher_factory.h"

#include "net/url_request/url_fetcher.h"

#include "testing/gtest/include/gtest/gtest.h"

@@ -33,11 +38,13 @@ 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.

+ // parent class, this does not add the infobar to |infobar_service|.

+ // 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.

+ // TODO(blundell): Simplify this test to use InfoBarManager instead of

+ // InfoBarService, see crbug.com/373233.

static infobars::InfoBar* Create(GoogleURLTrackerTest* test_harness,

InfoBarService* infobar_service,

GoogleURLTracker* google_url_tracker,

@@ -214,8 +221,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

-// objects, merely use them for comparisons and lookups, e.g. in |entry_map_|.

+// NavigationController* 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

// numbers. Then we write the necessary stubs/hooks, here and in

@@ -226,7 +233,7 @@ bool TestGoogleURLTrackerNavigationHelper::IsListeningForTabDestruction(

// we use here) is "implementation-defined". Since I've never seen a compiler

// break this, though, and the result would simply be a failing test rather than

// a bug in Chrome, we'll use it anyway.

-class GoogleURLTrackerTest : public testing::Test {

+class GoogleURLTrackerTest : public ChromeRenderViewHostTestHarness {

public:

// Called by TestInfoBarDelegate::Close().

void OnInfoBarClosed(scoped_ptr<infobars::InfoBar> infobar,

@@ -266,26 +273,27 @@ 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

- // object, we don't add the created infobar to it. Instead we will simulate

- // any helper<->infobar interaction necessary. The returned object will be

+ // Infobars are not added to |infobar_service|. Instead we will simulate any

+ // helper<->infobar interaction necessary. The returned object will be

// cleaned up in OnInfoBarClosed().

Peter Kasting 2014/05/28 22:27:19 If you're going to have a real infobar service, wh

droger 2014/06/02 13:29:46 Done.

infobars::InfoBar* CreateTestInfoBar(InfoBarService* infobar_service,

GoogleURLTracker* google_url_tracker,

const GURL& search_url);

- // These are required by the TestURLFetchers GoogleURLTracker will create (see

- // test_url_fetcher_factory.h).

- content::TestBrowserThreadBundle thread_bundle_;

+ // Gets the infobar service indexed by |unique_id|.

+ // Lazily creates WebContents instances, adds them in |web_contents_map_| and

+ // attaches InfoBarServices to them.

+ InfoBarService* GetInfoBarService(intptr_t unique_id);

// Creating this allows us to call

// net::NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests().

scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_;

net::TestURLFetcherFactory fetcher_factory_;

GoogleURLTrackerClient* client_;

GoogleURLTrackerNavigationHelper* nav_helper_;

- TestingProfile profile_;

scoped_ptr<GoogleURLTracker> google_url_tracker_;

TestCallbackListener listener_;

+ base::ScopedPtrHashMap<intptr_t, content::WebContents> web_contents_map_;

// 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_;

@@ -294,29 +302,26 @@ class GoogleURLTrackerTest : public testing::Test {

void GoogleURLTrackerTest::OnInfoBarClosed(

scoped_ptr<infobars::InfoBar> infobar,

InfoBarService* infobar_service) {

- // First, simulate the InfoBarService firing INFOBAR_REMOVED.

- infobars::InfoBar::RemovedDetails removed_details(infobar.get(), false);

+ // First, simulate the InfoBarService firing OnInfoBarRemoved().

GoogleURLTracker::EntryMap::const_iterator i =

google_url_tracker_->entry_map_.find(infobar_service);

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::Details<infobars::InfoBar::RemovedDetails>(&removed_details));

+ map_entry->OnInfoBarRemoved(infobar.get(), false);

// Second, simulate the infobar container closing the infobar in response.

// This happens automatically as |infobar| goes out of scope.

}

-GoogleURLTrackerTest::GoogleURLTrackerTest()

- : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {

- GoogleURLTrackerFactory::GetInstance()->

- RegisterUserPrefsOnBrowserContextForTest(&profile_);

+GoogleURLTrackerTest::GoogleURLTrackerTest() {

+ ChromeRenderViewHostTestHarness::SetUp();

Peter Kasting 2014/05/28 22:27:19 If we're going to have both constructor/destructor

+ GoogleURLTrackerFactory::GetInstance()

+ ->RegisterUserPrefsOnBrowserContextForTest(profile());

Peter Kasting 2014/05/28 22:27:19 Nit: I know not everyone agrees with this, but I t

}

GoogleURLTrackerTest::~GoogleURLTrackerTest() {

+ ChromeRenderViewHostTestHarness::TearDown();

}

void GoogleURLTrackerTest::SetUp() {

@@ -328,7 +333,7 @@ void GoogleURLTrackerTest::SetUp() {

scoped_ptr<GoogleURLTrackerClient> client(client_);

scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper(nav_helper_);

google_url_tracker_.reset(

- new GoogleURLTracker(&profile_,

+ new GoogleURLTracker(profile(),

client.Pass(),

nav_helper.Pass(),

GoogleURLTracker::UNIT_TEST_MODE));

@@ -339,6 +344,7 @@ void GoogleURLTrackerTest::SetUp() {

void GoogleURLTrackerTest::TearDown() {

while (!unique_ids_seen_.empty())

CloseTab(*unique_ids_seen_.begin());

+ web_contents_map_.clear();

Peter Kasting 2014/05/28 22:27:19 Is this necessary?

}

net::TestURLFetcher* GoogleURLTrackerTest::GetFetcher() {

@@ -378,11 +384,11 @@ void GoogleURLTrackerTest::NotifyIPAddressChanged() {

}

void GoogleURLTrackerTest::SetLastPromptedGoogleURL(const GURL& url) {

- profile_.GetPrefs()->SetString(prefs::kLastPromptedGoogleURL, url.spec());

+ profile()->GetPrefs()->SetString(prefs::kLastPromptedGoogleURL, url.spec());

}

GURL GoogleURLTrackerTest::GetLastPromptedGoogleURL() {

- return GURL(profile_.GetPrefs()->GetString(prefs::kLastPromptedGoogleURL));

+ return GURL(profile()->GetPrefs()->GetString(prefs::kLastPromptedGoogleURL));

}

void GoogleURLTrackerTest::SetNavigationPending(intptr_t unique_id,

@@ -396,7 +402,7 @@ void GoogleURLTrackerTest::SetNavigationPending(intptr_t unique_id,

if (client_->IsListeningForNavigationStart()) {

google_url_tracker_->OnNavigationPending(

reinterpret_cast<content::NavigationController*>(unique_id),

- reinterpret_cast<InfoBarService*>(unique_id), unique_id);

+ GetInfoBarService(unique_id), unique_id);

}

@@ -424,8 +430,8 @@ void GoogleURLTrackerTest::CommitSearch(intptr_t unique_id,

DCHECK(search_url.is_valid());

if (nav_helper_->IsListeningForNavigationCommit(

reinterpret_cast<content::NavigationController*>(unique_id))) {

- google_url_tracker_->OnNavigationCommitted(

- reinterpret_cast<InfoBarService*>(unique_id), search_url);

+ google_url_tracker_->OnNavigationCommitted(GetInfoBarService(unique_id),

+ search_url);

}

@@ -446,8 +452,7 @@ void GoogleURLTrackerTest::CloseTab(intptr_t unique_id) {

GoogleURLTrackerMapEntry* GoogleURLTrackerTest::GetMapEntry(

intptr_t unique_id) {

GoogleURLTracker::EntryMap::const_iterator i =

- google_url_tracker_->entry_map_.find(

- reinterpret_cast<InfoBarService*>(unique_id));

+ google_url_tracker_->entry_map_.find(GetInfoBarService(unique_id));

return (i == google_url_tracker_->entry_map_.end()) ? NULL : i->second;

}

@@ -481,6 +486,21 @@ infobars::InfoBar* GoogleURLTrackerTest::CreateTestInfoBar(

search_url);

}

+InfoBarService* GoogleURLTrackerTest::GetInfoBarService(intptr_t unique_id) {

+ base::ScopedPtrHashMap<intptr_t, content::WebContents>::const_iterator it =

+ web_contents_map_.find(unique_id);

+ if (it == web_contents_map_.end()) {

+ // Lazily create an InfoBarService for |unique_id|.

+ scoped_ptr<content::WebContents> web_contents(CreateTestWebContents());

+ InfoBarService::CreateForWebContents(web_contents.get());

+ it = web_contents_map_.set(unique_id, web_contents.Pass());

+ }

+ InfoBarService* service = InfoBarService::FromWebContents(it->second);

+ DCHECK(service);

+ return service;

// TestInfoBarDelegate --------------------------------------------------------

« chrome/browser/google/google_url_tracker_map_entry.cc ('K') | « chrome/browser/google/google_url_tracker_map_entry.cc ('k') | no next file » | no next file with comments »