Media Remoting Clean-up: Less-redundant naming, style consistency, etc.

media/remoting/remoting_cdm_context.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_CONTEXT_H_
 #define MEDIA_REMOTING_REMOTING_CDM_CONTEXT_H_
 
 #include "media/base/cdm_context.h"
 
 namespace media {
+namespace remoting {
 
 class RemotingCdm;
-class RemotingSourceImpl;
 
 // TODO(xjz): Merge this with erickung's implementation.
+// TODO(miu): This class should just be merged into RemotingCdm and implement
+// both the CDM and CdmContext interfaces.

[xjz, 2017/01/21 06:12:34]

This is required by media owner. The following is xhwang@'s comment on my
original CL: "The purpose that we have CdmContext is to isolate the media player
(e.g. renderer) from accessing
the CDM directly. Therefore, it's more desirable that we cast CdmContext to a
RemotingCdmContext here, and get GetRemotingSource() there, instead of getting
the CDM from the context, which defies the purpose of CdmContext."

[miu, 2017/01/23 20:57:36]

Interesting point. I looked into this more, and I think we can still merge the
classes *if* we get rid of the cdm() (or GetRemotingSource()) accessor. Instead
of the accessor, we could provide a new static function in SharedSession:

  // If CdmContext represents a remoting implementation, return a pointer to the
SharedSession it
  // is using. Otherwise, return nullptr.
  SharedSession* GetForCdmContext(CdmContext* context);

Then, in the controller (or renderer), there is no direct pointer to the Cdm.
All they would have is a CdmContext* (and no way to downcast it to the merged
impl class).

I think that would satisfy all requirements, and allow us to get rid of this
extra class that is only owned by RemotingCdm and points back to it.

WDYT? (For a later CL; not this one.)

[xjz, 2017/01/23 23:08:13]

sgtm.

 class RemotingCdmContext : public CdmContext {
  public:
   explicit RemotingCdmContext(RemotingCdm* remoting_cdm);
   ~RemotingCdmContext() override;
 
   // If |cdm_context| is an instance of RemotingCdmContext, return a type-casted
   // pointer to it. Otherwise, return nullptr.
   static RemotingCdmContext* From(CdmContext* cdm_context);
 
-  RemotingSourceImpl* GetRemotingSource();
+  RemotingCdm* cdm() const { return remoting_cdm_; }

[xjz, 2017/01/21 06:12:34]

This accessor can access the CDM directly. I am OK with this, but it seems
doesn't follow the purpose of CdmContext as commented above. Since the only
thing we want from CDM is the RemotingSource, it sounds good to just have a
GetSession() function. WDYT?

[miu, 2017/01/23 20:57:37]

For now, I put this back to a SharedSession getter. (See comment above for other
details.)

 
   // CdmContext implementations.
   Decryptor* GetDecryptor() override;
   int GetCdmId() const override;
   void* GetClassIdentifier() const override;
 
  private:
   RemotingCdm* const remoting_cdm_;  // Outlives this class.
 
   DISALLOW_COPY_AND_ASSIGN(RemotingCdmContext);
 };
 
+}  // namespace remoting
 }  // namespace media
 
 #endif  // MEDIA_REMOTING_REMOTING_CDM_CONTEXT_H_