media: Avoid access violation in MojoCdm 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
new file mode 100644
index 0000000000000000000000000000000000000000..a08e26a7edb83095db5265b439a481ae8955cc08
--- /dev/null
+++ b/media/mojo/clients/mojo_cdm_unittest.cc
@@ -0,0 +1,117 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <stdint.h>
+
+#include "base/bind.h"
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/run_loop.h"
+#include "base/test/test_message_loop.h"
+#include "media/base/cdm_config.h"
+#include "media/base/mock_filters.h"
+#include "media/cdm/default_cdm_factory.h"
+#include "media/mojo/clients/mojo_cdm.h"
+#include "media/mojo/interfaces/content_decryption_module.mojom.h"
+#include "media/mojo/services/mojo_cdm_service.h"
+#include "media/mojo/services/mojo_cdm_service_context.h"
+#include "mojo/public/cpp/bindings/interface_request.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::StrictMock;
+
+namespace media {
+
+namespace {
+
+const char kClearKeyKeySystem[] = "org.w3.clearkey";
+const char kTestSecurityOrigin[] = "https://www.test.com";
+
+}  // namespace
+
+class MojoCdmTest : public ::testing::Test {
+ public:
+  enum ExpectedResult { SUCCESS, CONNECTION_ERROR };
+
+  MojoCdmTest()
+      : mojo_cdm_service_(base::MakeUnique<MojoCdmService>(
+            mojo_cdm_service_context_.GetWeakPtr(),
+            &cdm_factory_)),
+        cdm_binding_(mojo_cdm_service_.get()) {}
+
+  virtual ~MojoCdmTest() {}
+
+  void Initialize(ExpectedResult expected_result) {
+    mojom::ContentDecryptionModulePtr remote_cdm;
+    auto cdm_request = mojo::GetProxy(&remote_cdm);
+
+    switch (expected_result) {
+      case SUCCESS:
+        cdm_binding_.Bind(std::move(cdm_request));
+        break;
+      case CONNECTION_ERROR:
+        cdm_request.ResetWithReason(0, "Request dropped for testing.");
+        break;
+    }
+
+    MojoCdm::Create(
+        kClearKeyKeySystem, GURL(kTestSecurityOrigin), CdmConfig(),
+        std::move(remote_cdm), base::Bind(&MockCdmClient::OnSessionMessage,
+                                          base::Unretained(&cdm_client_)),
+        base::Bind(&MockCdmClient::OnSessionClosed,
+                   base::Unretained(&cdm_client_)),
+        base::Bind(&MockCdmClient::OnSessionKeysChange,
+                   base::Unretained(&cdm_client_)),
+        base::Bind(&MockCdmClient::OnSessionExpirationUpdate,
+                   base::Unretained(&cdm_client_)),
+        base::Bind(&MojoCdmTest::OnCdmCreated, base::Unretained(this)));
+
+    base::RunLoop().RunUntilIdle();
+
+    if (expected_result == SUCCESS) {
+      EXPECT_TRUE(mojo_cdm_);
+    } else {
+      EXPECT_FALSE(mojo_cdm_);
+    }
+  }
+
+  void OnCdmCreated(const scoped_refptr<MediaKeys>& cdm,
+                    const std::string& error_message) {

[jrummell, 2016/12/06 21:07:07]

Maybe pass expected_result in the Bind() above, and put
EXPECT_EQ(expected_result, CONNECTION_ERROR); (or SUCCESS) as appropriate in
this function.

[xhwang, 2016/12/06 21:33:43]

In the future we may have other types of failures, so the only thing we can
reliably check is

if (cdm) {
  EXPECT_EQ(expected_result, SUCCESS);
}

+    if (!cdm) {
+      LOG(ERROR) << error_message;

[jrummell, 2016/12/06 21:07:07]

I think it would be better if this simply did
EXPECT_FALSE(error_message.empty()). No need to log the message.

[xhwang, 2016/12/06 21:33:43]

We don't "force" all impls to provide a non-empty message upon failure.  
Updating this to DVLOG() to be less noisy.

+      return;
+    }
+
+    mojo_cdm_ = cdm;
+  }
+
+  // Fixture members.
+  base::TestMessageLoop message_loop_;
+
+  MojoCdmServiceContext mojo_cdm_service_context_;
+  StrictMock<MockCdmClient> cdm_client_;
+
+  // TODO(jrummell): Use a MockCdmFactory to create a MockCdm here to more test

[jrummell, 2016/12/06 21:07:07]

s/to more/for more/

[xhwang, 2016/12/06 21:33:43]

Done.

+  // coverage.
+  DefaultCdmFactory cdm_factory_;
+
+  std::unique_ptr<MojoCdmService> mojo_cdm_service_;
+  mojo::Binding<mojom::ContentDecryptionModule> cdm_binding_;
+  scoped_refptr<MediaKeys> mojo_cdm_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MojoCdmTest);
+};
+
+TEST_F(MojoCdmTest, Create_Success) {
+  Initialize(SUCCESS);
+}
+
+TEST_F(MojoCdmTest, Create_ConnectionError) {
+  Initialize(CONNECTION_ERROR);
+}
+
+// TODO(xhwang): Add more test cases!
+
+}  // namespace media