Add basic coalescing behavior to the permission bubble manager.

chrome/browser/ui/website_settings/permission_bubble_manager_unittest.cc

Index: chrome/browser/ui/website_settings/permission_bubble_manager_unittest.cc
diff --git a/chrome/browser/ui/website_settings/permission_bubble_manager_unittest.cc b/chrome/browser/ui/website_settings/permission_bubble_manager_unittest.cc
index 0e12b56b83dfc05d8ed4fdc20534c2c19e4f9934..76cc721dc0b073aad9c11a5f221c908d74beca2f 100644
--- a/chrome/browser/ui/website_settings/permission_bubble_manager_unittest.cc
+++ b/chrome/browser/ui/website_settings/permission_bubble_manager_unittest.cc
@@ -3,15 +3,20 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/website_settings/permission_bubble_manager.h"
 #include "chrome/browser/ui/website_settings/permission_bubble_request.h"
 #include "chrome/browser/ui/website_settings/permission_bubble_view.h"
 #include "chrome/common/chrome_switches.h"
+#include "content/public/test/test_browser_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
+const int kCoalesceTestingTimeMs = 25;

[Greg Billock, 2014/02/07 20:14:24]

I think typically we'd want this to be 0 for the existing tests, and just do a
RunLoop RunUntilIdle in WaitForCoalescing.

[leng, 2014/02/08 00:11:18]

Good idea.  Done.

+const int kPaddedCoalesceTimeMs = 50;
+
 class MockRequest : public PermissionBubbleRequest {
  public:
   MockRequest() : granted_(false), cancelled_(false), finished_(false) {}
@@ -106,15 +111,31 @@ class PermissionBubbleManagerTest : public testing::Test {
     manager_->Accept();
   }
 
+  void WaitForCoalescing() {

[Greg Billock, 2014/02/07 20:14:24]

I think if we can touch the WebcontentsObserver method we can get rid of this.

[leng, 2014/02/08 00:11:18]

I've set up DidFinishLoad to only extend the timer, as I found the requests
being added after everything else had completed.  So I'll leave the timer
mechanism in place, rather than calling a WebContentsObserver method.

+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE, base::Bind(&DoneWaiting),
+        base::TimeDelta::FromMilliseconds(kPaddedCoalesceTimeMs));
+    base::MessageLoop::current()->Run();
+  }
+
+  static void DoneWaiting() {
+    base::MessageLoop::current()->QuitNow();
+  }
+
  protected:
   MockRequest request1_;
   MockRequest request2_;
   MockView view_;
   scoped_ptr<PermissionBubbleManager> manager_;
+
+  base::MessageLoop message_loop_;
+  content::TestBrowserThread ui_thread_;
 };
 
 PermissionBubbleManagerTest::PermissionBubbleManagerTest()
-    : manager_(new PermissionBubbleManager(NULL)) {
+    : manager_(new PermissionBubbleManager(NULL)),
+      ui_thread_(content::BrowserThread::UI, &message_loop_) {
+  manager_->SetCoalesceIntervalForTesting(kCoalesceTestingTimeMs);
 }
 
 TEST_F(PermissionBubbleManagerTest, TestFlag) {
@@ -127,6 +148,22 @@ TEST_F(PermissionBubbleManagerTest, TestFlag) {
 TEST_F(PermissionBubbleManagerTest, SingleRequest) {
   manager_->AddRequest(&request1_);
   manager_->SetView(&view_);
+  WaitForCoalescing();
+
+  EXPECT_TRUE(view_.delegate_ == manager_.get());
+  EXPECT_TRUE(view_.shown_);
+  ASSERT_EQ(static_cast<size_t>(1), view_.permission_requests_.size());
+  EXPECT_EQ(&request1_, view_.permission_requests_[0]);
+
+  ToggleAccept(0, true);
+  Accept();
+  EXPECT_TRUE(request1_.granted_);
+}
+
+TEST_F(PermissionBubbleManagerTest, SingleRequestViewFirst) {
+  manager_->SetView(&view_);
+  manager_->AddRequest(&request1_);
+  WaitForCoalescing();
 
   EXPECT_TRUE(view_.delegate_ == manager_.get());
   EXPECT_TRUE(view_.shown_);
@@ -142,6 +179,7 @@ TEST_F(PermissionBubbleManagerTest, TwoRequests) {
   manager_->AddRequest(&request1_);
   manager_->AddRequest(&request2_);
   manager_->SetView(&view_);
+  WaitForCoalescing();
 
   EXPECT_TRUE(view_.delegate_ == manager_.get());
   EXPECT_TRUE(view_.shown_);
@@ -160,6 +198,7 @@ TEST_F(PermissionBubbleManagerTest, TwoRequestsTabSwitch) {
   manager_->AddRequest(&request1_);
   manager_->AddRequest(&request2_);
   manager_->SetView(&view_);
+  WaitForCoalescing();
 
   EXPECT_TRUE(view_.delegate_ == manager_.get());
   EXPECT_TRUE(view_.shown_);
@@ -188,3 +227,68 @@ TEST_F(PermissionBubbleManagerTest, TwoRequestsTabSwitch) {
   EXPECT_FALSE(request2_.granted_);
 }
 
+TEST_F(PermissionBubbleManagerTest, NoRequests) {
+  manager_->SetView(&view_);
+  WaitForCoalescing();
+  EXPECT_FALSE(view_.shown_);
+}
+
+TEST_F(PermissionBubbleManagerTest, NoView) {
+  manager_->AddRequest(&request1_);
+  WaitForCoalescing();
+  EXPECT_FALSE(view_.shown_);
+}
+
+TEST_F(PermissionBubbleManagerTest, TwoRequestsCoalesce) {
+  manager_->SetView(&view_);
+  manager_->AddRequest(&request1_);
+  manager_->AddRequest(&request2_);
+  EXPECT_FALSE(view_.shown_);
+  WaitForCoalescing();
+
+  EXPECT_TRUE(view_.shown_);
+  EXPECT_EQ(2u, view_.permission_requests_.size());
+}
+
+TEST_F(PermissionBubbleManagerTest, TwoRequestsDoNotCoalesce) {
+  manager_->SetView(&view_);
+  manager_->AddRequest(&request1_);
+  WaitForCoalescing();
+  manager_->AddRequest(&request2_);
+
+  EXPECT_TRUE(view_.shown_);
+  EXPECT_EQ(1u, view_.permission_requests_.size());
+}
+
+TEST_F(PermissionBubbleManagerTest, TwoRequestsShownInTwoBubbles) {
+  manager_->SetView(&view_);
+  manager_->AddRequest(&request1_);
+  WaitForCoalescing();
+  manager_->AddRequest(&request2_);
+
+  EXPECT_TRUE(view_.shown_);
+  ASSERT_EQ(1u, view_.permission_requests_.size());
+  EXPECT_EQ(&request1_, view_.permission_requests_[0]);
+
+  view_.Hide();
+  Accept();
+  EXPECT_FALSE(view_.shown_);
+
+  WaitForCoalescing();
+  EXPECT_TRUE(view_.shown_);
+  ASSERT_EQ(1u, view_.permission_requests_.size());
+  EXPECT_EQ(&request2_, view_.permission_requests_[0]);
+}
+
+TEST_F(PermissionBubbleManagerTest, TestAddDuplicateRequest) {
+  manager_->SetView(&view_);
+  manager_->AddRequest(&request1_);
+  manager_->AddRequest(&request2_);
+  manager_->AddRequest(&request1_);
+
+  WaitForCoalescing();
+  EXPECT_TRUE(view_.shown_);
+  ASSERT_EQ(2u, view_.permission_requests_.size());
+  EXPECT_EQ(&request1_, view_.permission_requests_[0]);
+  EXPECT_EQ(&request2_, view_.permission_requests_[1]);
+}