Bug 1186750 part 5 - Convert nsDOMDeviceStorage::CheckPermission to take already_AddRefed&&. r=dhylands
authorXidorn Quan <quanxunzhen@gmail.com>
Fri, 20 Nov 2015 18:47:49 +1100
changeset 273513 bd192633b5e49db4b88e31ffd11157be6d446ae8
parent 273512 ab8b0e58aa828f1c1d8f6d016fd220763c6eb8dd
child 273514 dc63519e3cae242606a1009f707af3d0f6943344
push id29705
push userkwierso@gmail.com
push dateFri, 20 Nov 2015 22:36:48 +0000
treeherdermozilla-central@e3d9cf3dc326 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdhylands
bugs1186750
milestone45.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 1186750 part 5 - Convert nsDOMDeviceStorage::CheckPermission to take already_AddRefed&&. r=dhylands
dom/devicestorage/DeviceStorage.h
dom/devicestorage/nsDeviceStorage.cpp
--- a/dom/devicestorage/DeviceStorage.h
+++ b/dom/devicestorage/DeviceStorage.h
@@ -302,17 +302,17 @@ public:
 
   uint32_t CreateDOMRequest(DOMRequest** aRequest, ErrorResult& aRv);
   uint32_t CreateDOMCursor(DeviceStorageCursorRequest* aRequest,
                            nsDOMDeviceStorageCursor** aCursor,
                            ErrorResult& aRv);
   already_AddRefed<DOMRequest> CreateAndRejectDOMRequest(const char *aReason,
                                                          ErrorResult& aRv);
 
-  nsresult CheckPermission(DeviceStorageRequest* aRequest);
+  nsresult CheckPermission(already_AddRefed<DeviceStorageRequest>&& aRequest);
   void StorePermission(DeviceStorageRequest* aRequest, bool aAllow);
 
   bool IsOwningThread();
   nsresult DispatchToOwningThread(nsIRunnable* aRunnable);
 
 private:
   ~nsDOMDeviceStorage();
 
--- a/dom/devicestorage/nsDeviceStorage.cpp
+++ b/dom/devicestorage/nsDeviceStorage.cpp
@@ -2397,20 +2397,20 @@ class DeviceStoragePermissionCheck final
   : public nsIContentPermissionRequest
   , public nsIRunnable
 {
 public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(DeviceStoragePermissionCheck,
                                            nsIContentPermissionRequest)
 
-  DeviceStoragePermissionCheck(DeviceStorageRequest* aRequest,
-                               uint64_t aWindowID,
-                               const PrincipalInfo &aPrincipalInfo)
-    : mRequest(aRequest)
+  DeviceStoragePermissionCheck(
+    already_AddRefed<DeviceStorageRequest>&& aRequest,
+    uint64_t aWindowID, const PrincipalInfo &aPrincipalInfo)
+    : mRequest(Move(aRequest))
     , mWindowID(aWindowID)
     , mPrincipalInfo(aPrincipalInfo)
   {
     MOZ_ASSERT(mRequest);
   }
 
   NS_IMETHOD Run() override
   {
@@ -2552,47 +2552,41 @@ nsDOMDeviceStorage::nsDOMDeviceStorage(n
   , mOwningThread(NS_GetCurrentThread())
 {
   MOZ_ASSERT(NS_IsMainThread()); // worker support incomplete
   sInstanceCount++;
   DS_LOG_DEBUG("%p (%d)", this, sInstanceCount);
 }
 
 nsresult
-nsDOMDeviceStorage::CheckPermission(DeviceStorageRequest* aRequest)
+nsDOMDeviceStorage::CheckPermission(
+  already_AddRefed<DeviceStorageRequest>&& aRequest)
 {
   MOZ_ASSERT(mManager);
-  uint32_t cache = mManager->CheckPermission(aRequest->GetAccess());
+  RefPtr<DeviceStorageRequest> request(aRequest);
+  uint32_t cache = mManager->CheckPermission(request->GetAccess());
   switch (cache) {
     case nsIPermissionManager::ALLOW_ACTION:
-      return aRequest->Allow();
+      return request->Allow();
     case nsIPermissionManager::DENY_ACTION:
-      return aRequest->Cancel();
+      return request->Cancel();
     case nsIPermissionManager::PROMPT_ACTION:
     default:
     {
       nsCOMPtr<nsIThread> mainThread;
       nsresult rv = NS_GetMainThread(getter_AddRefs(mainThread));
       if (NS_WARN_IF(NS_FAILED(rv))) {
-        return aRequest->Reject(POST_ERROR_EVENT_UNKNOWN);
+        return request->Reject(POST_ERROR_EVENT_UNKNOWN);
       }
 
-      /* We need to do a bit of a song and dance here to release the object
-         because while we can initially increment the ownership count (no one
-         else is using it), we cannot safely decrement after dispatching because
-         it uses cycle collection and requires the main thread to free it. */
-      nsCOMPtr<nsIRunnable> r
-        = new DeviceStoragePermissionCheck(aRequest, mInnerWindowID,
-                                           *mPrincipalInfo);
-      rv = mainThread->Dispatch(r, NS_DISPATCH_NORMAL);
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        rv = aRequest->Reject(POST_ERROR_EVENT_UNKNOWN);
-      }
-      NS_ProxyRelease(mainThread, r.forget().take());
-      return rv;
+      return mainThread->Dispatch(
+        MakeAndAddRef<DeviceStoragePermissionCheck>(request.forget(),
+                                                    mInnerWindowID,
+                                                    *mPrincipalInfo),
+        NS_DISPATCH_NORMAL);
     }
   }
 }
 
 bool
 nsDOMDeviceStorage::IsOwningThread()
 {
   bool owner = false;
@@ -3112,17 +3106,17 @@ nsDOMDeviceStorage::AddOrAppendNamed(Blo
 
   RefPtr<DeviceStorageRequest> request;
   if (aCreate) {
     request = new DeviceStorageCreateRequest();
   } else {
     request = new DeviceStorageAppendRequest();
   }
   request->Initialize(mManager, dsf, id, aBlob->Impl());
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
   return domRequest.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::GetInternal(const nsAString& aPath, bool aEditable,
                                 ErrorResult& aRv)
 {
   MOZ_ASSERT(IsOwningThread());
@@ -3148,17 +3142,17 @@ nsDOMDeviceStorage::GetInternal(const ns
   uint32_t id = CreateDOMRequest(getter_AddRefs(domRequest), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   RefPtr<DeviceStorageRequest> request = new DeviceStorageOpenRequest();
   request->Initialize(mManager, dsf, id);
 
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
   return domRequest.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::Delete(const nsAString& aPath, ErrorResult& aRv)
 {
   MOZ_ASSERT(IsOwningThread());
 
@@ -3182,17 +3176,17 @@ nsDOMDeviceStorage::Delete(const nsAStri
   uint32_t id = CreateDOMRequest(getter_AddRefs(domRequest), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   RefPtr<DeviceStorageRequest> request = new DeviceStorageDeleteRequest();
   request->Initialize(mManager, dsf, id);
 
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
   return domRequest.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::FreeSpace(ErrorResult& aRv)
 {
   MOZ_ASSERT(IsOwningThread());
 
@@ -3203,17 +3197,17 @@ nsDOMDeviceStorage::FreeSpace(ErrorResul
   uint32_t id = CreateDOMRequest(getter_AddRefs(domRequest), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   RefPtr<DeviceStorageRequest> request = new DeviceStorageFreeSpaceRequest();
   request->Initialize(mManager, dsf, id);
 
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
   return domRequest.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::UsedSpace(ErrorResult& aRv)
 {
   MOZ_ASSERT(IsOwningThread());
 
@@ -3228,17 +3222,17 @@ nsDOMDeviceStorage::UsedSpace(ErrorResul
   uint32_t id = CreateDOMRequest(getter_AddRefs(domRequest), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   RefPtr<DeviceStorageRequest> request = new DeviceStorageUsedSpaceRequest();
   request->Initialize(mManager, dsf, id);
 
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
   return domRequest.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::Available(ErrorResult& aRv)
 {
   MOZ_ASSERT(IsOwningThread());
 
@@ -3249,17 +3243,17 @@ nsDOMDeviceStorage::Available(ErrorResul
   uint32_t id = CreateDOMRequest(getter_AddRefs(domRequest), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   RefPtr<DeviceStorageRequest> request = new DeviceStorageAvailableRequest();
   request->Initialize(mManager, dsf, id);
 
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
   return domRequest.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::StorageStatus(ErrorResult& aRv)
 {
   MOZ_ASSERT(IsOwningThread());
 
@@ -3270,17 +3264,17 @@ nsDOMDeviceStorage::StorageStatus(ErrorR
   uint32_t id = CreateDOMRequest(getter_AddRefs(domRequest), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   RefPtr<DeviceStorageRequest> request = new DeviceStorageStatusRequest();
   request->Initialize(mManager, dsf, id);
 
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
   return domRequest.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::Format(ErrorResult& aRv)
 {
   MOZ_ASSERT(IsOwningThread());
 
@@ -3291,17 +3285,17 @@ nsDOMDeviceStorage::Format(ErrorResult& 
   uint32_t id = CreateDOMRequest(getter_AddRefs(domRequest), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   RefPtr<DeviceStorageRequest> request = new DeviceStorageFormatRequest();
   request->Initialize(mManager, dsf, id);
 
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
   return domRequest.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::Mount(ErrorResult& aRv)
 {
   MOZ_ASSERT(IsOwningThread());
 
@@ -3312,17 +3306,17 @@ nsDOMDeviceStorage::Mount(ErrorResult& a
   uint32_t id = CreateDOMRequest(getter_AddRefs(domRequest), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   RefPtr<DeviceStorageRequest> request = new DeviceStorageMountRequest();
   request->Initialize(mManager, dsf, id);
 
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
   return domRequest.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::Unmount(ErrorResult& aRv)
 {
   MOZ_ASSERT(IsOwningThread());
 
@@ -3334,17 +3328,17 @@ nsDOMDeviceStorage::Unmount(ErrorResult&
   if (aRv.Failed()) {
     return nullptr;
   }
 
 
   RefPtr<DeviceStorageRequest> request = new DeviceStorageUnmountRequest();
   request->Initialize(mManager, dsf, id);
 
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
   return domRequest.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::CreateFileDescriptor(const nsAString& aPath,
                                          DeviceStorageFileDescriptor* aDSFileDescriptor,
                                          ErrorResult& aRv)
 {
@@ -3382,17 +3376,17 @@ nsDOMDeviceStorage::CreateFileDescriptor
   uint32_t id = CreateDOMRequest(getter_AddRefs(domRequest), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   RefPtr<DeviceStorageRequest> request = new DeviceStorageCreateFdRequest();
   request->Initialize(mManager, dsf, id, aDSFileDescriptor);
 
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
   return domRequest.forget();
 }
 
 bool
 nsDOMDeviceStorage::Default()
 {
   nsString defaultStorageName;
   GetDefaultStorageName(mStorageType, defaultStorageName);
@@ -3488,17 +3482,17 @@ nsDOMDeviceStorage::EnumerateInternal(co
   if (aRv.Failed()) {
     return nullptr;
   }
 
   if (!dsf->IsSafePath()) {
     aRv = mManager->Reject(id, POST_ERROR_EVENT_PERMISSION_DENIED);
   } else {
     request->Initialize(mManager, dsf, id, since);
-    aRv = CheckPermission(request);
+    aRv = CheckPermission(request.forget());
   }
 
   return cursor.forget();
 }
 
 void
 nsDOMDeviceStorage::OnWritableNameChanged()
 {
@@ -3691,17 +3685,17 @@ nsDOMDeviceStorage::EventListenerWasAdde
   if (aRv.Failed()) {
     return;
   }
 
   RefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(mStorageType,
                                                           mStorageName);
   RefPtr<DeviceStorageRequest> request = new DeviceStorageWatchRequest();
   request->Initialize(mManager, dsf, id);
-  aRv = CheckPermission(request);
+  aRv = CheckPermission(request.forget());
 }
 
 Atomic<uint32_t> DeviceStorageRequestManager::sLastRequestId(0);
 
 DeviceStorageRequestManager::DeviceStorageRequestManager()
   : mOwningThread(NS_GetCurrentThread())
   , mMutex("DeviceStorageRequestManager::mMutex")
   , mShutdown(false)