[Presentation API] Handle multiple Presentation URLs in PresentationRequest::getAvailability()

content/renderer/presentation/presentation_dispatcher_unittest.cc

Index: content/renderer/presentation/presentation_dispatcher_unittest.cc
diff --git a/content/renderer/presentation/presentation_dispatcher_unittest.cc b/content/renderer/presentation/presentation_dispatcher_unittest.cc
index 65b3a8979899d814a372736c8bac20a4e1e90911..d8fe3728a155d26590214d2fbd1a9039817dbcf5 100644
--- a/content/renderer/presentation/presentation_dispatcher_unittest.cc
+++ b/content/renderer/presentation/presentation_dispatcher_unittest.cc
@@ -49,6 +49,8 @@ class MockPresentationAvailabilityObserver
  public:
   explicit MockPresentationAvailabilityObserver(const WebVector<WebURL>& urls)
       : urls_(urls) {}
+  explicit MockPresentationAvailabilityObserver(const std::vector<GURL>& gurls)

[mark a. foltz, 2017/01/13 19:01:16]

Do we need both mock constructors?

[zhaobin, 2017/01/14 01:45:33]

Done.

+      : urls_(gurls) {}
   ~MockPresentationAvailabilityObserver() override {}
 
   MOCK_METHOD1(availabilityChanged, void(bool is_available));
@@ -105,6 +107,13 @@ class MockPresentationService : public PresentationService {
                void(PresentationSessionInfo* session_info));
 };
 
+class MockPresentationAvailabilityCallbacks final
+    : public blink::WebCallbacks<bool, const blink::WebPresentationError&> {
+ public:
+  MOCK_METHOD1(onSuccess, void(bool value));
+  MOCK_METHOD1(onError, void(const blink::WebPresentationError&));
+};
+
 class TestWebPresentationConnectionCallback
     : public WebPresentationConnectionCallback {
  public:
@@ -172,17 +181,28 @@ class TestPresentationDispatcher : public PresentationDispatcher {
 
 class PresentationDispatcherTest : public ::testing::Test {
  public:
+  enum class URLState { Available, Unavailable, Unsupported };
+
   PresentationDispatcherTest()
       : gurl1_(GURL("https://www.example.com/1.html")),
         gurl2_(GURL("https://www.example.com/2.html")),
-        gurls_({gurl1_, gurl2_}),
+        gurl3_(GURL("https://www.example.com/3.html")),
+        gurl4_(GURL("https://www.example.com/4.html")),
+        gurls_({gurl1_, gurl2_, gurl3_, gurl4_}),
         url1_(WebURL(gurl1_)),
         url2_(WebURL(gurl2_)),
+        url3_(WebURL(gurl3_)),
+        url4_(WebURL(gurl4_)),
         urls_(WebVector<WebURL>(gurls_)),
         presentation_id_(WebString::fromUTF8("test-id")),
         array_buffer_(WebArrayBuffer::create(4, 1)),
         observer_(urls_),
+        mock_observer1_({gurl1_, gurl2_, gurl3_}),
+        mock_observer2_({gurl2_, gurl3_, gurl4_}),
+        mock_observer3_({gurl2_, gurl3_}),
+        mock_observers_({&mock_observer1_, &mock_observer2_, &mock_observer3_}),
         dispatcher_(&presentation_service_) {}
+
   ~PresentationDispatcherTest() override {}
 
   void SetUp() override {
@@ -194,16 +214,66 @@ class PresentationDispatcherTest : public ::testing::Test {
     return static_cast<uint8_t*>(array_buffer_.data());
   }
 
+  void ChangeURLState(const GURL& gurl, URLState state) {

[mark a. foltz, 2017/01/13 19:01:15]

s|gurl|url|

[zhaobin, 2017/01/14 01:45:33]

Done.

+    switch (state) {
+      case URLState::Available:
+        dispatcher_.OnScreenAvailabilityUpdated(gurl, true);
+        break;
+      case URLState::Unavailable:
+        dispatcher_.OnScreenAvailabilityUpdated(gurl, false);
+        break;
+      case URLState::Unsupported:
+        dispatcher_.OnScreenAvailabilityNotSupported(gurl);
+        break;
+    }
+  }
+
+  void StartListening(MockPresentationAvailabilityObserver* observer) {
+    dispatcher_.startListening(observer);

[mark a. foltz, 2017/01/13 19:01:16]

Since |dispatcher_| is protected, it should be possible to call this directly in
the test case.

[zhaobin, 2017/01/14 01:45:33]

Done.

+  }
+
+  void StopListening(MockPresentationAvailabilityObserver* observer) {
+    dispatcher_.stopListening(observer);

[mark a. foltz, 2017/01/13 19:01:16]

Similar comment as StartListening().

[zhaobin, 2017/01/14 01:45:33]

Done.

+  }
+
+  void TestGetAvailability(

[mark a. foltz, 2017/01/13 19:01:16]

Can you document what this is testing? E.g.,

Tests that PresenationService is called for getAvailability(urls), after |urls|
change state to |states|.

[zhaobin, 2017/01/14 01:45:33]

Done.

+      const blink::WebVector<blink::WebURL>& urls,
+      const std::vector<URLState>& states,
+      MockPresentationAvailabilityCallbacks* mock_callback) {
+    for (const auto& url : urls) {
+      EXPECT_CALL(presentation_service_, ListenForScreenAvailability(GURL(url)))
+          .Times(1);
+    }
+
+    base::RunLoop run_loop;
+    client()->getAvailability(urls, base::WrapUnique(mock_callback));

[mark a. foltz, 2017/01/13 19:01:17]

Document that this function takes ownership of |mock_callback|.

[zhaobin, 2017/01/14 01:45:33]

Done.

+    for (size_t i = 0; i < std::min(urls.size(), states.size()); i++)

[mark a. foltz, 2017/01/13 19:01:15]

Should you require urls.size() == states.size()?

[zhaobin, 2017/01/14 01:45:33]

Done.

+      ChangeURLState(urls[i], states[i]);

[mark a. foltz, 2017/01/13 19:01:16]

Nit: blank line after loop statement

[zhaobin, 2017/01/14 01:45:33]

Done.

+    run_loop.RunUntilIdle();

[mark a. foltz, 2017/01/13 19:01:15]

It seems like in most cases |mock_callback| will be invoked; do you need to set
an expectation on it?  Or is that done in the test cases below?

[zhaobin, 2017/01/14 01:45:33]

Set in test cases below.

+  }
+
+  blink::WebPresentationClient* client() { return &dispatcher_; }
+
  protected:
   const GURL gurl1_;
   const GURL gurl2_;
+  const GURL gurl3_;
+  const GURL gurl4_;
   const std::vector<GURL> gurls_;
   const WebURL url1_;
   const WebURL url2_;
+  const WebURL url3_;
+  const WebURL url4_;
   const WebVector<WebURL> urls_;
+  const std::vector<blink::WebVector<blink::WebURL>> urls_set_;

[mark a. foltz, 2017/01/13 19:01:16]

|urls_| ?

[zhaobin, 2017/01/14 01:45:33]

removed :)

   const WebString presentation_id_;
   const WebArrayBuffer array_buffer_;
   MockPresentationAvailabilityObserver observer_;
+  MockPresentationAvailabilityObserver mock_observer1_;
+  MockPresentationAvailabilityObserver mock_observer2_;
+  MockPresentationAvailabilityObserver mock_observer3_;
+  std::vector<MockPresentationAvailabilityObserver*> mock_observers_;
+
   MockPresentationService presentation_service_;
   TestPresentationDispatcher dispatcher_;
 
@@ -372,14 +442,14 @@ TEST_F(PresentationDispatcherTest, TestTerminateSession) {
 
 TEST_F(PresentationDispatcherTest, TestListenForScreenAvailability) {
   base::RunLoop run_loop1;
-  EXPECT_CALL(presentation_service_, ListenForScreenAvailability(gurl1_));
+  for (const auto& gurl : gurls_)
+    EXPECT_CALL(presentation_service_, ListenForScreenAvailability(gurl));
   dispatcher_.getAvailability(
       urls_, base::MakeUnique<WebPresentationAvailabilityCallbacks>());
   dispatcher_.OnScreenAvailabilityUpdated(url1_, true);
   run_loop1.RunUntilIdle();
 
   base::RunLoop run_loop2;
-  EXPECT_CALL(presentation_service_, ListenForScreenAvailability(gurl1_));
   dispatcher_.startListening(&observer_);
   run_loop2.RunUntilIdle();
 
@@ -388,8 +458,10 @@ TEST_F(PresentationDispatcherTest, TestListenForScreenAvailability) {
   dispatcher_.OnScreenAvailabilityUpdated(url1_, false);
   EXPECT_CALL(observer_, availabilityChanged(true));
   dispatcher_.OnScreenAvailabilityUpdated(url1_, true);
-  EXPECT_CALL(presentation_service_,
-              StopListeningForScreenAvailability(gurl1_));
+  for (const auto& gurl : gurls_) {
+    EXPECT_CALL(presentation_service_,
+                StopListeningForScreenAvailability(gurl));
+  }
   dispatcher_.stopListening(&observer_);
   run_loop3.RunUntilIdle();
 
@@ -407,4 +479,207 @@ TEST_F(PresentationDispatcherTest, TestSetDefaultPresentationUrls) {
   run_loop.RunUntilIdle();
 }
 
+TEST_F(PresentationDispatcherTest, GetAvailabilityOneUrlNoAvailabilityChange) {
+  auto* mock_callback = new MockPresentationAvailabilityCallbacks();

[mark a. foltz, 2017/01/13 19:01:16]

You can use a StrictMock<> to verify that mock_callback is not invoked in this
test.

https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md

[zhaobin, 2017/01/14 01:45:33]

Done.

+  blink::WebVector<blink::WebURL> urls(std::vector<GURL>({url1_}));
+  std::vector<URLState> states;
+  TestGetAvailability(urls, states, mock_callback);

[mark a. foltz, 2017/01/13 19:01:16]

Could this just take a vector<GURL> and convert to WebURL internally, so you
could write:

TestGetAvailability({url1_}, {}, mock_callback)

[zhaobin, 2017/01/14 01:45:33]

Done.

+}
+
+TEST_F(PresentationDispatcherTest, GetAvailabilityOneUrlBecomesAvailable) {
+  auto* mock_callback = new MockPresentationAvailabilityCallbacks();
+  EXPECT_CALL(*mock_callback, onSuccess(true));

[mark a. foltz, 2017/01/13 19:01:16]

Nit: consistently add a blank line after setting expectations

[zhaobin, 2017/01/14 01:45:33]

Done.

+  blink::WebVector<blink::WebURL> urls(std::vector<GURL>({url1_}));
+  std::vector<URLState> states = {URLState::Available};
+  TestGetAvailability(urls, states, mock_callback);
+}
+
+TEST_F(PresentationDispatcherTest, GetAvailabilityOneUrlBecomesUnavailable) {
+  auto* mock_callback = new MockPresentationAvailabilityCallbacks();
+  EXPECT_CALL(*mock_callback, onSuccess(false));
+
+  blink::WebVector<blink::WebURL> urls(std::vector<GURL>({url1_}));
+  std::vector<URLState> states = {URLState::Unavailable};
+  TestGetAvailability(urls, states, mock_callback);
+}
+
+TEST_F(PresentationDispatcherTest, GetAvailabilityOneUrlBecomesNotSupported) {
+  auto* mock_callback = new MockPresentationAvailabilityCallbacks();
+  EXPECT_CALL(*mock_callback, onError(_));
+  EXPECT_CALL(presentation_service_,
+              StopListeningForScreenAvailability(gurl1_));
+
+  blink::WebVector<blink::WebURL> urls(std::vector<GURL>({url1_}));
+  std::vector<URLState> states = {URLState::Unsupported};
+  TestGetAvailability(urls, states, mock_callback);
+}
+
+TEST_F(PresentationDispatcherTest,
+       GetAvailabilityMultipleUrlsAllBecomesAvailable) {
+  auto* mock_callback = new MockPresentationAvailabilityCallbacks();
+  EXPECT_CALL(*mock_callback, onSuccess(true)).Times(1);
+
+  blink::WebVector<blink::WebURL> urls(std::vector<GURL>({url1_, url2_}));
+  std::vector<URLState> state_seq = {URLState::Available, URLState::Available};
+  TestGetAvailability(urls, state_seq, mock_callback);
+}
+
+TEST_F(PresentationDispatcherTest,
+       GetAvailabilityMultipleUrlsAllBecomesUnavailable) {
+  auto* mock_callback = new MockPresentationAvailabilityCallbacks();
+  EXPECT_CALL(*mock_callback, onSuccess(false)).Times(1);
+
+  blink::WebVector<blink::WebURL> urls(std::vector<GURL>({url1_, url2_}));
+  std::vector<URLState> state_seq = {URLState::Unavailable,
+                                     URLState::Unavailable};
+  TestGetAvailability(urls, state_seq, mock_callback);
+}
+
+TEST_F(PresentationDispatcherTest,
+       GetAvailabilityMultipleUrlsAllBecomesUnsupported) {
+  auto* mock_callback = new MockPresentationAvailabilityCallbacks();
+  EXPECT_CALL(*mock_callback, onError(_)).Times(1);
+  EXPECT_CALL(presentation_service_,
+              StopListeningForScreenAvailability(gurl1_));
+  EXPECT_CALL(presentation_service_,
+              StopListeningForScreenAvailability(gurl2_));
+
+  blink::WebVector<blink::WebURL> urls(std::vector<GURL>({url1_, url2_}));
+  std::vector<URLState> state_seq = {URLState::Unsupported,
+                                     URLState::Unsupported};
+  TestGetAvailability(urls, state_seq, mock_callback);
+}
+
+TEST_F(PresentationDispatcherTest,
+       GetAvailabilityReturnsDirectlyForAlreadyListeningUrls) {
+  auto* mock_callback_1 = new MockPresentationAvailabilityCallbacks();
+  EXPECT_CALL(*mock_callback_1, onSuccess(false)).Times(1);
+
+  std::vector<URLState> state_seq = {URLState::Unavailable, URLState::Available,
+                                     URLState::Unavailable};
+  TestGetAvailability(mock_observer1_.urls(), state_seq, mock_callback_1);
+
+  auto* mock_callback_2 = new MockPresentationAvailabilityCallbacks();
+  EXPECT_CALL(*mock_callback_2, onSuccess(true)).Times(1);
+
+  base::RunLoop run_loop;
+  client()->getAvailability(mock_observer3_.urls(),
+                            base::WrapUnique(mock_callback_2));
+  run_loop.RunUntilIdle();
+}
+
+TEST_F(PresentationDispatcherTest, StartListeningListenToEachURLOnce) {
+  for (const auto& gurl : gurls_) {
+    EXPECT_CALL(presentation_service_, ListenForScreenAvailability(gurl))
+        .Times(1);
+  }
+
+  base::RunLoop run_loop;
+  for (auto* mock_observer : mock_observers_) {
+    client()->getAvailability(
+        mock_observer->urls(),
+        base::MakeUnique<WebPresentationAvailabilityCallbacks>());
+    StartListening(mock_observer);
+  }
+  run_loop.RunUntilIdle();
+}
+
+TEST_F(PresentationDispatcherTest, StopListeningListenToEachURLOnce) {
+  for (const auto& gurl : gurls_) {
+    EXPECT_CALL(presentation_service_, ListenForScreenAvailability(gurl))
+        .Times(1);
+    EXPECT_CALL(presentation_service_, StopListeningForScreenAvailability(gurl))
+        .Times(1);
+  }
+
+  // Set up |availability_set_| and |listening_status_|
+  base::RunLoop run_loop;
+  for (auto* mock_observer : mock_observers_) {
+    client()->getAvailability(
+        mock_observer->urls(),
+        base::MakeUnique<WebPresentationAvailabilityCallbacks>());
+  }
+  // Clean up callbacks.
+  ChangeURLState(gurl2_, URLState::Unavailable);
+
+  for (auto* mock_observer : mock_observers_)
+    StartListening(mock_observer);
+  for (auto* mock_observer : mock_observers_)
+    StopListening(mock_observer);
+
+  run_loop.RunUntilIdle();
+}
+
+TEST_F(PresentationDispatcherTest,
+       StopListeningDoesNotStopIfURLListenedByOthers) {
+  for (const auto& gurl : gurls_) {
+    EXPECT_CALL(presentation_service_, ListenForScreenAvailability(gurl))
+        .Times(1);
+  }
+  EXPECT_CALL(presentation_service_, StopListeningForScreenAvailability(gurl1_))
+      .Times(1);
+  EXPECT_CALL(presentation_service_, StopListeningForScreenAvailability(gurl2_))
+      .Times(0);
+  EXPECT_CALL(presentation_service_, StopListeningForScreenAvailability(gurl3_))
+      .Times(0);
+
+  // Set up |availability_set_| and |listening_status_|
+  base::RunLoop run_loop;
+  for (auto& mock_observer : mock_observers_) {
+    client()->getAvailability(
+        mock_observer->urls(),
+        base::MakeUnique<WebPresentationAvailabilityCallbacks>());
+  }
+
+  // Clean up callbacks.
+  ChangeURLState(gurl2_, URLState::Unavailable);

[mark a. foltz, 2017/01/13 19:01:16]

Set expectation that mock_observer{1,2}_ will have availabilityChanged(false)
invoked

[zhaobin, 2017/01/14 01:45:33]

Done.

+
+  for (auto* mock_observer : mock_observers_)
+    StartListening(mock_observer);
+  StopListening(&mock_observer1_);
+
+  run_loop.RunUntilIdle();
+}
+
+TEST_F(PresentationDispatcherTest,
+       OnScreenAvailabilityUpdatedInvokesAvailabilityChanged) {
+  for (const auto& gurl : gurls_) {
+    EXPECT_CALL(presentation_service_, ListenForScreenAvailability(gurl))
+        .Times(1);
+  }
+
+  EXPECT_CALL(mock_observer1_, availabilityChanged(true));
+
+  base::RunLoop run_loop;
+  for (auto* mock_observer : mock_observers_) {
+    client()->getAvailability(
+        mock_observer->urls(),
+        base::MakeUnique<WebPresentationAvailabilityCallbacks>());
+    StartListening(mock_observer);
+  }
+
+  ChangeURLState(gurl1_, URLState::Available);

[mark a. foltz, 2017/01/13 19:01:16]

Can you change the state to Unavailable and test that availabiltyChanged(false)
is invoked?

[zhaobin, 2017/01/14 01:45:33]

Done.

+  run_loop.RunUntilIdle();
+}
+
+TEST_F(PresentationDispatcherTest,
+       OnScreenAvailabilityUpdatedInvokesMultipleAvailabilityChanged) {

[mark a. foltz, 2017/01/13 19:01:15]

I didn't see any test of availabilityChanged() in this test case.

[zhaobin, 2017/01/14 01:45:33]

Done.

+  for (const auto& gurl : gurls_) {
+    EXPECT_CALL(presentation_service_, ListenForScreenAvailability(gurl))
+        .Times(1);
+  }
+
+  base::RunLoop run_loop;
+  for (auto* mock_observer : mock_observers_) {
+    EXPECT_CALL(*mock_observer, availabilityChanged(true));
+    client()->getAvailability(
+        mock_observer->urls(),
+        base::MakeUnique<WebPresentationAvailabilityCallbacks>());
+    StartListening(mock_observer);
+  }
+
+  ChangeURLState(gurl2_, URLState::Available);
+  run_loop.RunUntilIdle();
+}
+
 }  // namespace content