Bug 928558 - AutoMounter mark volumes as shared a bit early when open files are detected. r=qDot
authorDave Hylands <dhylands@mozilla.com>
Fri, 18 Oct 2013 20:21:52 -0700
changeset 166276 44b92484bfe62e479b994191017b20d0ecc27608
parent 166275 51e1f24155fd74036a6131545a64024d25032986
child 166277 d95ba4dea1ecf0084da5eb5524ddf1a50295e678
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqDot
bugs928558
milestone27.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 928558 - AutoMounter mark volumes as shared a bit early when open files are detected. r=qDot
dom/system/gonk/AutoMounter.cpp
dom/system/gonk/Volume.cpp
dom/system/gonk/nsVolume.cpp
dom/system/gonk/nsVolume.h
dom/system/gonk/nsVolumeService.cpp
--- a/dom/system/gonk/AutoMounter.cpp
+++ b/dom/system/gonk/AutoMounter.cpp
@@ -311,21 +311,47 @@ AutoMounterResponseCallback::ResponseRec
       aCommand->CmdStr(), ResponseCode(), ResponseStr().get());
 
   if (++mErrorCount < kMaxErrorCount) {
     DBG("Calling UpdateState due to VolumeResponseCallback::OnError");
     sAutoMounter->UpdateState();
   }
 }
 
+class AutoBool {
+public:
+    explicit AutoBool(bool &aBool) : mBool(aBool) {
+      mBool = true;
+    }
+
+    ~AutoBool() {
+      mBool = false;
+    }
+
+private:
+    bool &mBool;
+};
+
 /***************************************************************************/
 
 void
 AutoMounter::UpdateState()
 {
+  static bool inUpdateState = false;
+  if (inUpdateState) {
+    // When UpdateState calls SetISharing, this causes a volume state
+    // change to occur, which would normally cause UpdateState to be called
+    // again. We want the volume state change to go out (so that device
+    // storage will see the change in sharing state), but since we're
+    // already in UpdateState we just want to prevent recursion from screwing
+    // things up.
+    return;
+  }
+  AutoBool inUpdateStateDetector(inUpdateState);
+
   MOZ_ASSERT(MessageLoop::current() == XRE_GetIOMessageLoop());
 
   // If the following preconditions are met:
   //    - UMS is available (i.e. compiled into the kernel)
   //    - UMS is enabled
   //    - AutoMounter is enabled
   //    - USB cable is plugged in
   //  then we will try to unmount and share
@@ -374,16 +400,17 @@ AutoMounter::UpdateState()
     }
   }
 
   bool tryToShare = (umsAvail && umsEnabled && enabled && usbCablePluggedIn);
   LOG("UpdateState: umsAvail:%d umsEnabled:%d mode:%d usbCablePluggedIn:%d tryToShare:%d",
       umsAvail, umsEnabled, mMode, usbCablePluggedIn, tryToShare);
 
   bool filesOpen = false;
+  static unsigned filesOpenDelayCount = 0;
   VolumeArray::index_type volIndex;
   VolumeArray::size_type  numVolumes = VolumeManager::NumVolumes();
   for (volIndex = 0; volIndex < numVolumes; volIndex++) {
     RefPtr<Volume>  vol = VolumeManager::GetVolume(volIndex);
     Volume::STATE   volState = vol->State();
 
     if (vol->State() == nsIVolume::STATE_MOUNTED) {
       LOG("UpdateState: Volume %s is %s and %s @ %s gen %d locked %d sharing %c",
@@ -408,16 +435,22 @@ AutoMounter::UpdateState()
           if (vol->IsMountLocked()) {
             // The volume is currently locked, so leave it in the mounted
             // state.
             LOGW("UpdateState: Mounted volume %s is locked, not sharing",
                  vol->NameStr());
             break;
           }
 
+          // Mark the volume as if we've started sharing. This will cause
+          // apps which watch device storage notifications to see the volume
+          // go into the shared state, and prompt them to close any open files
+          // that they might have.
+          vol->SetIsSharing(true);
+
           // Check to see if there are any open files on the volume and
           // don't initiate the unmount while there are open files.
           OpenFileFinder::Info fileInfo;
           OpenFileFinder fileFinder(vol->MountPoint());
           if (fileFinder.First(&fileInfo)) {
             LOGW("The following files are open under '%s'",
                  vol->MountPoint().get());
             do {
@@ -426,33 +459,43 @@ AutoMounter::UpdateState()
                    fileInfo.mFileName.get(),
                    fileInfo.mAppName.get(),
                    fileInfo.mComm.get(),
                    fileInfo.mExe.get());
             } while (fileFinder.Next(&fileInfo));
             LOGW("UpdateState: Mounted volume %s has open files, not sharing",
                  vol->NameStr());
 
-            // Check again in 5 seconds to see if the files are closed. Since
-            // we're trying to share the volume, this implies that we're
+            // Check again in a few seconds to see if the files are closed.
+            // Since we're trying to share the volume, this implies that we're
             // plugged into the PC via USB and this in turn implies that the
             // battery is charging, so we don't need to be too concerned about
             // wasting battery here.
+            //
+            // If we just detected that there were files open, then we use
+            // a short timer. We will have told the apps that we're trying
+            // trying to share, and they'll be closing their files. This makes
+            // the sharing more responsive. If after a few seconds, the apps
+            // haven't closed their files, then we back off.
+
+            int delay = 1000;
+            if (filesOpenDelayCount > 10) {
+              delay = 5000;
+            }
             MessageLoopForIO::current()->
               PostDelayedTask(FROM_HERE,
                               NewRunnableMethod(this, &AutoMounter::UpdateState),
-                              5000);
+                              delay);
             filesOpen = true;
             break;
           }
 
           // Volume is mounted, we need to unmount before
           // we can share.
           LOG("UpdateState: Unmounting %s", vol->NameStr());
-          vol->SetIsSharing(true);
           vol->StartUnmount(mResponseCallback);
           return; // UpdateState will be called again when the Unmount command completes
         }
         case nsIVolume::STATE_IDLE: {
           // Volume is unmounted. We can go ahead and share.
           LOG("UpdateState: Sharing %s", vol->NameStr());
           vol->StartShare(mResponseCallback);
           return; // UpdateState will be called again when the Share command completes
@@ -483,18 +526,20 @@ AutoMounter::UpdateState()
           break;
         }
       }
     }
   }
 
   int32_t status = AUTOMOUNTER_STATUS_DISABLED;
   if (filesOpen) {
+    filesOpenDelayCount++;
     status = AUTOMOUNTER_STATUS_FILES_OPEN;
   } else if (enabled) {
+    filesOpenDelayCount = 0;
     status = AUTOMOUNTER_STATUS_ENABLED;
   }
   SetAutoMounterStatus(status);
 }
 
 /***************************************************************************/
 
 static void
--- a/dom/system/gonk/Volume.cpp
+++ b/dom/system/gonk/Volume.cpp
@@ -63,17 +63,25 @@ Volume::Volume(const nsCSubstring& aName
     mIsSharing(false)
 {
   DBG("Volume %s: created", NameStr());
 }
 
 void
 Volume::SetIsSharing(bool aIsSharing)
 {
+  if (aIsSharing == mIsSharing) {
+    return;
+  }
   mIsSharing = aIsSharing;
+  LOG("Volume %s: IsSharing set to %d state %s",
+      NameStr(), (int)mIsSharing, StateStr(mState));
+  if (mIsSharing) {
+    mEventObserverList.Broadcast(this);
+  }
 }
 
 void
 Volume::SetMediaPresent(bool aMediaPresent)
 {
   MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
   MOZ_ASSERT(MessageLoop::current() == XRE_GetIOMessageLoop());
 
--- a/dom/system/gonk/nsVolume.cpp
+++ b/dom/system/gonk/nsVolume.cpp
@@ -90,16 +90,22 @@ bool nsVolume::Equals(nsIVolume* aVolume
   }
 
   bool isFake;
   aVolume->GetIsFake(&isFake);
   if (mIsFake != isFake) {
     return false;
   }
 
+  bool isSharing;
+  aVolume->GetIsSharing(&isSharing);
+  if (mIsSharing != isSharing) {
+    return false;
+  }
+
   return true;
 }
 
 NS_IMETHODIMP nsVolume::GetIsMediaPresent(bool *aIsMediaPresent)
 {
   *aIsMediaPresent = mIsMediaPresent;
   return NS_OK;
 }
--- a/dom/system/gonk/nsVolume.h
+++ b/dom/system/gonk/nsVolume.h
@@ -66,28 +66,29 @@ public:
   bool IsMountLocked() const          { return mMountLocked; }
 
   const nsString& MountPoint() const  { return mMountPoint; }
   nsCString MountPointStr() const     { return NS_LossyConvertUTF16toASCII(mMountPoint); }
 
   int32_t State() const               { return mState; }
   const char* StateStr() const        { return NS_VolumeStateStr(mState); }
 
+  bool IsFake() const                 { return mIsFake; }
+  bool IsMediaPresent() const         { return mIsMediaPresent; }
+  bool IsSharing() const              { return mIsSharing; }
+
   typedef nsTArray<nsRefPtr<nsVolume> > Array;
 
 private:
   ~nsVolume() {}
 
   friend class nsVolumeService; // Calls the following XxxMountLock functions
   void UpdateMountLock(const nsAString& aMountLockState);
   void UpdateMountLock(bool aMountLocked);
 
-  bool IsFake() const                 { return mIsFake; }
-  bool IsMediaPresent() const         { return mIsMediaPresent; }
-  bool IsSharing() const              { return mIsSharing; }
   void SetIsFake(bool aIsFake);
   void SetState(int32_t aState);
 
   nsString mName;
   nsString mMountPoint;
   int32_t  mState;
   int32_t  mMountGeneration;
   bool     mMountLocked;
--- a/dom/system/gonk/nsVolumeService.cpp
+++ b/dom/system/gonk/nsVolumeService.cpp
@@ -449,15 +449,15 @@ private:
 
 void
 nsVolumeService::UpdateVolumeIOThread(const Volume* aVolume)
 {
   DBG("UpdateVolumeIOThread: Volume '%s' state %s mount '%s' gen %d locked %d "
       "media %d sharing %d",
       aVolume->NameStr(), aVolume->StateStr(), aVolume->MountPoint().get(),
       aVolume->MountGeneration(), (int)aVolume->IsMountLocked(),
-      (int)aVolume->IsMediaPresent(), (int)aVolume->IsSharing());
+      (int)aVolume->MediaPresent(), (int)aVolume->IsSharing());
   MOZ_ASSERT(MessageLoop::current() == XRE_GetIOMessageLoop());
   NS_DispatchToMainThread(new UpdateVolumeRunnable(this, aVolume));
 }
 
 } // namespace system
 } // namespace mozilla