Bug 1166320 - Make volume service safer to use off main thread. r=dhylands

dom/system/gonk/nsVolumeService.cpp

@@ -368,7 +368,7 @@ nsVolumeService::FindVolumeByMountLockName(const nsAString& aMountLockName)
 }
 
 already_AddRefed<nsVolume>
-nsVolumeService::FindVolumeByName(const nsAString& aName)
+nsVolumeService::FindVolumeByName(const nsAString& aName, nsVolume::Array::index_type* aIndex)
 {
   mArrayMonitor.AssertCurrentThreadOwns();
 
@@ -377,52 +377,34 @@ nsVolumeService::FindVolumeByName(const nsAString& aName)
   for (volIndex = 0; volIndex < numVolumes; volIndex++) {
     nsRefPtr<nsVolume> vol = mVolumeArray[volIndex];
     if (vol->Name().Equals(aName)) {
+      if (aIndex) {
+        *aIndex = volIndex;
+      }
       return vol.forget();
     }
   }
   return nullptr;
 }
 
-//static
-already_AddRefed<nsVolume>
-nsVolumeService::CreateOrFindVolumeByName(const nsAString& aName, bool aIsFake /*= false*/)
-{
-  MonitorAutoLock autoLock(mArrayMonitor);
-
-  nsRefPtr<nsVolume> vol;
-  vol = FindVolumeByName(aName);
-  if (vol) {
-    return vol.forget();
-  }
-  // Volume not found - add a new one
-  vol = new nsVolume(aName);
-  vol->SetIsFake(aIsFake);
-  mVolumeArray.AppendElement(vol);
-  return vol.forget();
-}
-
 void
-nsVolumeService::UpdateVolume(nsIVolume* aVolume, bool aNotifyObservers)
+nsVolumeService::UpdateVolume(nsVolume* aVolume, bool aNotifyObservers)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  nsString volName;
-  aVolume->GetName(volName);
-  bool aIsFake;
-  aVolume->GetIsFake(&aIsFake);
-  nsRefPtr<nsVolume> vol = CreateOrFindVolumeByName(volName, aIsFake);
-  if (vol->Equals(aVolume)) {
-    // Nothing has really changed. Don't bother telling anybody.
-    return;
+  {
+    MonitorAutoLock autoLock(mArrayMonitor);
+    nsVolume::Array::index_type volIndex;
+    nsRefPtr<nsVolume> vol = FindVolumeByName(aVolume->Name(), &volIndex);
+    if (!vol) {
+      mVolumeArray.AppendElement(aVolume);
+    } else if (vol->Equals(aVolume) || (!vol->IsFake() && aVolume->IsFake())) {
+      // Ignore if nothing changed or if a fake tries to override a real volume.
+      return;
+    } else {
+      mVolumeArray.ReplaceElementAt(volIndex, aVolume);
+    }
   }
 
-  if (!vol->IsFake() && aIsFake) {
-    // Prevent an incoming fake volume from overriding an existing real volume.
-    return;
-  }
-
-  vol->Set(aVolume);
-
   if (!aNotifyObservers) {
     return;
   }
@@ -431,8 +413,8 @@ nsVolumeService::UpdateVolume(nsIVolume* aVolume, bool aNotifyObservers)
   if (!obs) {
     return;
   }
-  NS_ConvertUTF8toUTF16 stateStr(vol->StateStr());
-  obs->NotifyObservers(vol, NS_VOLUME_STATE_CHANGED, stateStr.get());
+  NS_ConvertUTF8toUTF16 stateStr(aVolume->StateStr());
+  obs->NotifyObservers(aVolume, NS_VOLUME_STATE_CHANGED, stateStr.get());
 }
 
 NS_IMETHODIMP
@@ -471,9 +453,7 @@ nsVolumeService::SetFakeVolumeState(const nsAString& name, int32_t state)
       return NS_ERROR_NOT_AVAILABLE;
     }
 
-    // UpdateVolume expects the volume passed in to NOT be the
-    // same pointer as what CreateOrFindVolumeByName would return,
-    // which is why we allocate a temporary volume here.
+    // Clone the existing volume so we can replace it
     nsRefPtr<nsVolume> volume = new nsVolume(name);
     volume->Set(vol);
     volume->SetState(state);
@@ -502,15 +482,15 @@ nsVolumeService::RemoveFakeVolume(const nsAString& name)
 void
 nsVolumeService::RemoveVolumeByName(const nsAString& aName)
 {
-  nsRefPtr<nsVolume> vol;
   {
     MonitorAutoLock autoLock(mArrayMonitor);
-    vol = FindVolumeByName(aName);
+    nsVolume::Array::index_type volIndex;
+    nsRefPtr<nsVolume> vol = FindVolumeByName(aName, &volIndex);
+    if (!vol) {
+      return;
+    }
+    mVolumeArray.RemoveElementAt(volIndex);
   }
-  if (!vol) {
-    return;
-  }
-  mVolumeArray.RemoveElement(vol);
 
   if (XRE_GetProcessType() == GeckoProcessType_Default) {
     nsCOMPtr<nsIObserverService> obs = GetObserverService();

dom/system/gonk/nsVolumeService.h

@@ -47,7 +47,7 @@ public:
   void DumpNoLock(const char* aLabel);
 
   // To use this function, you have to create a new volume and pass it in.
-  void UpdateVolume(nsIVolume* aVolume, bool aNotifyObservers = true);
+  void UpdateVolume(nsVolume* aVolume, bool aNotifyObservers = true);
   void UpdateVolumeIOThread(const Volume* aVolume);
 
   void RecvVolumesFromParent(const nsTArray<dom::VolumeInfo>& aVolumes);
@@ -61,8 +61,9 @@ private:
   void CheckMountLock(const nsAString& aMountLockName,
                       const nsAString& aMountLockState);
   already_AddRefed<nsVolume> FindVolumeByMountLockName(const nsAString& aMountLockName);
-  already_AddRefed<nsVolume> FindVolumeByName(const nsAString& aName);
-  already_AddRefed<nsVolume> CreateOrFindVolumeByName(const nsAString& aName, bool aIsFake = false);
+
+  already_AddRefed<nsVolume> FindVolumeByName(const nsAString& aName,
+                                              nsVolume::Array::index_type* aIndex = nullptr);
 
   Monitor mArrayMonitor;
   nsVolume::Array mVolumeArray;