Make TabContents own its infobar delegates....

chrome/browser/geolocation/geolocation_permission_context_unittest.cc

Index: chrome/browser/geolocation/geolocation_permission_context_unittest.cc
===================================================================
--- chrome/browser/geolocation/geolocation_permission_context_unittest.cc	(revision 65711)
+++ chrome/browser/geolocation/geolocation_permission_context_unittest.cc	(working copy)
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/geolocation/geolocation_permission_context.h"
 
+#include <set>
+
 #include "base/scoped_vector.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/geolocation/geolocation_content_settings_map.h"
@@ -15,49 +17,50 @@
 #include "chrome/browser/renderer_host/test/test_render_view_host.h"
 #include "chrome/browser/tab_contents/infobar_delegate.h"
 #include "chrome/browser/tab_contents/test_tab_contents.h"
-#include "chrome/common/notification_details.h"
-#include "chrome/common/notification_type.h"
+#include "chrome/common/notification_registrar.h"
+#include "chrome/common/notification_service.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/test/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-// TestTabContents short-circuits TAB_CONTENTS_INFOBAR_REMOVED to call
-// InfoBarClosed() directly. We need to observe it and call InfoBarClosed()
-// later.
-class TestTabContentsWithPendingInfoBar : public TestTabContents {
+// We need to track which infobars were closed.
+class ClosedDelegateTracker : public NotificationObserver {
  public:
-   TestTabContentsWithPendingInfoBar(Profile* profile, SiteInstance* instance)
-    : TestTabContents(profile, instance),
-      removed_infobar_delegate_(NULL) {
+  ClosedDelegateTracker() {
+    registrar_.Add(this, NotificationType::TAB_CONTENTS_INFOBAR_REMOVED,
+                   NotificationService::AllSources());
   }
 
-  void Observe(NotificationType type,
+  virtual void Observe(NotificationType type,
                const NotificationSource& source,
                const NotificationDetails& details) {
-    // We need to special case TAB_CONTENTS_INFOBAR_REMOVED to track which
-    // delegate was removed and avoid calling InfoBarClosed() supplied in the
-    // base class. All other notification types are delegated to the base class.
-    switch (type.value) {
-      case NotificationType::TAB_CONTENTS_INFOBAR_REMOVED:
-        removed_infobar_delegate_ = Details<InfoBarDelegate>(details).ptr();
-        break;
-      default:
-        TestTabContents::Observe(type, source, details);
-        break;
-    }
+    DCHECK(type.value == NotificationType::TAB_CONTENTS_INFOBAR_REMOVED);
+    removed_infobar_delegates_.insert(Details<InfoBarDelegate>(details).ptr());
   }
 
-  InfoBarDelegate* removed_infobar_delegate_;
+  bool Contains(InfoBarDelegate* delegate) const {
+    return removed_infobar_delegates_.count(delegate) != 0;
+  }
+
+  size_t size() const {
+    return removed_infobar_delegates_.size();
+  }
+
+  void Clear() {
+    removed_infobar_delegates_.clear();
+  }
+
+ private:
+  NotificationRegistrar registrar_;
+  std::set<InfoBarDelegate*> removed_infobar_delegates_;
 };
 
-// This class sets up GeolocationArbitrator and injects
-// TestTabContentsWithPendingInfoBar.
+// This class sets up GeolocationArbitrator.
 class GeolocationPermissionContextTests : public RenderViewHostTestHarness {
  public:
   GeolocationPermissionContextTests()
     : RenderViewHostTestHarness(),
-      ui_thread_(BrowserThread::UI, MessageLoop::current()),
-      tab_contents_with_pending_infobar_(NULL) {
+      ui_thread_(BrowserThread::UI, MessageLoop::current()) {
   }
 
   virtual ~GeolocationPermissionContextTests() {
@@ -67,17 +70,15 @@
     RenderViewHostTestHarness::SetUp();
     GeolocationArbitrator::SetProviderFactoryForTest(
         &NewAutoSuccessMockNetworkLocationProvider);
-    SiteInstance* site_instance = contents_->GetSiteInstance();
-    tab_contents_with_pending_infobar_ =
-        new TestTabContentsWithPendingInfoBar(profile_.get(), site_instance);
-    contents_.reset(tab_contents_with_pending_infobar_);
     geolocation_permission_context_ =
         new GeolocationPermissionContext(profile());
+    closed_delegate_tracker_.reset(new ClosedDelegateTracker());
   }
 
   virtual void TearDown() {
-    RenderViewHostTestHarness::TearDown();
+    closed_delegate_tracker_.reset();
     GeolocationArbitrator::SetProviderFactoryForTest(NULL);
+    RenderViewHostTestHarness::TearDown();
   }
 
   int process_id() {
@@ -126,8 +127,8 @@
   }
 
   void AddNewTab(const GURL& url) {
-    TestTabContentsWithPendingInfoBar* new_tab =
-        new TestTabContentsWithPendingInfoBar(profile(), NULL);
+    TabContents* new_tab =
+        new TabContents(profile(), NULL, MSG_ROUTING_NONE, NULL, NULL);
     new_tab->controller().LoadURL(url, GURL(), PageTransition::TYPED);
     static_cast<TestRenderViewHost*>(new_tab->render_manager()->
        current_host())->SendNavigate(extra_tabs_.size() + 1, url);
@@ -151,9 +152,9 @@
 
  protected:
   BrowserThread ui_thread_;
-  TestTabContentsWithPendingInfoBar* tab_contents_with_pending_infobar_;
   scoped_refptr<GeolocationPermissionContext> geolocation_permission_context_;
-  ScopedVector<TestTabContentsWithPendingInfoBar> extra_tabs_;
+  scoped_ptr<ClosedDelegateTracker> closed_delegate_tracker_;
+  ScopedVector<TabContents> extra_tabs_;
 };
 
 TEST_F(GeolocationPermissionContextTests, SinglePermission) {
@@ -197,9 +198,9 @@
   CheckPermissionMessageSent(bridge_id(), true);
 
   contents()->RemoveInfoBar(infobar_0);
-  EXPECT_EQ(infobar_0,
-            tab_contents_with_pending_infobar_->removed_infobar_delegate_);
-  infobar_0->InfoBarClosed();
+  EXPECT_EQ(1U, closed_delegate_tracker_->size());
+  EXPECT_TRUE(closed_delegate_tracker_->Contains(infobar_0));
+  closed_delegate_tracker_->Clear();
   // Now we should have a new infobar for the second frame.
   EXPECT_EQ(1, contents()->infobar_delegate_count());
 
@@ -214,9 +215,8 @@
   CheckTabContentsState(requesting_frame_1, CONTENT_SETTING_BLOCK);
   CheckPermissionMessageSent(bridge_id() + 1, false);
   contents()->RemoveInfoBar(infobar_1);
-  EXPECT_EQ(infobar_1,
-            tab_contents_with_pending_infobar_->removed_infobar_delegate_);
-  infobar_1->InfoBarClosed();
+  EXPECT_EQ(1U, closed_delegate_tracker_->size());
+  EXPECT_TRUE(closed_delegate_tracker_->Contains(infobar_1));
   EXPECT_EQ(0, contents()->infobar_delegate_count());
   // Ensure the persisted permissions are ok.
   EXPECT_EQ(
@@ -259,9 +259,9 @@
   // is removed and the next pending infobar is created.
   geolocation_permission_context_->CancelGeolocationPermissionRequest(
       process_id(), render_id(), bridge_id(), requesting_frame_0);
-  EXPECT_EQ(infobar_0,
-            tab_contents_with_pending_infobar_->removed_infobar_delegate_);
-  infobar_0->InfoBarClosed();
+  EXPECT_EQ(1U, closed_delegate_tracker_->size());
+  EXPECT_TRUE(closed_delegate_tracker_->Contains(infobar_0));
+  closed_delegate_tracker_->Clear();
   EXPECT_EQ(1, contents()->infobar_delegate_count());
 
   ConfirmInfoBarDelegate* infobar_1 =
@@ -275,9 +275,8 @@
   CheckTabContentsState(requesting_frame_1, CONTENT_SETTING_ALLOW);
   CheckPermissionMessageSent(bridge_id() + 1, true);
   contents()->RemoveInfoBar(infobar_1);
-  EXPECT_EQ(infobar_1,
-            tab_contents_with_pending_infobar_->removed_infobar_delegate_);
-  infobar_1->InfoBarClosed();
+  EXPECT_EQ(1U, closed_delegate_tracker_->size());
+  EXPECT_TRUE(closed_delegate_tracker_->Contains(infobar_1));
   EXPECT_EQ(0, contents()->infobar_delegate_count());
   // Ensure the persisted permissions are ok.
   EXPECT_EQ(
@@ -303,14 +302,12 @@
 
   geolocation_permission_context_->StopUpdatingRequested(
       process_id(), render_id(), bridge_id());
-  EXPECT_EQ(infobar_0,
-            tab_contents_with_pending_infobar_->removed_infobar_delegate_);
-  infobar_0->InfoBarClosed();
+  EXPECT_EQ(1U, closed_delegate_tracker_->size());
+  EXPECT_TRUE(closed_delegate_tracker_->Contains(infobar_0));
   EXPECT_EQ(0, contents()->infobar_delegate_count());
-  EXPECT_EQ(
-      CONTENT_SETTING_ASK,
-      profile()->GetGeolocationContentSettingsMap()->GetContentSetting(
-          requesting_frame, requesting_frame));
+  EXPECT_EQ(CONTENT_SETTING_ASK,
+            profile()->GetGeolocationContentSettingsMap()->GetContentSetting(
+                requesting_frame, requesting_frame));
 }
 
 TEST_F(GeolocationPermissionContextTests, InvalidURL) {
@@ -354,14 +351,12 @@
   infobar_0->Accept();
   CheckPermissionMessageSent(bridge_id(), true);
   contents()->RemoveInfoBar(infobar_0);
-  EXPECT_EQ(infobar_0,
-            tab_contents_with_pending_infobar_->removed_infobar_delegate_);
-  infobar_0->InfoBarClosed();
+  EXPECT_EQ(2U, closed_delegate_tracker_->size());
+  EXPECT_TRUE(closed_delegate_tracker_->Contains(infobar_0));
   // Now the infobar for the tab with the same origin should have gone.
   EXPECT_EQ(0, extra_tabs_[1]->infobar_delegate_count());
   CheckPermissionMessageSentForTab(1, bridge_id(), true);
-  // Destroy the infobar that has just been removed.
-  removed_infobar->InfoBarClosed();
+  EXPECT_TRUE(closed_delegate_tracker_->Contains(removed_infobar));
 
   // But the other tab should still have the info bar...
   EXPECT_EQ(1, extra_tabs_[0]->infobar_delegate_count());
@@ -397,14 +392,13 @@
   infobar_0->Accept();
   CheckPermissionMessageSentForTab(0, bridge_id(), true);
   extra_tabs_[0]->RemoveInfoBar(infobar_0);
-  EXPECT_EQ(infobar_0,
-            extra_tabs_[0]->removed_infobar_delegate_);
-  infobar_0->InfoBarClosed();
+  EXPECT_EQ(2U, closed_delegate_tracker_->size());
+  EXPECT_TRUE(closed_delegate_tracker_->Contains(infobar_0));
   // Now the infobar for the tab with the same origin should have gone.
   EXPECT_EQ(0, contents()->infobar_delegate_count());
   CheckPermissionMessageSent(bridge_id(), true);
-  // Destroy the infobar that has just been removed.
-  removed_infobar->InfoBarClosed();
+  EXPECT_TRUE(closed_delegate_tracker_->Contains(removed_infobar));
+  closed_delegate_tracker_->Clear();
 
   // And we should have the queued infobar displayed now.
   EXPECT_EQ(1, extra_tabs_[0]->infobar_delegate_count());
@@ -416,9 +410,8 @@
   infobar_1->Accept();
   CheckPermissionMessageSentForTab(0, bridge_id() + 1, true);
   extra_tabs_[0]->RemoveInfoBar(infobar_1);
-  EXPECT_EQ(infobar_1,
-            extra_tabs_[0]->removed_infobar_delegate_);
-  infobar_1->InfoBarClosed();
+  EXPECT_EQ(1U, closed_delegate_tracker_->size());
+  EXPECT_TRUE(closed_delegate_tracker_->Contains(infobar_1));
 
   extra_tabs_.reset();
 }
Property changes on: chrome/browser/geolocation/geolocation_permission_context_unittest.cc
___________________________________________________________________
Added: svn:eol-style
   + LF