[Presentation API] Additional plumbing for PresentationServiceImpl.

content/browser/presentation/presentation_service_impl_unittest.cc

Index: content/browser/presentation/presentation_service_impl_unittest.cc
diff --git a/content/browser/presentation/presentation_service_impl_unittest.cc b/content/browser/presentation/presentation_service_impl_unittest.cc
index 89a6b3c779aa02419bb6b210c2d8f33c1e779ae4..aaed328245333be415efb102cb10002ffa708e3f 100644
--- a/content/browser/presentation/presentation_service_impl_unittest.cc
+++ b/content/browser/presentation/presentation_service_impl_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/run_loop.h"
 #include "content/browser/presentation/presentation_service_impl.h"
 #include "content/public/browser/presentation_service_delegate.h"
+#include "content/public/browser/presentation_session.h"
 #include "content/test/test_render_frame_host.h"
 #include "content/test/test_render_view_host.h"
 #include "content/test/test_web_contents.h"
@@ -18,6 +19,7 @@ using ::testing::Eq;
 using ::testing::InvokeWithoutArgs;
 using ::testing::Mock;
 using ::testing::Return;
+using ::testing::SaveArg;
 
 namespace content {
 
@@ -41,6 +43,27 @@ class MockPresentationServiceDelegate : public PresentationServiceDelegate {
       void(
           int render_process_id,
           int routing_id));
+  MOCK_METHOD3(SetDefaultPresentationUrl,
+      void(
+          int render_process_id,
+          int routing_id,
+          const std::string& default_presentation_url));
+  MOCK_METHOD6(StartSession,
+      void(
+          int render_process_id,
+          int render_frame_id,
+          const std::string& presentation_url,
+          const std::string& presentation_id,
+          const PresentationSessionSuccessCallback& success_cb,
+          const PresentationSessionErrorCallback& error_cb));
+  MOCK_METHOD6(JoinSession,
+      void(
+          int render_process_id,
+          int render_frame_id,
+          const std::string& presentation_url,
+          const std::string& presentation_id,
+          const PresentationSessionSuccessCallback& success_cb,
+          const PresentationSessionErrorCallback& error_cb));
 };
 
 class PresentationServiceImplTest : public RenderViewHostImplTestHarness {
@@ -93,6 +116,10 @@ class PresentationServiceImplTest : public RenderViewHostImplTestHarness {
     EXPECT_TRUE(it == contexts.end());
   }
 
+  void RunLoopUntilIdle() {
+    base::RunLoop().RunUntilIdle();
+  }
+
   void RunLoopFor(base::TimeDelta duration) {
     base::RunLoop run_loop;
     base::MessageLoop::current()->PostDelayedTask(
@@ -126,6 +153,31 @@ class PresentationServiceImplTest : public RenderViewHostImplTestHarness {
       run_loop_quit_closure_.Run();
   }
 
+  void ExpectReset() {
+    EXPECT_CALL(mock_delegate_, RemoveAllScreenAvailabilityListeners(_, _))
+        .Times(1);
+    EXPECT_CALL(mock_delegate_, SetDefaultPresentationUrl(_, _, Eq("")))
+        .Times(1);
+  }
+
+  void ExpectNewSessionMojoCallbackSuccess(
+      presentation::PresentationSessionInfoPtr info,
+      presentation::PresentationErrorPtr error) {
+    EXPECT_FALSE(info.is_null());
+    EXPECT_TRUE(error.is_null());
+    if (!run_loop_quit_closure_.is_null())
+      run_loop_quit_closure_.Run();
+  }
+
+  void ExpectNewSessionMojoCallbackError(
+      presentation::PresentationSessionInfoPtr info,
+      presentation::PresentationErrorPtr error) {
+    EXPECT_TRUE(info.is_null());
+    EXPECT_FALSE(error.is_null());
+    if (!run_loop_quit_closure_.is_null())
+      run_loop_quit_closure_.Run();
+  }
+
   MockPresentationServiceDelegate mock_delegate_;
   scoped_ptr<PresentationServiceImpl> service_impl_;
   mojo::InterfacePtr<presentation::PresentationService> service_ptr_;
@@ -170,7 +222,7 @@ TEST_F(PresentationServiceImplTest, GetScreenAvailability) {
   EXPECT_EQ(2, callback_count_);
 }
 
-TEST_F(PresentationServiceImplTest, RemoveAllListeners) {
+TEST_F(PresentationServiceImplTest, Reset) {
   std::string presentation_url("http://fooUrl");
   GetScreenAvailabilityAndWait(
       presentation_url,
@@ -178,7 +230,8 @@ TEST_F(PresentationServiceImplTest, RemoveAllListeners) {
           base::Unretained(this)),
       true);
 
-  service_impl_->RemoveAllListeners();
+  ExpectReset();
+  service_impl_->Reset();
 
   ExpectListenerDoesNotExist(presentation_url);
 
@@ -193,6 +246,7 @@ TEST_F(PresentationServiceImplTest, DidNavigateThisFrame) {
           base::Unretained(this)),
       true);
 
+  ExpectReset();
   service_impl_->DidNavigateAnyFrame(
       contents()->GetMainFrame(),
       content::LoadCommittedDetails(),
@@ -232,6 +286,7 @@ TEST_F(PresentationServiceImplTest, ThisRenderFrameDeleted) {
           base::Unretained(this)),
       true);
 
+  ExpectReset();
   service_impl_->RenderFrameDeleted(contents()->GetMainFrame());
 
   ExpectListenerDoesNotExist(presentation_url);
@@ -296,4 +351,134 @@ TEST_F(PresentationServiceImplTest, DelegateFails) {
   ExpectListenerDoesNotExist(presentation_url);
 }
 
+TEST_F(PresentationServiceImplTest, SetDefaultPresentationUrl) {
+  std::string url1("http://fooUrl");
+  EXPECT_CALL(mock_delegate_, SetDefaultPresentationUrl(_, _, Eq(url1)))
+        .Times(1);
+  service_impl_->SetDefaultPresentationUrl(url1);
+
+  std::string url2("http://barUrl");
+  EXPECT_CALL(mock_delegate_, SetDefaultPresentationUrl(_, _, Eq(url2)))
+        .Times(1);
+  service_impl_->SetDefaultPresentationUrl(url2);
+}
+
+TEST_F(PresentationServiceImplTest, SetSameDefaultPresentationUrl) {
+  std::string url("http://fooUrl");
+  EXPECT_CALL(mock_delegate_, SetDefaultPresentationUrl(_, _, Eq(url)))
+        .Times(1);
+  service_impl_->SetDefaultPresentationUrl(url);
+  EXPECT_TRUE(Mock::VerifyAndClearExpectations(&mock_delegate_));

[whywhat, 2015/03/10 17:05:04]

Shouldn't this be called (or even not called) after the second
SetDefaultPresentatilUrl call to verify that the delegate is actually called
only once?

[imcheng, 2015/03/10 18:31:15]

Of course, I added that after the second call.

+
+  // Same URL as before; no-ops.
+  service_impl_->SetDefaultPresentationUrl(url);
+}
+
+TEST_F(PresentationServiceImplTest, ClearDefaultPresentationUrl) {
+  std::string url("http://fooUrl");
+  EXPECT_CALL(mock_delegate_, SetDefaultPresentationUrl(_, _, Eq(url)))

[whywhat, 2015/03/10 17:05:04]

I don't think you need this EXPECT_CALL here - you already test this
functionality in the first test case. No need to test it in every test case.

[imcheng, 2015/03/10 18:31:16]

I need the EXPECT_CALL to satisfy the mock. Otherwise I get spammy warning
messages about unexpected calls.

+        .Times(1);
+  service_impl_->SetDefaultPresentationUrl(url);
+
+  // Same URL as before; no-ops.

[whywhat, 2015/03/10 17:05:05]

The comment seems to be irrelevant to the actual test.

[imcheng, 2015/03/10 18:31:16]

Removed.

+  EXPECT_CALL(
+      mock_delegate_,
+      SetDefaultPresentationUrl(_, _, Eq(std::string())))
+      .Times(1);

[whywhat, 2015/03/10 17:05:05]

The test doesn't show there's any difference between setting a different
non-empty DPU vs an empty DPU. Is there anything else this test could check?

[imcheng, 2015/03/10 18:31:16]

How about checking default_presentation_url_ field is empty?

[whywhat, 2015/03/10 23:20:23]

Hm, in the non-empty case, the default_presentation_url_ would be non-empty,
right. So you would check it's the same value as was passed in both cases (which
is a good expectation to add I guess).

What's different in the implementation is juggling the callback for the DPU I
guess, so you could check there's no pending callback if DPU is set to empty,
but there's one if it's set to non-empty.

Although given the first comment discussion, it might not be true anymore.

[imcheng, 2015/03/12 20:03:40]

Ok, I will add the assertions that you mentioned.

To clarify, given the way the device availability listeners are implemented in
Blink and the current proposed DPU spec, is it logically correct for the PSImpl
to transfer the callback when there's a listener already registered and the DPU
changes?

+  service_impl_->SetDefaultPresentationUrl(std::string());
+}
+
+TEST_F(PresentationServiceImplTest, StartSessionSuccess) {
+  std::string presentation_url("http://fooUrl");
+  std::string presentation_id("presentationId");
+  service_ptr_->StartSession(
+      presentation_url,
+      presentation_id,
+      base::Bind(
+          &PresentationServiceImplTest::ExpectNewSessionMojoCallbackSuccess,
+          base::Unretained(this)));
+  base::Callback<void(const PresentationSessionInfo&)> success_cb;
+  EXPECT_CALL(mock_delegate_, StartSession(
+      _, _, Eq(presentation_url), Eq(presentation_id), _, _))
+      .WillOnce(SaveArg<4>(&success_cb));

[whywhat, 2015/03/10 17:05:04]

Could you use InvokeArgument<4> here instead?

[imcheng, 2015/03/10 18:31:16]

I tried this and unfortunately it does not work. InvokeArgument requires the
argument to have a call operator, which does not exist in base::Callback.

We could define our own action template like this example:
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...

but it seems like overkill to me. Thoughts?

[whywhat, 2015/03/10 23:20:23]

I think you should leave it as it is then. Sorry my suggestion didn't work.

[imcheng, 2015/03/12 20:03:41]

Acknowledged.

+  RunLoopUntilIdle();
+  success_cb.Run(PresentationSessionInfo(presentation_url, presentation_id));
+  SaveQuitClosureAndRunLoop();
+}
+
+TEST_F(PresentationServiceImplTest, StartSessionError) {
+  std::string presentation_url("http://fooUrl");
+  std::string presentation_id("presentationId");
+  service_ptr_->StartSession(
+      presentation_url,
+      presentation_id,
+      base::Bind(
+          &PresentationServiceImplTest::ExpectNewSessionMojoCallbackError,
+          base::Unretained(this)));
+  base::Callback<void(const PresentationError&)> error_cb;
+  EXPECT_CALL(mock_delegate_, StartSession(
+      _, _, Eq(presentation_url), Eq(presentation_id), _, _))
+      .WillOnce(SaveArg<5>(&error_cb));
+  RunLoopUntilIdle();
+  error_cb.Run(PresentationError(PRESENTATION_ERROR_UNKNOWN, "Error message"));
+  SaveQuitClosureAndRunLoop();
+}
+
+TEST_F(PresentationServiceImplTest, JoinSessionSuccess) {
+  std::string presentation_url("http://fooUrl");
+  std::string presentation_id("presentationId");
+  service_ptr_->JoinSession(
+      presentation_url,
+      presentation_id,
+      base::Bind(
+          &PresentationServiceImplTest::ExpectNewSessionMojoCallbackSuccess,
+          base::Unretained(this)));
+  base::Callback<void(const PresentationSessionInfo&)> success_cb;
+  EXPECT_CALL(mock_delegate_, JoinSession(
+      _, _, Eq(presentation_url), Eq(presentation_id), _, _))
+      .WillOnce(SaveArg<4>(&success_cb));
+  RunLoopUntilIdle();
+  success_cb.Run(PresentationSessionInfo(presentation_url, presentation_id));
+  SaveQuitClosureAndRunLoop();
+}
+
+TEST_F(PresentationServiceImplTest, JoinSessionError) {
+  std::string presentation_url("http://fooUrl");
+  std::string presentation_id("presentationId");
+  service_ptr_->JoinSession(
+      presentation_url,
+      presentation_id,
+      base::Bind(
+          &PresentationServiceImplTest::ExpectNewSessionMojoCallbackError,
+          base::Unretained(this)));
+  base::Callback<void(const PresentationError&)> error_cb;
+  EXPECT_CALL(mock_delegate_, JoinSession(
+      _, _, Eq(presentation_url), Eq(presentation_id), _, _))
+      .WillOnce(SaveArg<5>(&error_cb));
+  RunLoopUntilIdle();
+  error_cb.Run(PresentationError(PRESENTATION_ERROR_UNKNOWN, "Error message"));
+  SaveQuitClosureAndRunLoop();
+}
+
+TEST_F(PresentationServiceImplTest, StartSessionInProgress) {

[whywhat, 2015/03/10 17:05:04]

Perhaps add the same test case for JoinSession after JoinSession and
combinations (Join after Start and vice versa?) Could be in the same test case I
guess.

[imcheng, 2015/03/10 18:31:16]

I can add a test for multiple join sessions in progress once it's clarified.

+  std::string presentation_url("http://fooUrl");
+  std::string presentation_id("presentationId");
+  service_ptr_->StartSession(
+      presentation_url,
+      presentation_id,
+      base::Bind(
+          &PresentationServiceImplTest::ExpectNewSessionMojoCallbackSuccess,
+          base::Unretained(this)));
+  service_ptr_->StartSession(
+      presentation_url,
+      presentation_id,
+      base::Bind(
+          &PresentationServiceImplTest::ExpectNewSessionMojoCallbackSuccess,
+          base::Unretained(this)));
+  EXPECT_CALL(mock_delegate_, StartSession(
+      _, _, Eq(presentation_url), Eq(presentation_id), _, _))
+      .Times(1);
+  RunLoopUntilIdle();
+}
+
 }  // namespace content