Show queued geolocation InfoBars when InfoBar is hidden.

chrome/browser/geolocation/chrome_geolocation_permission_context_unittest.cc

Index: chrome/browser/geolocation/chrome_geolocation_permission_context_unittest.cc
diff --git a/chrome/browser/geolocation/chrome_geolocation_permission_context_unittest.cc b/chrome/browser/geolocation/chrome_geolocation_permission_context_unittest.cc
index cd583458772eafeae3d2cf0a03beb2ac66ab137a..f542e9b0f3fdc395c2f52d1b0756b843cff8c89d 100644
--- a/chrome/browser/geolocation/chrome_geolocation_permission_context_unittest.cc
+++ b/chrome/browser/geolocation/chrome_geolocation_permission_context_unittest.cc
@@ -40,6 +40,7 @@ namespace {
 
 // We need to track which infobars were closed.
 class ClosedDelegateTracker : public content::NotificationObserver {
+  FRIEND_TEST_ALL_PREFIXES(GeolocationPermissionContextTests, TabDestroyed);

[Peter Kasting, 2012/03/26 19:12:22]

This doesn't go above the public section, it goes in the private section.

[John Knottenbelt, 2012/03/27 10:21:29]

Done.

  public:
   ClosedDelegateTracker();
   virtual ~ClosedDelegateTracker();
@@ -86,9 +87,6 @@ void ClosedDelegateTracker::Clear() {
   removed_infobar_delegates_.clear();
 }
 
-}  // namespace
-
-
 // GeolocationPermissionContextTests ------------------------------------------
 
 // This class sets up GeolocationArbitrator.
@@ -139,8 +137,8 @@ class GeolocationPermissionContextTests : public TabContentsWrapperTestHarness {
 
  private:
   // TabContentsWrapperTestHarness:
-  virtual void SetUp();
-  virtual void TearDown();
+  virtual void SetUp() OVERRIDE;
+  virtual void TearDown() OVERRIDE;
 
   content::TestBrowserThread ui_thread_;
   content::MockGeolocation mock_geolocation_;
@@ -239,6 +237,7 @@ void GeolocationPermissionContextTests::SetUp() {
 }
 
 void GeolocationPermissionContextTests::TearDown() {
+  extra_tabs_.reset();
   mock_geolocation_.TearDown();
   TabContentsWrapperTestHarness::TearDown();
 }
@@ -471,8 +470,6 @@ TEST_F(GeolocationPermissionContextTests, SameOriginMultipleTabs) {
   EXPECT_EQ(1U, closed_delegate_tracker_.size());
   EXPECT_TRUE(closed_delegate_tracker_.Contains(infobar_1));
   infobar_1->InfoBarClosed();
-
-  extra_tabs_.reset();
 }
 
 TEST_F(GeolocationPermissionContextTests, QueuedOriginMultipleTabs) {
@@ -528,8 +525,6 @@ TEST_F(GeolocationPermissionContextTests, QueuedOriginMultipleTabs) {
   EXPECT_EQ(1U, closed_delegate_tracker_.size());
   EXPECT_TRUE(closed_delegate_tracker_.Contains(infobar_1));
   infobar_1->InfoBarClosed();
-
-  extra_tabs_.reset();
 }
 
 TEST_F(GeolocationPermissionContextTests, TabDestroyed) {
@@ -567,6 +562,16 @@ TEST_F(GeolocationPermissionContextTests, TabDestroyed) {
   // Delete the tab contents.
   DeleteContents();
   infobar_0->InfoBarClosed();
+
+  // During contents destruction, the infobar will have been closed, and a
+  // second (with it's own new delegate) will have been created. In Chromium,
+  // this would be properly deleted by the InfoBarContainer, but in this unit
+  // test, the closest thing we have to that is the ClosedDelegateTracker.
+  ASSERT_EQ(2U, closed_delegate_tracker_.size());
+  ASSERT_TRUE(closed_delegate_tracker_.Contains(infobar_0));
+  closed_delegate_tracker_.removed_infobar_delegates_.erase(infobar_0);
+  (*closed_delegate_tracker_.removed_infobar_delegates_.begin())->
+      InfoBarClosed();
 }
 
 TEST_F(GeolocationPermissionContextTests, InfoBarUsesCommittedEntry) {
@@ -598,3 +603,5 @@ TEST_F(GeolocationPermissionContextTests, InfoBarUsesCommittedEntry) {
   DeleteContents();
   infobar_0->InfoBarClosed();
 }
+
+}  // namespace

[Peter Kasting, 2012/03/26 19:12:22]

Don't extend the namespace to cover the unit tests, or we can't friend them from
any other source files.  If you need the helper class in the same namespace then
remove both from the namespace.

[John Knottenbelt, 2012/03/27 10:21:29]

I removed the namespace. The issue is that the FRIEND_TEST macro looks up the
class in the anonymous namespace and there is no documented way to forward
declare the test class outside of the namespace. 

I could do:

class GeolocationPermissionContextTests_TabDestroyed_Test;

or

class GTEST_TEST_CLASS_NAME_(GeolocationPermissionContextTests, TabDestroyed);

but I think this is relies too much on the internal implementation of gtest.

On 2012/03/26 19:12:22, Peter Kasting wrote: