From 0ffd2e95b87a56402eaf9a51d4febd7ffe87a16e Mon Sep 17 00:00:00 2001 From: Dave Hylands Date: Thu, 22 Aug 2013 16:12:25 -0700 Subject: [PATCH] Bug 878310 - Improve DeviceStorage volume status reporting. r=qDot This changeset causes tranistory states when changing from mounted to shared, and back to mounted to be reported as shared instead of unavailable. We also don't send the same status twice in a row. --- dom/devicestorage/DeviceStorage.h | 1 + dom/devicestorage/nsDeviceStorage.cpp | 25 ++++++++++++++++--- dom/ipc/ContentChild.cpp | 9 +++++-- dom/ipc/ContentChild.h | 4 ++- dom/ipc/ContentParent.cpp | 7 +++++- dom/ipc/PContent.ipdl | 3 ++- dom/system/gonk/AutoMounter.cpp | 1 + dom/system/gonk/Volume.cpp | 35 ++++++++++++++++++++++++--- dom/system/gonk/Volume.h | 3 +++ dom/system/gonk/nsIVolume.idl | 16 +++++++++++- dom/system/gonk/nsVolume.cpp | 35 ++++++++++++++++++++++++--- dom/system/gonk/nsVolume.h | 20 +++++++++++---- dom/system/gonk/nsVolumeService.cpp | 21 +++++++++++----- 13 files changed, 153 insertions(+), 27 deletions(-) diff --git a/dom/devicestorage/DeviceStorage.h b/dom/devicestorage/DeviceStorage.h index d8f73f9f293..83059aa60a7 100644 --- a/dom/devicestorage/DeviceStorage.h +++ b/dom/devicestorage/DeviceStorage.h @@ -300,6 +300,7 @@ private: static mozilla::StaticRefPtr sVolumeNameCache; #ifdef MOZ_WIDGET_GONK + nsString mLastStatus; void DispatchMountChangeEvent(nsAString& aVolumeStatus); #endif diff --git a/dom/devicestorage/nsDeviceStorage.cpp b/dom/devicestorage/nsDeviceStorage.cpp index 676069ff659..304c40737c2 100644 --- a/dom/devicestorage/nsDeviceStorage.cpp +++ b/dom/devicestorage/nsDeviceStorage.cpp @@ -1261,14 +1261,27 @@ DeviceStorageFile::GetStatus(nsAString& aStatus) nsCOMPtr vol; nsresult rv = vs->GetVolumeByName(mStorageName, getter_AddRefs(vol)); NS_ENSURE_SUCCESS_VOID(rv); + if (!vol) { + return; + } + bool isMediaPresent; + rv = vol->GetIsMediaPresent(&isMediaPresent); + NS_ENSURE_SUCCESS_VOID(rv); + if (!isMediaPresent) { + return; + } + bool isSharing; + rv = vol->GetIsSharing(&isSharing); + NS_ENSURE_SUCCESS_VOID(rv); + if (isSharing) { + aStatus.AssignLiteral("shared"); + return; + } int32_t volState; rv = vol->GetState(&volState); NS_ENSURE_SUCCESS_VOID(rv); if (volState == nsIVolume::STATE_MOUNTED) { aStatus.AssignLiteral("available"); - } else if (volState == nsIVolume::STATE_SHARED || - volState == nsIVolume::STATE_SHAREDMNT) { - aStatus.AssignLiteral("shared"); } #endif } @@ -3159,6 +3172,12 @@ nsDOMDeviceStorage::EnumerateInternal(const nsAString& aPath, void nsDOMDeviceStorage::DispatchMountChangeEvent(nsAString& aVolumeStatus) { + if (aVolumeStatus == mLastStatus) { + // We've already sent this status, don't bother sending it again. + return; + } + mLastStatus = aVolumeStatus; + nsCOMPtr event; NS_NewDOMDeviceStorageChangeEvent(getter_AddRefs(event), this, nullptr, nullptr); diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index 511357c68b6..1660278db72 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -1248,11 +1248,14 @@ bool ContentChild::RecvFileSystemUpdate(const nsString& aFsName, const nsString& aVolumeName, const int32_t& aState, - const int32_t& aMountGeneration) + const int32_t& aMountGeneration, + const bool& aIsMediaPresent, + const bool& aIsSharing) { #ifdef MOZ_WIDGET_GONK nsRefPtr volume = new nsVolume(aFsName, aVolumeName, aState, - aMountGeneration); + aMountGeneration, aIsMediaPresent, + aIsSharing); nsRefPtr vs = nsVolumeService::GetSingleton(); if (vs) { @@ -1264,6 +1267,8 @@ ContentChild::RecvFileSystemUpdate(const nsString& aFsName, unused << aVolumeName; unused << aState; unused << aMountGeneration; + unused << aIsMediaPresent; + unused << aIsSharing; #endif return true; } diff --git a/dom/ipc/ContentChild.h b/dom/ipc/ContentChild.h index 127fad3de4f..4db71103ffa 100644 --- a/dom/ipc/ContentChild.h +++ b/dom/ipc/ContentChild.h @@ -208,7 +208,9 @@ public: virtual bool RecvFileSystemUpdate(const nsString& aFsName, const nsString& aVolumeName, const int32_t& aState, - const int32_t& aMountGeneration); + const int32_t& aMountGeneration, + const bool& aIsMediaPresent, + const bool& aIsSharing); virtual bool RecvNotifyProcessPriorityChanged(const hal::ProcessPriority& aPriority); virtual bool RecvMinimizeMemoryUsage(); diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index bad396dee70..d001a1c076d 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -1676,14 +1676,19 @@ ContentParent::Observe(nsISupports* aSubject, nsString mountPoint; int32_t state; int32_t mountGeneration; + bool isMediaPresent; + bool isSharing; vol->GetName(volName); vol->GetMountPoint(mountPoint); vol->GetState(&state); vol->GetMountGeneration(&mountGeneration); + vol->GetIsMediaPresent(&isMediaPresent); + vol->GetIsSharing(&isSharing); unused << SendFileSystemUpdate(volName, mountPoint, state, - mountGeneration); + mountGeneration, isMediaPresent, + isSharing); } #endif #ifdef ACCESSIBILITY diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index d856baa55d1..89763a664a8 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -269,7 +269,8 @@ child: nsCString reasons); FileSystemUpdate(nsString fsName, nsString mountPoint, int32_t fsState, - int32_t mountGeneration); + int32_t mountGeneration, bool isMediaPresent, + bool isSharing); NotifyProcessPriorityChanged(ProcessPriority priority); MinimizeMemoryUsage(); diff --git a/dom/system/gonk/AutoMounter.cpp b/dom/system/gonk/AutoMounter.cpp index 09006519deb..190ced792d4 100644 --- a/dom/system/gonk/AutoMounter.cpp +++ b/dom/system/gonk/AutoMounter.cpp @@ -443,6 +443,7 @@ AutoMounter::UpdateState() // 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 } diff --git a/dom/system/gonk/Volume.cpp b/dom/system/gonk/Volume.cpp index e2b00a307ae..532be04c078 100644 --- a/dom/system/gonk/Volume.cpp +++ b/dom/system/gonk/Volume.cpp @@ -59,11 +59,18 @@ Volume::Volume(const nsCSubstring& aName) mMountGeneration(-1), mMountLocked(true), // Needs to agree with nsVolume::nsVolume mSharingEnabled(false), - mCanBeShared(true) + mCanBeShared(true), + mIsSharing(false) { DBG("Volume %s: created", NameStr()); } +void +Volume::SetIsSharing(bool aIsSharing) +{ + mIsSharing = aIsSharing; +} + void Volume::SetMediaPresent(bool aMediaPresent) { @@ -133,9 +140,29 @@ Volume::SetState(Volume::STATE aNewState) StateStr(aNewState), mEventObserverList.Length()); } - if (aNewState == nsIVolume::STATE_NOMEDIA) { - // Cover the startup case where we don't get insertion/removal events - mMediaPresent = false; + switch (aNewState) { + case nsIVolume::STATE_NOMEDIA: + // Cover the startup case where we don't get insertion/removal events + mMediaPresent = false; + mIsSharing = false; + break; + + case nsIVolume::STATE_MOUNTED: + case nsIVolume::STATE_FORMATTING: + mIsSharing = false; + break; + + case nsIVolume::STATE_SHARED: + case nsIVolume::STATE_SHAREDMNT: + // Covers startup cases. Normally, mIsSharing would be set to true + // when we issue the command to initiate the sharing process, but + // it's conceivable that a volume could already be in a shared state + // when b2g starts. + mIsSharing = true; + break; + + default: + break; } mState = aNewState; mEventObserverList.Broadcast(this); diff --git a/dom/system/gonk/Volume.h b/dom/system/gonk/Volume.h index 68a35f7989d..02b3b4d8e13 100644 --- a/dom/system/gonk/Volume.h +++ b/dom/system/gonk/Volume.h @@ -47,6 +47,7 @@ public: bool MediaPresent() const { return mMediaPresent; } bool CanBeShared() const { return mCanBeShared; } bool IsSharingEnabled() const { return mCanBeShared && mSharingEnabled; } + bool IsSharing() const { return mIsSharing; } void SetSharingEnabled(bool aSharingEnabled); @@ -71,6 +72,7 @@ private: void StartShare(VolumeResponseCallback* aCallback); void StartUnshare(VolumeResponseCallback* aCallback); + void SetIsSharing(bool aIsSharing); void SetState(STATE aNewState); void SetMediaPresent(bool aMediaPresent); void SetMountPoint(const nsCSubstring& aMountPoint); @@ -90,6 +92,7 @@ private: bool mMountLocked; bool mSharingEnabled; bool mCanBeShared; + bool mIsSharing; static EventObserverList mEventObserverList; }; diff --git a/dom/system/gonk/nsIVolume.idl b/dom/system/gonk/nsIVolume.idl index c7170fbb04b..f8b21c5e4fc 100644 --- a/dom/system/gonk/nsIVolume.idl +++ b/dom/system/gonk/nsIVolume.idl @@ -5,7 +5,7 @@ #include "nsISupports.idl" #include "nsIVolumeStat.idl" -[scriptable, uuid(4b5bd562-bd05-4658-ab0f-f668a9e25fb5)] +[scriptable, uuid(e476e7ea-5cde-4d5a-b00d-d60daad76398)] interface nsIVolume : nsISupports { // These MUST match the states from android's system/vold/Volume.h header @@ -48,6 +48,20 @@ interface nsIVolume : nsISupports // Determines if a mountlock is currently being held against this volume. readonly attribute boolean isMountLocked; + // Determines if media is actually present or not. Note, that when an sdcard + // is ejected, it may go through several tranistory states before finally + // arriving at STATE_NOMEDIA. So isMediaPresent may be false even when the + // current state isn't STATE_NOMEDIA. + readonly attribute boolean isMediaPresent; + + // Determines if the volume is currently being shared. This covers off + // more than just state == STATE_SHARED. isSharing will return true from the + // time that the volume leaves the mounted state, until it gets back to + // mounted, nomedia, or formatting states. This attribute is to allow + // device storage to suppress unwanted 'unavailable' status when + // transitioning from mounted to sharing and back again. + readonly attribute boolean isSharing; + nsIVolumeStat getStats(); // Whether this is a fake volume. diff --git a/dom/system/gonk/nsVolume.cpp b/dom/system/gonk/nsVolume.cpp index c0791364adc..67061a5d1d0 100644 --- a/dom/system/gonk/nsVolume.cpp +++ b/dom/system/gonk/nsVolume.cpp @@ -51,7 +51,9 @@ nsVolume::nsVolume(const Volume* aVolume) mState(aVolume->State()), mMountGeneration(aVolume->MountGeneration()), mMountLocked(aVolume->IsMountLocked()), - mIsFake(false) + mIsFake(false), + mIsMediaPresent(aVolume->MediaPresent()), + mIsSharing(aVolume->IsSharing()) { } @@ -96,12 +98,24 @@ bool nsVolume::Equals(nsIVolume* aVolume) return true; } +NS_IMETHODIMP nsVolume::GetIsMediaPresent(bool *aIsMediaPresent) +{ + *aIsMediaPresent = mIsMediaPresent; + return NS_OK; +} + NS_IMETHODIMP nsVolume::GetIsMountLocked(bool *aIsMountLocked) { *aIsMountLocked = mMountLocked; return NS_OK; } +NS_IMETHODIMP nsVolume::GetIsSharing(bool *aIsSharing) +{ + *aIsSharing = mIsSharing; + return NS_OK; +} + NS_IMETHODIMP nsVolume::GetName(nsAString& aName) { aName = mName; @@ -154,9 +168,11 @@ void nsVolume::LogState() const { if (mState == nsIVolume::STATE_MOUNTED) { - LOG("nsVolume: %s state %s @ '%s' gen %d locked %d fake %d", + LOG("nsVolume: %s state %s @ '%s' gen %d locked %d fake %d " + "media %d sharing %d", NameStr().get(), StateStr(), MountPointStr().get(), - MountGeneration(), (int)IsMountLocked(), (int)IsFake()); + MountGeneration(), (int)IsMountLocked(), (int)IsFake(), + (int)IsMediaPresent(), (int)IsSharing()); return; } @@ -171,6 +187,8 @@ void nsVolume::Set(nsIVolume* aVolume) aVolume->GetMountPoint(mMountPoint); aVolume->GetState(&mState); aVolume->GetIsFake(&mIsFake); + aVolume->GetIsMediaPresent(&mIsMediaPresent); + aVolume->GetIsSharing(&mIsSharing); int32_t volMountGeneration; aVolume->GetMountGeneration(&volMountGeneration); @@ -237,6 +255,17 @@ nsVolume::UpdateMountLock(bool aMountLocked) MountGeneration(), aMountLocked)); } +void +nsVolume::SetIsFake(bool aIsFake) +{ + mIsFake = aIsFake; + if (mIsFake) { + // The media is always present for fake volumes. + mIsMediaPresent = true; + MOZ_ASSERT(!mIsSharing); + } +} + void nsVolume::SetState(int32_t aState) { diff --git a/dom/system/gonk/nsVolume.h b/dom/system/gonk/nsVolume.h index b2f85fdb1a2..b77ae3a4be2 100644 --- a/dom/system/gonk/nsVolume.h +++ b/dom/system/gonk/nsVolume.h @@ -25,15 +25,19 @@ public: // This constructor is used by the UpdateVolumeRunnable constructor nsVolume(const Volume* aVolume); - // This constructor is used by ContentChild::RecvFileSystemUpdate + // This constructor is used by ContentChild::RecvFileSystemUpdate which is + // used to update the volume cache maintained in the child process. nsVolume(const nsAString& aName, const nsAString& aMountPoint, - const int32_t& aState, const int32_t& aMountGeneration) + const int32_t& aState, const int32_t& aMountGeneration, + const bool& aIsMediaPresent, const bool& aIsSharing) : mName(aName), mMountPoint(aMountPoint), mState(aState), mMountGeneration(aMountGeneration), mMountLocked(false), - mIsFake(false) + mIsFake(false), + mIsMediaPresent(aIsMediaPresent), + mIsSharing(aIsSharing) { } @@ -44,7 +48,9 @@ public: mState(STATE_INIT), mMountGeneration(-1), mMountLocked(true), // Needs to agree with Volume::Volume - mIsFake(false) + mIsFake(false), + mIsMediaPresent(false), + mIsSharing(false) { } @@ -75,7 +81,9 @@ private: void UpdateMountLock(bool aMountLocked); bool IsFake() const { return mIsFake; } - void SetIsFake(bool aIsFake) { mIsFake = aIsFake; } + bool IsMediaPresent() const { return mIsMediaPresent; } + bool IsSharing() const { return mIsSharing; } + void SetIsFake(bool aIsFake); void SetState(int32_t aState); nsString mName; @@ -84,6 +92,8 @@ private: int32_t mMountGeneration; bool mMountLocked; bool mIsFake; + bool mIsMediaPresent; + bool mIsSharing; }; } // system diff --git a/dom/system/gonk/nsVolumeService.cpp b/dom/system/gonk/nsVolumeService.cpp index 1e9cc29fa08..8e87c0008bb 100644 --- a/dom/system/gonk/nsVolumeService.cpp +++ b/dom/system/gonk/nsVolumeService.cpp @@ -246,7 +246,9 @@ nsVolumeService::CreateOrGetVolumeByPath(const nsAString& aPath, nsIVolume** aRe // from the pathname, so that the caller can determine the volume size. nsCOMPtr vol = new nsVolume(NS_LITERAL_STRING("fake"), aPath, nsIVolume::STATE_MOUNTED, - -1 /*generation*/); + -1 /* generation */, + true /* isMediaPresent*/, + false /* isSharing */); vol.forget(aResult); return NS_OK; } @@ -375,7 +377,10 @@ NS_IMETHODIMP nsVolumeService::CreateFakeVolume(const nsAString& name, const nsAString& path) { if (XRE_GetProcessType() == GeckoProcessType_Default) { - nsRefPtr vol = new nsVolume(name, path, nsIVolume::STATE_INIT, -1); + nsRefPtr vol = new nsVolume(name, path, nsIVolume::STATE_INIT, + -1 /* mountGeneration */, + true /* isMediaPresent */, + false /* isSharing */); vol->SetIsFake(true); vol->LogState(); UpdateVolume(vol.get()); @@ -425,9 +430,11 @@ public: NS_IMETHOD Run() { MOZ_ASSERT(NS_IsMainThread()); - DBG("UpdateVolumeRunnable::Run '%s' state %s gen %d locked %d", + DBG("UpdateVolumeRunnable::Run '%s' state %s gen %d locked %d " + "media %d sharing %d", mVolume->NameStr().get(), mVolume->StateStr(), - mVolume->MountGeneration(), (int)mVolume->IsMountLocked()); + mVolume->MountGeneration(), (int)mVolume->IsMountLocked(), + (int)mVolume->IsMediaPresent(), mVolume->IsSharing()); mVolumeService->UpdateVolume(mVolume); mVolumeService = nullptr; @@ -443,9 +450,11 @@ private: void nsVolumeService::UpdateVolumeIOThread(const Volume* aVolume) { - DBG("UpdateVolumeIOThread: Volume '%s' state %s mount '%s' gen %d locked %d", + 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()); + aVolume->MountGeneration(), (int)aVolume->IsMountLocked(), + (int)aVolume->IsMediaPresent(), (int)aVolume->IsSharing()); MOZ_ASSERT(MessageLoop::current() == XRE_GetIOMessageLoop()); NS_DispatchToMainThread(new UpdateVolumeRunnable(this, aVolume)); }