We must implement content/media/gmp/gmp-api/gmp-decryption.h, so that out of process Gecko Media Plugins can do decryption.

Comment on attachment 8460612 [details] [diff] [review] Patch v1 Review of attachment 8460612 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/gmp/GMPDecryptorChild.cpp @@ +6,5 @@ > +#include "GMPDecryptorChild.h" > +#include "GMPChild.h" > +#include "mozilla/TimeStamp.h" > +#include <ctime> > +#include "mozilla/unused.h" move <ctime> to after the first (main) include per normal guidelines ::: content/media/gmp/GMPDecryptorParent.cpp @@ +25,5 @@ > + > +void > +GMPDecryptorParent::Init(GMPDecryptorProxyCallback* aCallback) > +{ > + if (!mCanSendMessages) { You may want to mirror some of the changes/naming (mIsOpen, etc) from bug 1041232 @@ +131,5 @@ > + NS_WARNING("Trying to use an invalid GMP decrypter!"); > + return; > + } > + // Caller should ensure parameters passed in are valid. > + MOZ_ASSERT(!aBuffer.IsEmpty() && aCrypto.valid); runtime opt check, maybe? ::: content/media/gmp/GMPEncryptedBufferDataImpl.cpp @@ +26,5 @@ > + mClearBytes = aData.mClearBytes(); > + MOZ_ASSERT(mClearBytes.Elements() != aData.mClearBytes().Elements()); > + mCipherBytes = aData.mCipherBytes(); > + MOZ_ASSERT(mCipherBytes.Elements() != aData.mCipherBytes().Elements()); > + MOZ_ASSERT(mClearBytes.Length() == mCipherBytes.Length()); All these asserts are Debug-only. What if these are violated in opt/release builds? Should there be error kickouts, or should it use MOZ_CRASH for security safety ::: content/media/gmp/GMPEncryptedBufferDataImpl.h @@ +14,5 @@ > +namespace mozilla { > +namespace gmp { > + > +class GMPEncryptedBufferDataImpl : public GMPEncryptedBufferMetadata { > + typedef mp4_demuxer::CryptoSample CryptoSample; be explicit if this is private ::: content/media/gmp/GMPParent.cpp @@ +121,5 @@ > } > > + // Invalidate and remove any remaining API objects. > + for (uint32_t i = mDecryptors.Length(); i > 0; i--) { > + mDecryptors[i - 1]->DecryptingComplete(); I'll be renaming this to Shutdown() @@ +220,5 @@ > + } > + GMPDecryptorParent* vdp = static_cast<GMPDecryptorParent*>(pvdp); > + mDecryptors.AppendElement(vdp); > + *aGMPKS = vdp; > + Take care here about interface reference ala bug 1041232

(In reply to Randell Jesup [:jesup] from comment #2) > Comment on attachment 8460612 [details] [diff] [review] > @@ +131,5 @@ > > + NS_WARNING("Trying to use an invalid GMP decrypter!"); > > + return; > > + } > > + // Caller should ensure parameters passed in are valid. > > + MOZ_ASSERT(!aBuffer.IsEmpty() && aCrypto.valid); > > runtime opt check, maybe? If aBuffer is empty, we'll just send a 0-length sample, which shouldn't cause any problems. Basically we're asserting here that the demuxer isn't misbehaving. > ::: content/media/gmp/GMPEncryptedBufferDataImpl.cpp > @@ +26,5 @@ > > + mClearBytes = aData.mClearBytes(); > > + MOZ_ASSERT(mClearBytes.Elements() != aData.mClearBytes().Elements()); > > + mCipherBytes = aData.mCipherBytes(); > > + MOZ_ASSERT(mCipherBytes.Elements() != aData.mCipherBytes().Elements()); > > + MOZ_ASSERT(mClearBytes.Length() == mCipherBytes.Length()); > > All these asserts are Debug-only. What if these are violated in opt/release > builds? Should there be error kickouts, or should it use MOZ_CRASH for > security safety The (mClearBytes.Length() == mCipherBytes.Length()) assert should be guaranteed by the demuxer, so I don't think it needs to be fatal. But I will change GMPEncryptedBufferDataImpl::NumSubsamples() to return min(mClearBytes.Length(),mCipherBytes.Length()) to ensure there's no chance of array index out of bounds in opt builds. The other assert here... I actually don't think we need, as they're just asserting that the data was copied...

Landed roll up of both patches: https://hg.mozilla.org/integration/mozilla-inbound/rev/0616023bd207

Warnings as errors fix: https://hg.mozilla.org/integration/mozilla-inbound/rev/b8d1045bbb44

https://hg.mozilla.org/mozilla-central/rev/0616023bd207 https://hg.mozilla.org/mozilla-central/rev/b8d1045bbb44