Media Remoting: Add remoting control logic for encrypted contents.

media/remoting/remoting_cdm.h

+// 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.
+
+#ifndef MEDIA_REMOTING_REMOTING_CDM_H_
+#define MEDIA_REMOTING_REMOTING_CDM_H_
+
+#include "media/base/cdm_context.h"
+#include "media/base/cdm_factory.h"
+#include "media/base/media_keys.h"
+#include "media/remoting/remoting_cdm_controller.h"
+
+namespace media {
+
+// TODO(xjz): Merge this with Eric's implementation.

[xhwang, 2016/11/01 08:21:28]

nit: use ldap instead of first name for easier tracking

[xjz, 2016/11/01 21:55:52]

Done.

+class RemotingCdm : public MediaKeys, public CdmContext {
+ public:
+  RemotingCdm(const std::string& key_system,
+              const GURL& security_origin,
+              const CdmConfig& cdm_config,
+              const SessionMessageCB& session_message_cb,
+              const SessionClosedCB& session_closed_cb,
+              const SessionKeysChangeCB& session_keys_change_cb,
+              const SessionExpirationUpdateCB& session_expiration_update_cb,
+              const CdmCreatedCB& cdm_created_cb,
+              std::unique_ptr<RemotingCdmController> remoting_controller);
+
+  // If |cdm| is an instance of RemotingCdm, return a type-casted pointer to it.
+  // Otherwise, return nullptr.
+  static RemotingCdm* From(CdmContext* cdm);

[xhwang, 2016/11/01 08:21:28]

We don't want to get the CDM from the CdmContext :) Please see comment below (on
OnSetCdm()) for reasons.

[xjz, 2016/11/01 21:55:52]

Done.

+
+  RemotingSourceImpl* GetRemotingSource();

[xhwang, 2016/11/01 08:21:28]

By looking at code below, it seems RSI is ref-counted. We should not pass around
raw-pointers to a ref-counted object...

[xhwang, 2016/11/01 08:21:28]

Who owns the returned value? If it's owned by this CDM, how can we make sure the
returned value isn't accessed after this CDM is destroyed?

[xjz, 2016/11/01 21:55:52]

The returned value is stored to a scoped_refptr.

[xjz, 2016/11/01 21:55:52]

The below was commented by miu:
General scoped_refptr usage note: The return type of a direct accessor should be
the raw pointer (RemotingSourceImpl*) instead. This lets the caller decide
whether to increment the ref-count by storing the raw pointer into a
scoped_refptr.

+miu

[xhwang, 2016/11/02 06:47:05]

hmm, this seems a pattern that is not recommended now:

https://www.chromium.org/developers/smart-pointer-guidelines

If a function takes or returns a raw pointer, it may mean no ownership is
transferred, or it may not. Much of Chromium was written before
std::unique_ptr<> existed, or by people unfamiliar with using it to indicate
ownership transfers, and thus takes or returns raw pointers but transfers
ownership in the process. Because the compiler can't enforce correct behavior
here, this is less safe. Consider cleaning up such code, so that functions which
take or return raw pointers never transfer ownership.

This is more for unique_ptr, but I feel the idea also applies to
scoped_refptr...

[xjz, 2016/11/02 17:38:11]

+miu

Here is the coding style:
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...
It seems that we don't need to return raw pointer after c++11. WDYT?

[miu, 2016/11/03 00:03:08]

This is the text guiding this decision: "Conventions for return values are
similar: return raw pointers when the caller does not take ownership, and return
smart pointers by value otherwise, potentially in conjunction with std::move()."

So, does the caller take ownership? If not, raw pointer. If so, scoped_refptr<>.

[xhwang, 2016/11/03 06:41:01]

Agreed. But in the comment above, xjz@ said "the returned value is stored to a
scoped_refptr". In this case we should return scoped_refptr<> instead of the raw
pointer?

+
+  // MediaKeys implementations.
+  void SetServerCertificate(const std::vector<uint8_t>& certificate,
+                            std::unique_ptr<SimpleCdmPromise> promise) override;
+  void CreateSessionAndGenerateRequest(
+      SessionType session_type,
+      EmeInitDataType init_data_type,
+      const std::vector<uint8_t>& init_data,
+      std::unique_ptr<NewSessionCdmPromise> promise) override;
+  void LoadSession(SessionType session_type,
+                   const std::string& session_id,
+                   std::unique_ptr<NewSessionCdmPromise> promise) override;
+  void UpdateSession(const std::string& session_id,
+                     const std::vector<uint8_t>& response,
+                     std::unique_ptr<SimpleCdmPromise> promise) override;
+  void CloseSession(const std::string& session_id,
+                    std::unique_ptr<SimpleCdmPromise> promise) override;
+  void RemoveSession(const std::string& session_id,
+                     std::unique_ptr<SimpleCdmPromise> promise) override;
+  CdmContext* GetCdmContext() override;
+
+  // CdmContext implementations.
+  Decryptor* GetDecryptor() override;
+  int GetCdmId() const override;
+  void* GetClassIdentifier() const override;
+
+ private:
+  ~RemotingCdm() override;
+
+  const std::unique_ptr<RemotingCdmController> remoting_controller_;

[xhwang, 2016/11/01 08:21:28]

nit: will |remoting_cdm_controller_| be more explicit since we have other types
of remoting controller as well?

[xjz, 2016/11/01 21:55:52]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(RemotingCdm);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_REMOTING_REMOTING_CDM_H_