Bug 1130917 - Part 2 - improve error handling of StoreData() and ReadData(). r=edwin. a=lmandel
authorJW Wang <jwwang@mozilla.com>
Tue, 10 Feb 2015 18:18:00 +0100
changeset 250208 d02a943f7351
parent 250207 7a78fefaf5bd
child 250209 acb510bddadd
push id4521
push usercpearce@mozilla.com
push date2015-03-04 01:22 +0000
treeherdermozilla-beta@8abdbdecd2d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin, lmandel
bugs1130917
milestone37.0
Bug 1130917 - Part 2 - improve error handling of StoreData() and ReadData(). r=edwin. a=lmandel
media/gmp-clearkey/0.1/ClearKeyStorage.cpp
--- a/media/gmp-clearkey/0.1/ClearKeyStorage.cpp
+++ b/media/gmp-clearkey/0.1/ClearKeyStorage.cpp
@@ -13,161 +13,170 @@
 #include <vector>
 
 static GMPErr
 RunOnMainThread(GMPTask* aTask)
 {
   return GetPlatform()->runonmainthread(aTask);
 }
 
+GMPErr
+OpenRecord(const char* aName,
+           uint32_t aNameLength,
+           GMPRecord** aOutRecord,
+           GMPRecordClient* aClient)
+{
+  return GetPlatform()->createrecord(aName, aNameLength, aOutRecord, aClient);
+}
+
 class WriteRecordClient : public GMPRecordClient {
 public:
-  GMPErr Init(GMPRecord* aRecord,
-              GMPTask* aOnSuccess,
-              GMPTask* aOnFailure,
-              const uint8_t* aData,
-              uint32_t aDataSize) {
-    mRecord = aRecord;
-    mOnSuccess = aOnSuccess;
-    mOnFailure = aOnFailure;
-    mData.insert(mData.end(), aData, aData + aDataSize);
-    return mRecord->Open();
+  /*
+   * This function will take the memory ownership of the parameters and
+   * delete them when done.
+   */
+  static void Write(const std::string& aRecordName,
+                    const std::vector<uint8_t>& aData,
+                    GMPTask* aOnSuccess,
+                    GMPTask* aOnFailure) {
+    (new WriteRecordClient(aData, aOnSuccess, aOnFailure))->Do(aRecordName);
   }
 
   virtual void OpenComplete(GMPErr aStatus) MOZ_OVERRIDE {
     if (GMP_FAILED(aStatus) ||
         GMP_FAILED(mRecord->Write(&mData.front(), mData.size()))) {
-      RunOnMainThread(mOnFailure);
-      mOnSuccess->Destroy();
+      Done(mOnFailure, mOnSuccess);
     }
   }
 
   virtual void ReadComplete(GMPErr aStatus,
                             const uint8_t* aData,
                             uint32_t aDataSize) MOZ_OVERRIDE {
     MOZ_ASSERT(false, "Should not reach here.");
   }
 
   virtual void WriteComplete(GMPErr aStatus) MOZ_OVERRIDE {
+    if (GMP_FAILED(aStatus)) {
+      Done(mOnFailure, mOnSuccess);
+    } else {
+      Done(mOnSuccess, mOnFailure);
+    }
+  }
+
+private:
+  WriteRecordClient(const std::vector<uint8_t>& aData,
+                    GMPTask* aOnSuccess,
+                    GMPTask* aOnFailure)
+    : mRecord(nullptr)
+    , mOnSuccess(aOnSuccess)
+    , mOnFailure(aOnFailure)
+    , mData(aData) {}
+
+  void Do(const std::string& aName) {
+    auto err = OpenRecord(aName.c_str(), aName.size(), &mRecord, this);
+    if (GMP_FAILED(err) ||
+        GMP_FAILED(mRecord->Open())) {
+      Done(mOnFailure, mOnSuccess);
+    }
+  }
+
+  void Done(GMPTask* aToRun, GMPTask* aToDestroy) {
     // Note: Call Close() before running continuation, in case the
     // continuation tries to open the same record; if we call Close()
     // after running the continuation, the Close() call will arrive
     // just after the Open() call succeeds, immediately closing the
     // record we just opened.
-    mRecord->Close();
-    if (GMP_SUCCEEDED(aStatus)) {
-      RunOnMainThread(mOnSuccess);
-      mOnFailure->Destroy();
-    } else {
-      RunOnMainThread(mOnFailure);
-      mOnSuccess->Destroy();
+    if (mRecord) {
+      mRecord->Close();
     }
+    aToDestroy->Destroy();
+    RunOnMainThread(aToRun);
     delete this;
   }
 
-private:
   GMPRecord* mRecord;
   GMPTask* mOnSuccess;
   GMPTask* mOnFailure;
-  std::vector<uint8_t> mData;
+  const std::vector<uint8_t> mData;
 };
 
-GMPErr
-OpenRecord(const char* aName,
-           uint32_t aNameLength,
-           GMPRecord** aOutRecord,
-           GMPRecordClient* aClient)
-{
-  return GetPlatform()->createrecord(aName, aNameLength, aOutRecord, aClient);
-}
-
 void
 StoreData(const std::string& aRecordName,
           const std::vector<uint8_t>& aData,
           GMPTask* aOnSuccess,
           GMPTask* aOnFailure)
 {
-  GMPRecord* record;
-  WriteRecordClient* client = new WriteRecordClient();
-  if (GMP_FAILED(OpenRecord(aRecordName.c_str(),
-                            aRecordName.size(),
-                            &record,
-                            client)) ||
-      GMP_FAILED(client->Init(record,
-                              aOnSuccess,
-                              aOnFailure,
-                              &aData.front(),
-                              aData.size()))) {
-    RunOnMainThread(aOnFailure);
-    aOnSuccess->Destroy();
-  }
+  WriteRecordClient::Write(aRecordName, aData, aOnSuccess, aOnFailure);
 }
 
 class ReadRecordClient : public GMPRecordClient {
 public:
-  ReadRecordClient()
-    : mRecord(nullptr)
-    , mContinuation(nullptr)
-  {}
-  ~ReadRecordClient() {
-    delete mContinuation;
-  }
-
-  GMPErr Init(GMPRecord* aRecord,
-              ReadContinuation* aContinuation) {
-    mRecord = aRecord;
-    mContinuation = aContinuation;
-    return mRecord->Open();
+  /*
+   * This function will take the memory ownership of the parameters and
+   * delete them when done.
+   */
+  static void Read(const std::string& aRecordName,
+                   ReadContinuation* aContinuation) {
+    MOZ_ASSERT(aContinuation);
+    (new ReadRecordClient(aContinuation))->Do(aRecordName);
   }
 
   virtual void OpenComplete(GMPErr aStatus) MOZ_OVERRIDE {
-    auto err = mRecord->Read();
-    if (GMP_FAILED(err)) {
-      mContinuation->ReadComplete(err, nullptr, 0);
-      delete this;
+    auto err = aStatus;
+    if (GMP_FAILED(err) ||
+        GMP_FAILED(err = mRecord->Read())) {
+      Done(err, nullptr, 0);
     }
   }
 
   virtual void ReadComplete(GMPErr aStatus,
                             const uint8_t* aData,
                             uint32_t aDataSize) MOZ_OVERRIDE {
-    // Note: Call Close() before running continuation, in case the
-    // continuation tries to open the same record; if we call Close()
-    // after running the continuation, the Close() call will arrive
-    // just after the Open() call succeeds, immediately closing the
-    // record we just opened.
-    mRecord->Close();
-    mContinuation->ReadComplete(GMPNoErr, aData, aDataSize);
-    delete this;
+    Done(aStatus, aData, aDataSize);
   }
 
   virtual void WriteComplete(GMPErr aStatus) MOZ_OVERRIDE {
     MOZ_ASSERT(false, "Should not reach here.");
   }
 
 private:
+  explicit ReadRecordClient(ReadContinuation* aContinuation)
+    : mRecord(nullptr)
+    , mContinuation(aContinuation) {}
+
+  void Do(const std::string& aName) {
+    auto err = OpenRecord(aName.c_str(), aName.size(), &mRecord, this);
+    if (GMP_FAILED(err) ||
+        GMP_FAILED(err = mRecord->Open())) {
+      Done(err, nullptr, 0);
+    }
+  }
+
+  void Done(GMPErr err, const uint8_t* aData, uint32_t aDataSize) {
+    // Note: Call Close() before running continuation, in case the
+    // continuation tries to open the same record; if we call Close()
+    // after running the continuation, the Close() call will arrive
+    // just after the Open() call succeeds, immediately closing the
+    // record we just opened.
+    if (mRecord) {
+      mRecord->Close();
+    }
+    mContinuation->ReadComplete(err, aData, aDataSize);
+    delete mContinuation;
+    delete this;
+  }
+
   GMPRecord* mRecord;
   ReadContinuation* mContinuation;
 };
 
 void
 ReadData(const std::string& aRecordName,
          ReadContinuation* aContinuation)
 {
-  MOZ_ASSERT(aContinuation);
-  GMPRecord* record;
-  ReadRecordClient* client = new ReadRecordClient();
-  auto err = OpenRecord(aRecordName.c_str(),
-                        aRecordName.size(),
-                        &record,
-                        client);
-  if (GMP_FAILED(err) ||
-      GMP_FAILED(client->Init(record, aContinuation))) {
-    aContinuation->ReadComplete(err, nullptr, 0);
-    delete aContinuation;
-  }
+  ReadRecordClient::Read(aRecordName, aContinuation);
 }
 
 GMPErr
 EnumRecordNames(RecvGMPRecordIteratorPtr aRecvIteratorFunc)
 {
   return GetPlatform()->getrecordenumerator(aRecvIteratorFunc, nullptr);
 }