Bug 1102607 - make GMPStorage APIs callable off the main thread. r=cpearce.
authorJW Wang <jwwang@mozilla.com>
Fri, 28 Nov 2014 00:34:00 +0100
changeset 218168 4e320993022046dde11245ebffd48dc49e5abb9e
parent 218167 69c0c04139a2b1de58b0d98d5c68eeef871dfbee
child 218169 34ef51eca3bb528cbe71da4b21e8758c9b28a9da
push id27919
push userphilringnalda@gmail.com
push dateTue, 02 Dec 2014 03:07:26 +0000
treeherdermozilla-central@265e01c7ff55 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1102607
milestone37.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1102607 - make GMPStorage APIs callable off the main thread. r=cpearce.
dom/media/gmp/GMPDecryptorChild.cpp
dom/media/gmp/GMPPlatform.cpp
dom/media/gmp/GMPStorageChild.cpp
dom/media/gmp/GMPStorageChild.h
--- a/dom/media/gmp/GMPDecryptorChild.cpp
+++ b/dom/media/gmp/GMPDecryptorChild.cpp
@@ -338,8 +338,12 @@ GMPDecryptorChild::RecvDecryptingComplet
 
   unused << Send__delete__(this);
 
   return true;
 }
 
 } // namespace gmp
 } // namespace mozilla
+
+// avoid redefined macro in unified build
+#undef ON_GMP_THREAD
+#undef CALL_ON_GMP_THREAD
--- a/dom/media/gmp/GMPPlatform.cpp
+++ b/dom/media/gmp/GMPPlatform.cpp
@@ -153,22 +153,16 @@ CreateMutex(GMPMutex** aMutex)
 }
 
 GMPErr
 CreateRecord(const char* aRecordName,
              uint32_t aRecordNameSize,
              GMPRecord** aOutRecord,
              GMPRecordClient* aClient)
 {
-  MOZ_ASSERT(IsOnChildMainThread());
-
-  if (sMainLoop != MessageLoop::current()) {
-    MOZ_ASSERT(false, "GMP called CreateRecord() on non-main thread!");
-    return GMPGenericErr;
-  }
   if (aRecordNameSize > GMP_MAX_RECORD_NAME_SIZE) {
     NS_WARNING("GMP tried to CreateRecord with too long record name");
     return GMPGenericErr;
   }
   GMPStorageChild* storage = sChild->GetGMPStorage();
   if (!storage) {
     return GMPGenericErr;
   }
@@ -195,20 +189,16 @@ GetClock(GMPTimestamp* aOutTime)
   *aOutTime = time(0) * 1000;
   return GMPNoErr;
 }
 
 GMPErr
 CreateRecordIterator(RecvGMPRecordIteratorPtr aRecvIteratorFunc,
                      void* aUserArg)
 {
-  if (sMainLoop != MessageLoop::current()) {
-    MOZ_ASSERT(false, "GMP called CreateRecord() on non-main thread!");
-    return GMPGenericErr;
-  }
   if (!aRecvIteratorFunc) {
     return GMPInvalidArgErr;
   }
   GMPStorageChild* storage = sChild->GetGMPStorage();
   if (!storage) {
     return GMPGenericErr;
   }
   MOZ_ASSERT(storage);
--- a/dom/media/gmp/GMPStorageChild.cpp
+++ b/dom/media/gmp/GMPStorageChild.cpp
@@ -2,294 +2,297 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "GMPStorageChild.h"
 #include "GMPChild.h"
 #include "gmp-storage.h"
 
+#define ON_GMP_THREAD() (mPlugin->GMPMessageLoop() == MessageLoop::current())
+
+#define CALL_ON_GMP_THREAD(_func, ...) \
+  do { \
+    if (ON_GMP_THREAD()) { \
+      _func(__VA_ARGS__); \
+    } else { \
+      mPlugin->GMPMessageLoop()->PostTask( \
+        FROM_HERE, NewRunnableMethod(this, &GMPStorageChild::_func, ##__VA_ARGS__) \
+      ); \
+    } \
+  } while(false)
+
+static nsTArray<uint8_t>
+ToArray(const uint8_t* aData, uint32_t aDataSize)
+{
+  nsTArray<uint8_t> data;
+  data.AppendElements(aData, aDataSize);
+  return mozilla::Move(data);
+}
+
 namespace mozilla {
 namespace gmp {
 
 GMPRecordImpl::GMPRecordImpl(GMPStorageChild* aOwner,
                              const nsCString& aName,
                              GMPRecordClient* aClient)
   : mName(aName)
   , mClient(aClient)
   , mOwner(aOwner)
-  , mIsClosed(true)
 {
 }
 
 GMPErr
 GMPRecordImpl::Open()
 {
-  if (!mIsClosed) {
-    return GMPRecordInUse;
-  }
   return mOwner->Open(this);
 }
 
 void
 GMPRecordImpl::OpenComplete(GMPErr aStatus)
 {
-  mIsClosed = false;
   mClient->OpenComplete(aStatus);
 }
 
 GMPErr
 GMPRecordImpl::Read()
 {
-  if (mIsClosed) {
-    return GMPClosedErr;
-  }
   return mOwner->Read(this);
 }
 
 void
 GMPRecordImpl::ReadComplete(GMPErr aStatus,
                             const uint8_t* aBytes,
                             uint32_t aLength)
 {
   mClient->ReadComplete(aStatus, aBytes, aLength);
 }
 
 GMPErr
 GMPRecordImpl::Write(const uint8_t* aData, uint32_t aDataSize)
 {
-  if (mIsClosed) {
-    return GMPClosedErr;
-  }
   return mOwner->Write(this, aData, aDataSize);
 }
 
 void
 GMPRecordImpl::WriteComplete(GMPErr aStatus)
 {
   mClient->WriteComplete(aStatus);
 }
 
 GMPErr
 GMPRecordImpl::Close()
 {
   nsRefPtr<GMPRecordImpl> kungfuDeathGrip(this);
-
-  if (!mIsClosed) {
-    // Delete the storage child's reference to us.
-    mOwner->Close(this);
-    // Owner should callback MarkClosed().
-    MOZ_ASSERT(mIsClosed);
-  }
-
   // Delete our self reference.
   Release();
-
+  mOwner->Close(this->Name());
   return GMPNoErr;
 }
 
-void
-GMPRecordImpl::MarkClosed()
-{
-  mIsClosed = true;
-}
-
 GMPStorageChild::GMPStorageChild(GMPChild* aPlugin)
-  : mPlugin(aPlugin)
+  : mMonitor("GMPStorageChild")
+  , mPlugin(aPlugin)
   , mShutdown(false)
 {
-  MOZ_ASSERT(mPlugin->GMPMessageLoop() == MessageLoop::current());
+  MOZ_ASSERT(ON_GMP_THREAD());
 }
 
 GMPErr
 GMPStorageChild::CreateRecord(const nsCString& aRecordName,
                               GMPRecord** aOutRecord,
                               GMPRecordClient* aClient)
 {
-  if (mPlugin->GMPMessageLoop() != MessageLoop::current()) {
-    NS_WARNING("GMP used GMPStorage on non-main thread.");
-    return GMPGenericErr;
-  }
+  MonitorAutoLock lock(mMonitor);
+
   if (mShutdown) {
     NS_WARNING("GMPStorage used after it's been shutdown!");
     return GMPClosedErr;
   }
+
   MOZ_ASSERT(aRecordName.Length() && aOutRecord);
   nsRefPtr<GMPRecordImpl> record(new GMPRecordImpl(this, aRecordName, aClient));
   mRecords.Put(aRecordName, record); // Addrefs
 
   // The GMPRecord holds a self reference until the GMP calls Close() on
   // it. This means the object is always valid (even if neutered) while
   // the GMP expects it to be.
   record.forget(aOutRecord);
 
   return GMPNoErr;
 }
 
+bool
+GMPStorageChild::HasRecord(const nsCString& aRecordName)
+{
+  mMonitor.AssertCurrentThreadOwns();
+  return mRecords.Contains(aRecordName);
+}
+
+already_AddRefed<GMPRecordImpl>
+GMPStorageChild::GetRecord(const nsCString& aRecordName)
+{
+  MonitorAutoLock lock(mMonitor);
+  nsRefPtr<GMPRecordImpl> record;
+  mRecords.Get(aRecordName, getter_AddRefs(record));
+  return record.forget();
+}
+
 GMPErr
 GMPStorageChild::Open(GMPRecordImpl* aRecord)
 {
-  if (mPlugin->GMPMessageLoop() != MessageLoop::current()) {
-    NS_WARNING("GMP used GMPStorage on non-main thread.");
-    return GMPGenericErr;
-  }
+  MonitorAutoLock lock(mMonitor);
+
   if (mShutdown) {
     NS_WARNING("GMPStorage used after it's been shutdown!");
     return GMPClosedErr;
   }
-  if (!SendOpen(aRecord->Name())) {
-    Close(aRecord);
+
+  if (!HasRecord(aRecord->Name())) {
+    // Trying to re-open a record that has already been closed.
     return GMPClosedErr;
   }
+
+  CALL_ON_GMP_THREAD(SendOpen, aRecord->Name());
+
   return GMPNoErr;
 }
 
 GMPErr
 GMPStorageChild::Read(GMPRecordImpl* aRecord)
 {
-  if (mPlugin->GMPMessageLoop() != MessageLoop::current()) {
-    NS_WARNING("GMP used GMPStorage on non-main thread.");
-    return GMPGenericErr;
-  }
+  MonitorAutoLock lock(mMonitor);
+
   if (mShutdown) {
     NS_WARNING("GMPStorage used after it's been shutdown!");
     return GMPClosedErr;
   }
-  if (!SendRead(aRecord->Name())) {
-    Close(aRecord);
+
+  if (!HasRecord(aRecord->Name())) {
+    // Record not opened.
     return GMPClosedErr;
   }
+
+  CALL_ON_GMP_THREAD(SendRead, aRecord->Name());
+
   return GMPNoErr;
 }
 
 GMPErr
 GMPStorageChild::Write(GMPRecordImpl* aRecord,
                        const uint8_t* aData,
                        uint32_t aDataSize)
 {
-  if (mPlugin->GMPMessageLoop() != MessageLoop::current()) {
-    NS_WARNING("GMP used GMPStorage on non-main thread.");
-    return GMPGenericErr;
+  if (aDataSize > GMP_MAX_RECORD_SIZE) {
+    return GMPQuotaExceededErr;
   }
+
+  MonitorAutoLock lock(mMonitor);
+
   if (mShutdown) {
     NS_WARNING("GMPStorage used after it's been shutdown!");
     return GMPClosedErr;
   }
-  if (aDataSize > GMP_MAX_RECORD_SIZE) {
-    return GMPQuotaExceededErr;
-  }
-  nsTArray<uint8_t> data;
-  data.AppendElements(aData, aDataSize);
-  if (!SendWrite(aRecord->Name(), data)) {
-    Close(aRecord);
+
+  if (!HasRecord(aRecord->Name())) {
+    // Record not opened.
     return GMPClosedErr;
   }
+
+  CALL_ON_GMP_THREAD(SendWrite, aRecord->Name(), ToArray(aData, aDataSize));
+
   return GMPNoErr;
 }
 
 GMPErr
-GMPStorageChild::Close(GMPRecordImpl* aRecord)
+GMPStorageChild::Close(const nsCString& aRecordName)
 {
-  if (mPlugin->GMPMessageLoop() != MessageLoop::current()) {
-    NS_WARNING("GMP used GMPStorage on non-main thread.");
-    return GMPGenericErr;
-  }
-  if (!mRecords.Contains(aRecord->Name())) {
+  MonitorAutoLock lock(mMonitor);
+
+  if (!HasRecord(aRecordName)) {
     // Already closed.
     return GMPClosedErr;
   }
 
-  GMPErr rv = GMPNoErr;
-  if (!mShutdown && !SendClose(aRecord->Name())) {
-    rv = GMPGenericErr;
+  mRecords.Remove(aRecordName);
+
+  if (!mShutdown) {
+    CALL_ON_GMP_THREAD(SendClose, aRecordName);
   }
 
-  aRecord->MarkClosed();
-  mRecords.Remove(aRecord->Name());
-
-  return rv;
+  return GMPNoErr;
 }
 
 bool
 GMPStorageChild::RecvOpenComplete(const nsCString& aRecordName,
                                   const GMPErr& aStatus)
 {
+  // We don't need a lock to read |mShutdown| since it is only changed in
+  // the GMP thread.
   if (mShutdown) {
     return true;
   }
-  nsRefPtr<GMPRecordImpl> record;
-  if (!mRecords.Get(aRecordName, getter_AddRefs(record)) || !record) {
+  nsRefPtr<GMPRecordImpl> record = GetRecord(aRecordName);
+  if (!record) {
     // Not fatal.
     return true;
   }
   record->OpenComplete(aStatus);
-  if (GMP_FAILED(aStatus)) {
-    Close(record);
-  }
   return true;
 }
 
 bool
 GMPStorageChild::RecvReadComplete(const nsCString& aRecordName,
                                   const GMPErr& aStatus,
                                   const InfallibleTArray<uint8_t>& aBytes)
 {
   if (mShutdown) {
     return true;
   }
-  nsRefPtr<GMPRecordImpl> record;
-  if (!mRecords.Get(aRecordName, getter_AddRefs(record)) || !record) {
+  nsRefPtr<GMPRecordImpl> record = GetRecord(aRecordName);
+  if (!record) {
     // Not fatal.
     return true;
   }
-  record->ReadComplete(aStatus,
-                       aBytes.Elements(),
-                       aBytes.Length());
-  if (GMP_FAILED(aStatus)) {
-    Close(record);
-  }
+  record->ReadComplete(aStatus, aBytes.Elements(), aBytes.Length());
   return true;
 }
 
 bool
 GMPStorageChild::RecvWriteComplete(const nsCString& aRecordName,
                                    const GMPErr& aStatus)
 {
   if (mShutdown) {
     return true;
   }
-  nsRefPtr<GMPRecordImpl> record;
-  if (!mRecords.Get(aRecordName, getter_AddRefs(record)) || !record) {
+  nsRefPtr<GMPRecordImpl> record = GetRecord(aRecordName);
+  if (!record) {
     // Not fatal.
     return true;
   }
   record->WriteComplete(aStatus);
-  if (GMP_FAILED(aStatus)) {
-    Close(record);
-  }
   return true;
 }
 
 GMPErr
 GMPStorageChild::EnumerateRecords(RecvGMPRecordIteratorPtr aRecvIteratorFunc,
                                   void* aUserArg)
 {
-  if (mPlugin->GMPMessageLoop() != MessageLoop::current()) {
-    MOZ_ASSERT(false, "GMP used GMPStorage on non-main thread.");
-    return GMPGenericErr;
-  }
+  MonitorAutoLock lock(mMonitor);
+
   if (mShutdown) {
     NS_WARNING("GMPStorage used after it's been shutdown!");
     return GMPClosedErr;
   }
-  if (!SendGetRecordNames()) {
-    return GMPGenericErr;
-  }
+
   MOZ_ASSERT(aRecvIteratorFunc);
   mPendingRecordIterators.push(RecordIteratorContext(aRecvIteratorFunc, aUserArg));
+
+  CALL_ON_GMP_THREAD(SendGetRecordNames);
+
   return GMPNoErr;
 }
 
 class GMPRecordIteratorImpl : public GMPRecordIterator {
 public:
   GMPRecordIteratorImpl(const InfallibleTArray<nsCString>& aRecordNames)
     : mRecordNames(aRecordNames)
     , mIndex(0)
@@ -325,37 +328,47 @@ private:
   nsTArray<nsCString> mRecordNames;
   size_t mIndex;
 };
 
 bool
 GMPStorageChild::RecvRecordNames(const InfallibleTArray<nsCString>& aRecordNames,
                                  const GMPErr& aStatus)
 {
-  if (mShutdown || mPendingRecordIterators.empty()) {
-    return true;
+  RecordIteratorContext ctx;
+  {
+    MonitorAutoLock lock(mMonitor);
+    if (mShutdown || mPendingRecordIterators.empty()) {
+      return true;
+    }
+    ctx = mPendingRecordIterators.front();
+    mPendingRecordIterators.pop();
   }
-  RecordIteratorContext ctx = mPendingRecordIterators.front();
-  mPendingRecordIterators.pop();
 
   if (GMP_FAILED(aStatus)) {
     ctx.mFunc(nullptr, ctx.mUserArg, aStatus);
   } else {
     ctx.mFunc(new GMPRecordIteratorImpl(aRecordNames), ctx.mUserArg, GMPNoErr);
   }
+
   return true;
 }
 
 bool
 GMPStorageChild::RecvShutdown()
 {
   // Block any new storage requests, and thus any messages back to the
   // parent. We don't delete any objects here, as that may invalidate
   // GMPRecord pointers held by the GMP.
+  MonitorAutoLock lock(mMonitor);
   mShutdown = true;
   while (!mPendingRecordIterators.empty()) {
     mPendingRecordIterators.pop();
   }
   return true;
 }
 
 } // namespace gmp
 } // namespace mozilla
+
+// avoid redefined macro in unified build
+#undef ON_GMP_THREAD
+#undef CALL_ON_GMP_THREAD
--- a/dom/media/gmp/GMPStorageChild.h
+++ b/dom/media/gmp/GMPStorageChild.h
@@ -37,24 +37,21 @@ public:
   virtual GMPErr Close() MOZ_OVERRIDE;
 
   const nsCString& Name() const { return mName; }
 
   void OpenComplete(GMPErr aStatus);
   void ReadComplete(GMPErr aStatus, const uint8_t* aBytes, uint32_t aLength);
   void WriteComplete(GMPErr aStatus);
 
-  void MarkClosed();
-
 private:
   ~GMPRecordImpl() {}
   const nsCString mName;
   GMPRecordClient* const mClient;
   GMPStorageChild* const mOwner;
-  bool mIsClosed;
 };
 
 class GMPStorageChild : public PGMPStorageChild
 {
 public:
   NS_INLINE_DECL_REFCOUNTING(GMPStorageChild)
 
   explicit GMPStorageChild(GMPChild* aPlugin);
@@ -66,46 +63,52 @@ public:
   GMPErr Open(GMPRecordImpl* aRecord);
 
   GMPErr Read(GMPRecordImpl* aRecord);
 
   GMPErr Write(GMPRecordImpl* aRecord,
                const uint8_t* aData,
                uint32_t aDataSize);
 
-  GMPErr Close(GMPRecordImpl* aRecord);
+  GMPErr Close(const nsCString& aRecordName);
 
   GMPErr EnumerateRecords(RecvGMPRecordIteratorPtr aRecvIteratorFunc,
                           void* aUserArg);
 
+private:
+  bool HasRecord(const nsCString& aRecordName);
+  already_AddRefed<GMPRecordImpl> GetRecord(const nsCString& aRecordName);
+
 protected:
   ~GMPStorageChild() {}
 
   // PGMPStorageChild
   virtual bool RecvOpenComplete(const nsCString& aRecordName,
                                 const GMPErr& aStatus) MOZ_OVERRIDE;
   virtual bool RecvReadComplete(const nsCString& aRecordName,
                                 const GMPErr& aStatus,
                                 const InfallibleTArray<uint8_t>& aBytes) MOZ_OVERRIDE;
   virtual bool RecvWriteComplete(const nsCString& aRecordName,
                                  const GMPErr& aStatus) MOZ_OVERRIDE;
   virtual bool RecvRecordNames(const InfallibleTArray<nsCString>& aRecordNames,
                                const GMPErr& aStatus) MOZ_OVERRIDE;
   virtual bool RecvShutdown() MOZ_OVERRIDE;
 
 private:
+  Monitor mMonitor;
   nsRefPtrHashtable<nsCStringHashKey, GMPRecordImpl> mRecords;
   GMPChild* mPlugin;
 
   struct RecordIteratorContext {
     explicit RecordIteratorContext(RecvGMPRecordIteratorPtr aFunc,
                                    void* aUserArg)
       : mFunc(aFunc)
       , mUserArg(aUserArg)
     {}
+    RecordIteratorContext() {}
     RecvGMPRecordIteratorPtr mFunc;
     void* mUserArg;
   };
 
   std::queue<RecordIteratorContext> mPendingRecordIterators;
   bool mShutdown;
 };