Separate MediaKeys interface from Decryptor interface.

webkit/media/crypto/proxy_decryptor.h

 // Copyright (c) 2012 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 WEBKIT_MEDIA_CRYPTO_PROXY_DECRYPTOR_H_
 #define WEBKIT_MEDIA_CRYPTO_PROXY_DECRYPTOR_H_
 
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/synchronization/lock.h"
 #include "media/base/decryptor.h"
+#include "media/base/media_keys.h"
 
 namespace WebKit {
 class WebFrame;
 class WebMediaPlayerClient;
 }
 
 namespace webkit_media {
 
+class PpapiDecryptor;
+
 // A decryptor proxy that creates a real decryptor object on demand and
 // forwards decryptor calls to it.
 // TODO(xhwang): Currently we don't support run-time switching among decryptor
 // objects. Fix this when needed.
-class ProxyDecryptor {
+class ProxyDecryptor : public media::MediaKeys {

[ddorwin, 2013/05/24 20:39:12]

We should probably rename this class in a future CL.

[xhwang, 2013/05/24 23:15:56]

Any idea on the new name? How about MediaKeysManager or DecryptorManager?

[ddorwin, 2013/05/28 19:44:33]

I'm not sure. It still hides the separation of Decryptor/MediaKeys from WMPI.
Maybe it will fade away with the new APIs.

At the very least, it's not a Decryptor - it contains one. DecryptorProxy or
DecryptorAdapter would be a big improvement. I don't think it's worth changing
at this point since we don't know what it should be. However, you might add a
TODO here that notes we need to change the name or delete this.

[xhwang, 2013/05/28 23:50:39]

Added TODO.

  public:
   ProxyDecryptor(WebKit::WebMediaPlayerClient* web_media_player_client,
                  WebKit::WebFrame* web_frame,
                  const media::KeyAddedCB& key_added_cb,
                  const media::KeyErrorCB& key_error_cb,
                  const media::KeyMessageCB& key_message_cb,
                  const media::NeedKeyCB& need_key_cb);
   virtual ~ProxyDecryptor();
 
   // Requests the ProxyDecryptor to notify the decryptor when it's ready through
   // the |decryptor_ready_cb| provided.
   // If |decryptor_ready_cb| is null, the existing callback will be fired with
   // NULL immediately and reset.
   void SetDecryptorReadyCB(const media::DecryptorReadyCB& decryptor_ready_cb);
 
-  bool GenerateKeyRequest(const std::string& key_system,
-                          const std::string& type,
-                          const uint8* init_data, int init_data_length);
-  void AddKey(const std::string& key_system,
-              const uint8* key, int key_length,
-              const uint8* init_data, int init_data_length,
-              const std::string& session_id);
-  void CancelKeyRequest(const std::string& key_system,
-                        const std::string& session_id);
+  // MediaKeys implementation.
+  virtual bool GenerateKeyRequest(const std::string& key_system,
+                                  const std::string& type,
+                                  const uint8* init_data,
+                                  int init_data_length) OVERRIDE;
+  virtual void AddKey(const std::string& key_system,
+                      const uint8* key, int key_length,
+                      const uint8* init_data, int init_data_length,
+                      const std::string& session_id) OVERRIDE;
+  virtual void CancelKeyRequest(const std::string& key_system,
+                                const std::string& session_id) OVERRIDE;
 
  private:
   // Helper functions to create decryptors to handle the given |key_system|.
 #if defined(ENABLE_PEPPER_CDMS)
-  scoped_ptr<media::Decryptor> CreatePpapiDecryptor(
+  scoped_ptr<PpapiDecryptor> CreatePpapiDecryptor(
       const std::string& key_system);
 #endif  // defined(ENABLE_PEPPER_CDMS)
-  scoped_ptr<media::Decryptor> CreateDecryptor(const std::string& key_system);
+  bool CreateDecryptor(const std::string& key_system);
 
   // Callbacks for firing key events.
   void KeyAdded(const std::string& key_system, const std::string& session_id);
   void KeyError(const std::string& key_system,
                 const std::string& session_id,
-                media::Decryptor::KeyError error_code,
+                media::MediaKeys::KeyError error_code,
                 int system_code);
   void KeyMessage(const std::string& key_system,
                   const std::string& session_id,
                   const std::string& message,
                   const std::string& default_url);
   void NeedKey(const std::string& key_system,
                const std::string& session_id,
                const std::string& type,
                scoped_ptr<uint8[]> init_data, int init_data_size);
 
   // Needed to create the PpapiDecryptor.
   WebKit::WebMediaPlayerClient* web_media_player_client_;
   WebKit::WebFrame* web_frame_;
   bool did_create_helper_plugin_;
 
   // Callbacks for firing key events.
   media::KeyAddedCB key_added_cb_;
   media::KeyErrorCB key_error_cb_;
   media::KeyMessageCB key_message_cb_;
   media::NeedKeyCB need_key_cb_;
 
   // Protects the |decryptor_|. Note that |decryptor_| itself should be thread
   // safe as per the Decryptor interface.
   base::Lock lock_;
 
   media::DecryptorReadyCB decryptor_ready_cb_;
 
-  // The real decryptor that does decryption for the ProxyDecryptor.
-  // This pointer is protected by the |lock_|.
-  scoped_ptr<media::Decryptor> decryptor_;
+  // The real MediaKeys and Decryptor objects that perform key operations and
+  // decryption for the ProxyDecryptor. These two pointers refer to the same
+  // object and |media_keys_| takes the ownership. They are protected by the
+  // |lock_|.
+  scoped_ptr<media::MediaKeys> media_keys_;
+  media::Decryptor* decryptor_;

[xhwang, 2013/05/24 00:25:12]

I like keeping MediaKeys and Decryptor separate. But this is the ugly part of
this CL that I don't like.

I am thinking: In desktop chromium's implementation the real Decryptor
(AesDecryptor and PpapiDecryptor) always implements both MediaKeys and Decryptor
interface. So maybe Decryptor interface could also inherit MediaKeys interface.
Then we don't have this problem (two pointers for the same object).

WDYT?

[ddorwin, 2013/05/24 20:39:12]

The inheritance would be for convenience and not really any real relationship.
What about exposing MediaKeys from the Decryptor? (I really like the idea of a
Decryptor(MediaKeys*) constructor, but that may not actually be practical. This
is close.

See my comment in the .cc file, which I think makes an argument for exposing it
in this way.

[xhwang, 2013/05/24 23:15:56]

Done.

 
   base::WeakPtrFactory<ProxyDecryptor> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ProxyDecryptor);
 };
 
 }  // namespace webkit_media
 
 #endif  // WEBKIT_MEDIA_CRYPTO_PROXY_DECRYPTOR_H_