media: Add MediaDrmBridgeCdmContext.

media/base/android/media_drm_bridge_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_BASE_ANDROID_MEDIA_DRM_BRIDGE_CDM_CONTEXT_H_
+#define MEDIA_BASE_ANDROID_MEDIA_DRM_BRIDGE_CDM_CONTEXT_H_
+
+#include <jni.h>
+
+#include "base/android/scoped_java_ref.h"
+#include "base/callback.h"
+#include "base/macros.h"
+#include "media/base/cdm_context.h"
+#include "media/base/media_export.h"
+#include "media/base/player_tracker.h"
+
+namespace media {
+
+class MediaDrmBridge;
+
+// Methods can be called on any thread. The registered callbacks will be fired

[ddorwin, 2016/03/18 18:27:14]

Describe the purpose of this class.

Specifically, it's weird that the CdmContext interface only returns invalid
values.

I worry about other pieces of code using this or calling GetCdmContext on
MediaDrmBridge expecting this to be a real CdmContext.

To address the former, perhaps this should be a nested class. I guess the name
implies that already, but scoping might be a bit better.

[xhwang, 2016/03/18 22:10:46]

I updated the CdmContext documentation to make this clear.
 
I like a separate file better, so that the media player/decoder doesn't need to
know about MediaDrmBridge at all, another layer of separation. But I don't have
strong opinions on this :)

+// on the thread |media_drm_bridge_| is running on. The caller should make sure
+// that the callbacks are posted to the correct thread.
+// TODO(xhwang): Remove PlayerTracker interface.
+class MEDIA_EXPORT MediaDrmBridgeCdmContext : public CdmContext,
+                                              public PlayerTracker {
+ public:
+  using JavaObjectPtr = scoped_ptr<base::android::ScopedJavaGlobalRef<jobject>>;
+
+  // Notification called when MediaCrypto object is ready.
+  // Parameters:
+  // |media_crypto| - global reference to MediaCrypto object

[ddorwin, 2016/03/18 18:27:14]

What does global reference mean?

[xhwang, 2016/03/18 22:10:46]

I think this is because the JavaObjectPtr is a ScopedJavaGlobalRef. But it seems
we don't need it in this comment. Updated.

+  // |needs_protected_surface| - true if protected surface is required.
+  using MediaCryptoReadyCB = base::Callback<void(JavaObjectPtr media_crypto,
+                                                 bool needs_protected_surface)>;
+
+  // The |media_drm_bridge| owns |this| is guaranteed to outlive |this|.

[ddorwin, 2016/03/18 18:27:14]

There seems to be a word missing. Perhaps either "that owns" or "and is."

[xhwang, 2016/03/18 22:10:46]

Done.

+  explicit MediaDrmBridgeCdmContext(MediaDrmBridge* media_drm_bridge);
+
+  ~MediaDrmBridgeCdmContext() final;
+
+  // CdmContext implementation.
+  Decryptor* GetDecryptor() final;
+  int GetCdmId() const final;
+
+  // PlayerTracker implementation. Can be called on any thread.

[ddorwin, 2016/03/18 18:27:14]

Nit: newline after the first sentence. Such lines are usually separate to stand
out.

[xhwang, 2016/03/18 22:10:46]

Done.

+  // The registered callbacks will be fired on |task_runner_|. The caller
+  // should make sure that the callbacks are posted to the correct thread.
+  //
+  // Note: RegisterPlayer() should be called before SetMediaCryptoReadyCB() to

[ddorwin, 2016/03/18 18:27:14]

s/should/must/?

[xhwang, 2016/03/18 22:10:46]

Done.

+  // avoid missing any new key notifications.
+  int RegisterPlayer(const base::Closure& new_key_cb,
+                     const base::Closure& cdm_unset_cb) final;
+  void UnregisterPlayer(int registration_id) final;
+
+  void SetMediaCryptoReadyCB(const MediaCryptoReadyCB& media_crypto_ready_cb);
+
+ private:
+  MediaDrmBridge* const media_drm_bridge_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaDrmBridgeCdmContext);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_ANDROID_MEDIA_DRM_BRIDGE_CDM_CONTEXT_H_