Remove Infobars notifications from GoogleURLTrackerMapEntry

chrome/browser/google/google_url_tracker_unittest.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/google/google_url_tracker.h"
 
 #include <set>
 #include <string>
 
 #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/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"
 
 class GoogleURLTrackerTest;
 
 namespace {
 
 // TestInfoBarDelegate --------------------------------------------------------
 
 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.
   static infobars::InfoBar* Create(GoogleURLTrackerTest* test_harness,
                                    InfoBarService* infobar_service,
                                    GoogleURLTracker* google_url_tracker,
                                    const GURL& search_url);
 
  private:
   TestInfoBarDelegate(GoogleURLTrackerTest* test_harness,
                       InfoBarService* infobar_service,
                       GoogleURLTracker* google_url_tracker,
                       const GURL& search_url);
@@ skipping 127 matching lines @@
 }
 
 }  // namespace
 
 
 // GoogleURLTrackerTest -------------------------------------------------------
 
 // 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
 // TestInfoBarDelegate above, to make everything continue to work.
 //
 // Technically, the C++98 spec defines the result of casting
 // T* -> intptr_t -> T* to be an identity, but intptr_t -> T* -> intptr_t (what
 // 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,
                        InfoBarService* infobar_service);
 
  protected:
   GoogleURLTrackerTest();
   virtual ~GoogleURLTrackerTest();
 
   // testing::Test
@@ skipping 19 matching lines @@
   void CommitSearch(intptr_t unique_id, const GURL& search_url);
   void CloseTab(intptr_t unique_id);
   GoogleURLTrackerMapEntry* GetMapEntry(intptr_t unique_id);
   GoogleURLTrackerInfoBarDelegate* GetInfoBarDelegate(intptr_t unique_id);
   void ExpectDefaultURLs() const;
   void ExpectListeningForCommit(intptr_t unique_id, bool listening);
   bool observer_notified() const { return observer_.notified(); }
   void clear_observer_notified() { observer_.clear_notified(); }
 
  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().
   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_| 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_;
   content::NotificationRegistrar registrar_;
   TestNotificationObserver observer_;
   GoogleURLTrackerNavigationHelper* nav_helper_;
-  TestingProfile profile_;
   scoped_ptr<GoogleURLTracker> google_url_tracker_;
+  ScopedVector<content::WebContents> web_contents_;
   // 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_;
 };
 
 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();
+  GoogleURLTrackerFactory::GetInstance()
+      ->RegisterUserPrefsOnBrowserContextForTest(profile());
 }
 
 GoogleURLTrackerTest::~GoogleURLTrackerTest() {
+  ChromeRenderViewHostTestHarness::TearDown();
 }
 
 void GoogleURLTrackerTest::SetUp() {
   network_change_notifier_.reset(net::NetworkChangeNotifier::CreateMock());
   // Ownership is passed to google_url_tracker_, but a weak pointer is kept;
   // this is safe since GoogleURLTracker keeps the observer for its lifetime.
   nav_helper_ = new TestGoogleURLTrackerNavigationHelper();
   scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper(nav_helper_);
-  google_url_tracker_.reset(
-      new GoogleURLTracker(&profile_, nav_helper.Pass(),
-                           GoogleURLTracker::UNIT_TEST_MODE));
+  google_url_tracker_.reset(new GoogleURLTracker(
+      profile(), nav_helper.Pass(), GoogleURLTracker::UNIT_TEST_MODE));
   google_url_tracker_->infobar_creator_ = base::Bind(
       &GoogleURLTrackerTest::CreateTestInfoBar, base::Unretained(this));
 }
 
 void GoogleURLTrackerTest::TearDown() {
   while (!unique_ids_seen_.empty())
     CloseTab(*unique_ids_seen_.begin());
 
   nav_helper_ = NULL;
   google_url_tracker_.reset();
   network_change_notifier_.reset();
+  web_contents_.clear();
 }
 
 net::TestURLFetcher* GoogleURLTrackerTest::GetFetcher() {
   // This will return the last fetcher created.  If no fetchers have been
   // created, we'll pass GetFetcherByID() "-1", and it will return NULL.
   return fetcher_factory_.GetFetcherByID(google_url_tracker_->fetcher_id_ - 1);
 }
 
 void GoogleURLTrackerTest::MockSearchDomainCheckResponse(
     const std::string& domain) {
   net::TestURLFetcher* fetcher = GetFetcher();
   if (!fetcher)
     return;
   fetcher_factory_.RemoveFetcherFromMap(fetcher->id());
   fetcher->set_url(GURL(GoogleURLTracker::kSearchDomainCheckURL));
   fetcher->set_response_code(200);
   fetcher->SetResponseString(domain);
   fetcher->delegate()->OnURLFetchComplete(fetcher);
   // At this point, |fetcher| is deleted.
 }
 
 void GoogleURLTrackerTest::RequestServerCheck() {
   if (!registrar_.IsRegistered(&observer_,
                                chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
-                               content::Source<Profile>(&profile_))) {
+                               content::Source<Profile>(profile()))) {
     registrar_.Add(&observer_, chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
-                   content::Source<Profile>(&profile_));
+                   content::Source<Profile>(profile()));
   }
   google_url_tracker_->SetNeedToFetch();
 }
 
 void GoogleURLTrackerTest::FinishSleep() {
   google_url_tracker_->FinishSleep();
 }
 
 void GoogleURLTrackerTest::NotifyIPAddressChanged() {
   net::NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests();
   // For thread safety, the NCN queues tasks to do the actual notifications, so
   // we need to spin the message loop so the tracker will actually be notified.
   base::MessageLoop::current()->RunUntilIdle();
 }
 
 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,
                                                 bool is_search) {
   if (is_search) {
     google_url_tracker_->SearchCommitted();
     // Note that the call above might not have actually registered a listener
     // for navigation starts if the searchdomaincheck response was bogus.
   }
   unique_ids_seen_.insert(unique_id);
   if (nav_helper_->IsListeningForNavigationStart()) {
     google_url_tracker_->OnNavigationPending(
         reinterpret_cast<content::NavigationController*>(unique_id),
-        reinterpret_cast<InfoBarService*>(unique_id), unique_id);
+        GetInfoBarService(unique_id), unique_id);
   }
 }
 
 void GoogleURLTrackerTest::CommitNonSearch(intptr_t unique_id) {
   GoogleURLTrackerMapEntry* map_entry = GetMapEntry(unique_id);
   if (!map_entry)
     return;
 
   ExpectListeningForCommit(unique_id, false);
 
   // The infobar should be showing; otherwise the pending non-search should
   // have closed it.
   ASSERT_TRUE(map_entry->has_infobar_delegate());
 
   // The pending_id should have been reset to 0 when the non-search became
   // pending.
   EXPECT_EQ(0, map_entry->infobar_delegate()->pending_id());
 
   // Committing the navigation would close the infobar.
   map_entry->infobar_delegate()->Close(false);
 }
 
 void GoogleURLTrackerTest::CommitSearch(intptr_t unique_id,
                                         const GURL& search_url) {
   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);
   }
 }
 
 void GoogleURLTrackerTest::CloseTab(intptr_t unique_id) {
   unique_ids_seen_.erase(unique_id);
   content::NavigationController* nav_controller =
       reinterpret_cast<content::NavigationController*>(unique_id);
   if (nav_helper_->IsListeningForTabDestruction(nav_controller)) {
     google_url_tracker_->OnTabClosed(nav_controller);
   } else {
     // Closing a tab with an infobar showing would close the infobar.
     GoogleURLTrackerInfoBarDelegate* delegate = GetInfoBarDelegate(unique_id);
     if (delegate)
       delegate->Close(false);
   }
 }
 
 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;
 }
 
 GoogleURLTrackerInfoBarDelegate* GoogleURLTrackerTest::GetInfoBarDelegate(
     intptr_t unique_id) {
   GoogleURLTrackerMapEntry* map_entry = GetMapEntry(unique_id);
   return map_entry ? map_entry->infobar_delegate() : NULL;
 }
 
 void GoogleURLTrackerTest::ExpectDefaultURLs() const {
@@ skipping 13 matching lines @@
 }
 
 infobars::InfoBar* GoogleURLTrackerTest::CreateTestInfoBar(
     InfoBarService* infobar_service,
     GoogleURLTracker* google_url_tracker,
     const GURL& search_url) {
   return TestInfoBarDelegate::Create(this, infobar_service, google_url_tracker,
                                      search_url);
 }
 
+InfoBarService* GoogleURLTrackerTest::GetInfoBarService(intptr_t unique_id) {
+  DCHECK(unique_id >= 0);
+
+  if ((size_t)unique_id >= web_contents_.size())

[blundell, 2014/05/15 15:52:47]

oof. maybe a map instead of a vector?

[droger, 2014/05/15 15:57:40]

That was my first idea too, but:
Unfortunately there is no ScopedMap, and this is far from trivial to write :(
A simpler alternative is to use a map of linked_ptr, but it seems that it is
discouraged.

(see
https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/6i8vKfmHnJQ...)

[droger, 2014/05/15 15:59:38]

As the test only actually needs 4 instances, maybe I can get rid of the data
structure entirely and always instantiate 4 instances.

[droger, 2014/05/26 15:22:48]

I found a base::ScopedPtrHashMap that I didn't know about.
I updated the CL to use this, which is much cleaner.

+    web_contents_.resize(unique_id + 1);
+
+  if (!web_contents_[unique_id]) {
+    content::WebContents* web_contents = CreateTestWebContents();
+    InfoBarService::CreateForWebContents(web_contents);
+    web_contents_[unique_id] = web_contents;
+  }
+
+  InfoBarService* service =
+      InfoBarService::FromWebContents(web_contents_[unique_id]);
+  DCHECK(service);
+  return service;
+}
 
 // TestInfoBarDelegate --------------------------------------------------------
 
 namespace {
 
 // static
 infobars::InfoBar* TestInfoBarDelegate::Create(
     GoogleURLTrackerTest* test_harness,
     InfoBarService* infobar_service,
     GoogleURLTracker* google_url_tracker,
@@ skipping 609 matching lines @@
   CommitSearch(2, GURL("http://www.google.co.uk/search?q=test2"));
   EXPECT_FALSE(GetInfoBarDelegate(1) == NULL);
   GoogleURLTrackerInfoBarDelegate* delegate2 = GetInfoBarDelegate(2);
   ASSERT_FALSE(delegate2 == NULL);
   SetNavigationPending(1, true);
   ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, true));
   delegate2->Close(false);
   SetNavigationPending(1, false);
   ASSERT_NO_FATAL_FAILURE(ExpectListeningForCommit(1, false));
 }