Add CDM allocator interface.

webkit/media/crypto/ppapi/cdm_allocator.h

Index: webkit/media/crypto/ppapi/cdm_allocator.h
diff --git a/webkit/media/crypto/ppapi/cdm_allocator.h b/webkit/media/crypto/ppapi/cdm_allocator.h
new file mode 100644
index 0000000000000000000000000000000000000000..61ab9a346d3a79fb445f473aa0a3b823c0535b28
--- /dev/null
+++ b/webkit/media/crypto/ppapi/cdm_allocator.h
@@ -0,0 +1,29 @@
+// 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_PPAPI_CDM_ALLOCATOR_H_
+#define WEBKIT_MEDIA_CRYPTO_PPAPI_CDM_ALLOCATOR_H_
+
+#include "ppapi/c/pp_stdint.h"
+
+namespace webkit_media {
+
+// Interface class intended to hide the PPAPI memory allocation implementation
+// details from CDMs (Content Decryption Modules). The class implementing the
+// interface handles allocation. The data will be freed when the last reference
+// to the shared memory is released (typically after the browser has received
+// data from the CDM wrapper).
+class CdmAllocatorInterface {

[xhwang, 2012/09/01 13:49:09]

We don't usually use "Interface" suffix for interface base classes. Probably
just use "CdmAllocator"? 

If I understand correctly (ddorwin: please correct me if I am wrong), this class
should be a generic allocator class for the CDM interface. It should not care
about how the allocator is implemented. The real allocator can use
PPB_Buffer_Dev to leverage the shared memory, or it can be as simple as
new/delete, malloc/release. So IMHO, this class should not depend on ppapi. If
this sounds good to you, please remove PPAPI related contents in the class
comment. Also, change the #include to use stdint.h (and add typedef for windows,
see the header of CDM.h).

[Tom Finegan, 2012/09/01 21:32:49]

Done, using CdmAllocator.
I think I get the underlying point here, but I think it's more complicated than
that:

1) I think there has to be at least limited information provided re the fact
that this allocator provides shared memory that is sent to the browser. I don't
think that the implementation of this interface could ever use new/delete (or
malloc/free) because the memory provided is owned by the ppapi resource tracker,
and allocated using the PPB_Buffer_Dev methods, and...

2) If/when at some point we change to use ArrayBuffers, the same will remain
true. This class will be using some PPB interface to allocate memory that
belongs to the var tracker.

Re my first point, maybe in a testing/mock situation it could use std memory
routines. I don't think that potential situation is worth hiding this
information from users/implementors of the interface.

Anyway-- I'm not certain that my reasoning is correct. ddorwin, dmichael?
Done.

[ddorwin, 2012/09/04 09:53:15]

This class should know nothing about PPAPi or how memory is allocated. This
seems to be the case except for comments.

+ public:
+  CdmAllocatorInterface() {}
+  virtual ~CdmAllocatorInterface() {}
+
+  // Returns a pointer to shared memory, or NULL on failure.
+  // Note: The memory returned is already mapped.

[xhwang, 2012/09/01 13:49:09]

ditto, remove implementation details, e.g. memoey is mapped..

[Tom Finegan, 2012/09/01 21:32:49]

Deferring this until open question above is resolved.

[ddorwin, 2012/09/04 09:53:15]

Agreed, the interface should not have implementation details. The only real
requirement is that the buffer is valid when returned. See comment in Patch Set
3 - it may help with this.

+  virtual uint8_t* Allocate(int32_t size, int32_t* id) = 0;

[xhwang, 2012/09/01 13:49:09]

Return void* ? We may not need to do that since we always uses uint8_t in the
CDM interface.

In theory, |id| is just some opaque data, but |id| sounds ok here and I don't
have a better name.

[ddorwin, 2012/09/04 09:53:15]

See comment in Patch Set 3 - it may help with this.

+};

[xhwang, 2012/09/01 13:49:09]

We also need to add a void Deallocate(uint8_t*, int32_t id) function.

[Tom Finegan, 2012/09/01 21:32:49]

I don't think this interface should have a public deallocation method. The
ownership of the memory is only temporary, and we don't want CDMs trying to free
it after it's been sent back to the browser.

Instead, CdmWrapper could keep a list of resource IDs that have not yet been
sent to the browser, and:

1) When sending an ID to the browser, remove it from the list.
2) In the dtor, release allocated resources that have not been sent to the
browser.

This would mean keeping some information around, and remembering to handle house
keeping in the Deliver* methods, but it might not be necessary. My understanding
is that if the plugin is destroyed, resources it owns will be cleaned up.
dmichael can clarify for us.

[ddorwin, 2012/09/04 09:53:15]

See comment in Patch Set 3 - If you have an object, you can encapsulate
destruction there.
I don't know whether we need to track. If you don't, you _could_ leak buffers,
but that shouldn't happen. Maybe we need to delete them all on a flush or
something.

+
+}  // namespace webkit_media;
+
+#endif  // WEBKIT_MEDIA_CRYPTO_PPAPI_CDM_ALLOCATOR_H_