Bug 928558 - AutoMounter mark volumes as shared a bit early when open files are detected. r=qDot a=koi
authorDave Hylands <dhylands@mozilla.com>
Fri, 18 Oct 2013 20:21:52 -0700
changeset 160844 e346a58d6b6ab10aa7a345d3ecd401947eaf417f
parent 160843 9ce74359001ef9592de6cc83ea642a9d3e835659
child 160845 f8a6be5f607d2305e2274fb37d919077b319425a
push id2961
push userlsblakk@mozilla.com
push dateMon, 28 Oct 2013 21:59:28 +0000
treeherdermozilla-beta@73ef4f13486f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqDot, koi
bugs928558
milestone26.0a2
Bug 928558 - AutoMounter mark volumes as shared a bit early when open files are detected. r=qDot a=koi
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