[eme] Reject CDM calls after connection error

media/mojo/clients/mojo_cdm_unittest.cc

Index: media/mojo/clients/mojo_cdm_unittest.cc
diff --git a/media/mojo/clients/mojo_cdm_unittest.cc b/media/mojo/clients/mojo_cdm_unittest.cc
index 3dee666e847cac101750556c08ee5c380f725d3e..77dbfac035477765f312035b83363444e61081dc 100644
--- a/media/mojo/clients/mojo_cdm_unittest.cc
+++ b/media/mojo/clients/mojo_cdm_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/test/test_message_loop.h"
+#include "media/base/cdm_callback_promise.h"
 #include "media/base/cdm_config.h"
 #include "media/base/mock_filters.h"
 #include "media/cdm/default_cdm_factory.h"
@@ -19,8 +20,14 @@
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ::testing::_;
+using ::testing::SaveArg;
 using ::testing::StrictMock;
 
+MATCHER(IsNotEmpty, "") {

[xhwang, 2016/12/09 06:13:30]

s/IsNotEmpty/NotEmpty

[jrummell, 2016/12/13 20:57:34]

Done.

+  return !arg.empty();
+}
+
 namespace media {
 
 namespace {
@@ -28,11 +35,18 @@ namespace {
 const char kClearKeyKeySystem[] = "org.w3.clearkey";
 const char kTestSecurityOrigin[] = "https://www.test.com";
 
+// Random key ID used to create a session.
+const uint8_t kKeyId[] = {
+    // base64 equivalent is AQIDBAUGBwgJCgsMDQ4PEA
+    0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
+    0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10,
+};
+
 }  // namespace
 
 class MojoCdmTest : public ::testing::Test {
  public:
-  enum ExpectedResult { SUCCESS, CONNECTION_ERROR };
+  enum ExpectedResult { SUCCESS, CONNECTION_ERROR, FAILURE };
 
   MojoCdmTest()
       : mojo_cdm_service_(base::MakeUnique<MojoCdmService>(
@@ -53,6 +67,9 @@ class MojoCdmTest : public ::testing::Test {
       case CONNECTION_ERROR:
         cdm_request.ResetWithReason(0, "Request dropped for testing.");
         break;
+      case FAILURE:
+        FAIL();

[xhwang, 2016/12/09 06:13:30]

Can you actually add a test to cover initialization failure? You can probably
use an unsupported key system.

[jrummell, 2016/12/13 20:57:34]

Done.

+        break;
     }
 
     MojoCdm::Create(kClearKeyKeySystem, GURL(kTestSecurityOrigin), CdmConfig(),
@@ -69,18 +86,13 @@ class MojoCdmTest : public ::testing::Test {
                                base::Unretained(this), expected_result));
 
     base::RunLoop().RunUntilIdle();
-
-    if (expected_result == SUCCESS) {
-      EXPECT_TRUE(mojo_cdm_);
-    } else {
-      EXPECT_FALSE(mojo_cdm_);
-    }
   }
 
   void OnCdmCreated(ExpectedResult expected_result,
                     const scoped_refptr<MediaKeys>& cdm,
                     const std::string& error_message) {
     if (!cdm) {
+      EXPECT_EQ(CONNECTION_ERROR, expected_result);

[xhwang, 2016/12/09 06:13:30]

If you actually have a init failure test, this will not be true.

[jrummell, 2016/12/13 20:57:35]

Fixed.

       DVLOG(1) << error_message;
       return;
     }
@@ -89,6 +101,96 @@ class MojoCdmTest : public ::testing::Test {
     mojo_cdm_ = cdm;
   }
 
+  void ForceConnectionError() {
+    // If there is an existing session it will get closed when the connection
+    // is broken.
+    if (!session_id_.empty()) {
+      EXPECT_CALL(cdm_client_, OnSessionClosed(session_id_));
+    }
+
+    static_cast<MojoCdm*>(mojo_cdm_.get())
+        ->OnConnectionError(1, "Dropping connection for testing.");
+  }

[xhwang, 2016/12/09 06:13:30]

You can reset the cdm_binding_ to trigger a connection error.

[jrummell, 2016/12/13 20:57:35]

Done.

+
+  void SetServerCertificateAndExpect(const std::vector<uint8_t>& certificate,
+                                     ExpectedResult expected_result) {
+    mojo_cdm_->SetServerCertificate(certificate,
+                                    CreatePromise(expected_result));
+
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void CreateSessionAndExpect(EmeInitDataType data_type,
+                              const std::vector<uint8_t>& key_id,
+                              ExpectedResult expected_result) {
+    if (expected_result == SUCCESS) {
+      EXPECT_CALL(cdm_client_, OnSessionMessage(IsNotEmpty(), _, _));
+    }
+
+    mojo_cdm_->CreateSessionAndGenerateRequest(
+        MediaKeys::SessionType::TEMPORARY_SESSION, data_type, key_id,
+        CreateSessionPromise(expected_result));
+
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void CloseSessionAndExpect(ExpectedResult expected_result) {
+    DCHECK(!session_id_.empty()) << "CloseSessionAndExpect() must be called "
+                                    "after a successful "
+                                    "CreateSessionAndExpect()";
+
+    if (expected_result == SUCCESS) {
+      EXPECT_CALL(cdm_client_, OnSessionClosed(session_id_));
+    }
+
+    mojo_cdm_->CloseSession(session_id_, CreatePromise(expected_result));
+
+    base::RunLoop().RunUntilIdle();
+  }

[xhwang, 2016/12/09 06:13:30]

This really is testing AesDecryptor, not MojoCdm...

[jrummell, 2016/12/13 20:57:34]

In the failure case MojoCdm will fail the promise. But you're right about the
success case. I only added a success test to make sure the code actually worked
if the CDM was connected.

+
+  // Create a promise. |expected_result| is used to indicate how the promise
+  // should be fulfilled.
+  std::unique_ptr<SimpleCdmPromise> CreatePromise(
+      ExpectedResult expected_result) {
+    if (expected_result == SUCCESS) {
+      EXPECT_CALL(*this, OnResolve());
+    } else {
+      EXPECT_CALL(*this, OnReject(_, _, IsNotEmpty()));
+    }
+
+    std::unique_ptr<SimpleCdmPromise> promise(new CdmCallbackPromise<>(
+        base::Bind(&MojoCdmTest::OnResolve, base::Unretained(this)),
+        base::Bind(&MojoCdmTest::OnReject, base::Unretained(this))));
+    return promise;
+  }
+
+  // Create a promise to be used when a new session is created.
+  // |expected_result| is used to indicate how the promise should be fulfilled.
+  std::unique_ptr<NewSessionCdmPromise> CreateSessionPromise(
+      ExpectedResult expected_result) {
+    if (expected_result == SUCCESS) {
+      EXPECT_CALL(*this, OnResolveWithSession(_))
+          .WillOnce(SaveArg<0>(&session_id_));
+    } else {
+      EXPECT_CALL(*this, OnReject(_, _, IsNotEmpty()));
+    }
+
+    std::unique_ptr<NewSessionCdmPromise> promise(
+        new CdmCallbackPromise<std::string>(
+            base::Bind(&MojoCdmTest::OnResolveWithSession,
+                       base::Unretained(this)),
+            base::Bind(&MojoCdmTest::OnReject, base::Unretained(this))));
+    return promise;
+  }
+
+  // Methods used for promise resolved/rejected.
+  MOCK_METHOD0(OnResolve, void());
+  MOCK_METHOD1(OnResolveWithSession, void(const std::string& session_id));
+  MOCK_METHOD3(OnReject,
+               void(CdmPromise::Exception exception_code,
+                    uint32_t system_code,
+                    const std::string& error_message));

[xhwang, 2016/12/09 06:13:30]

We have this in multiple tests. Maybe we should have a MockCdmPromise in
mock_filters.h.

[jrummell, 2016/12/13 20:57:34]

Done. I'll update the other places in another CL.

+
   // Fixture members.
   base::TestMessageLoop message_loop_;
 
@@ -103,6 +205,9 @@ class MojoCdmTest : public ::testing::Test {
   mojo::Binding<mojom::ContentDecryptionModule> cdm_binding_;
   scoped_refptr<MediaKeys> mojo_cdm_;
 
+  // |session_id_| is the latest successful result of calling CreateSession().
+  std::string session_id_;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(MojoCdmTest);
 };
@@ -115,6 +220,35 @@ TEST_F(MojoCdmTest, Create_ConnectionError) {
   Initialize(CONNECTION_ERROR);
 }
 
-// TODO(xhwang): Add more test cases!
+TEST_F(MojoCdmTest, SetServerCertificate_AfterConnectionError) {
+  Initialize(SUCCESS);
+  ForceConnectionError();
+  SetServerCertificateAndExpect({0, 1, 2}, FAILURE);
+}
+
+TEST_F(MojoCdmTest, CreateSessionAndGenerateRequest_AfterConnectionError) {
+  std::vector<uint8_t> key_id(kKeyId, kKeyId + arraysize(kKeyId));
+
+  Initialize(SUCCESS);
+  ForceConnectionError();
+  CreateSessionAndExpect(EmeInitDataType::WEBM, key_id, FAILURE);
+}
+
+TEST_F(MojoCdmTest, CloseSession_Success) {
+  std::vector<uint8_t> key_id(kKeyId, kKeyId + arraysize(kKeyId));
+
+  Initialize(SUCCESS);
+  CreateSessionAndExpect(EmeInitDataType::WEBM, key_id, SUCCESS);
+  CloseSessionAndExpect(SUCCESS);
+}
+
+TEST_F(MojoCdmTest, CloseSession_AfterConnectionError) {
+  std::vector<uint8_t> key_id(kKeyId, kKeyId + arraysize(kKeyId));
+
+  Initialize(SUCCESS);
+  CreateSessionAndExpect(EmeInitDataType::WEBM, key_id, SUCCESS);
+  ForceConnectionError();
+  CloseSessionAndExpect(FAILURE);
+}

[xhwang, 2016/12/09 06:13:30]

If you add a MockCdm first and use it in this test instead of the AesDecryptor,
it'll be more convenient to add more tests (e.g. update).

[jrummell, 2016/12/13 20:57:34]

Agreed. Later :)

 
 }  // namespace media