Bug 1205028 - Don't tell MTP server about files that it added/modified. r=alchen

With this patch, we introduce mtp-watcher-notify, used to tell device storage
about new files added by MTP, and mtp-watcher-update, used to tell MTP about
device storage changes.

By using 2 new messages, rather than trying to reuse file-watcher-notify and
file-watcher-update, we prevent update "loops".
This commit is contained in:
Dave Hylands 2015-10-13 15:35:11 -07:00
parent 7ad3ddf748
commit 9bce78fbcc
4 changed files with 82 additions and 53 deletions

View File

@ -30,9 +30,27 @@ static const char* kPrefTesting = "device.storage.testing";
static const char* kPrefPromptTesting = "device.storage.prompt.testing";
static const char* kPrefWritableName = "device.storage.writable.name";
// file-watcher-notify comes from some process (but not the MTP Server)
// to indicate that a file has changed. It eventually winds up in the
// parent process, and then gets broadcast out to all child listeners
// as a file-watcher-update and mtp-watcher-update.
//
// mtp-watcher-notify comes from the MTP Server whenever it detects a change
// and this gets rebroadcast as file-watcher-update to the device storage
// listeners.
//
// download-watcher-notify is treated similarly to file-watcher-notify,
// and gets converted into file-watcher-update and mtp-watcher-update.
//
// We need to make sure that the MTP server doesn't get notified about
// files which it told us it added, otherwise it confuses some clients
// (like the Android-File-Transfer program which runs under OS X).
static const char* kFileWatcherUpdate = "file-watcher-update";
static const char* kMtpWatcherUpdate = "mtp-watcher-update";
static const char* kDiskSpaceWatcher = "disk-space-watcher";
static const char* kFileWatcherNotify = "file-watcher-notify";
static const char* kMtpWatcherNotify = "mtp-watcher-notify";
static const char* kDownloadWatcherNotify = "download-watcher-notify";
StaticRefPtr<DeviceStorageStatics> DeviceStorageStatics::sInstance;
@ -100,6 +118,7 @@ DeviceStorageStatics::Init()
if (obs) {
obs->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false);
obs->AddObserver(this, kFileWatcherNotify, false);
obs->AddObserver(this, kMtpWatcherNotify, false);
obs->AddObserver(this, kDownloadWatcherNotify, false);
}
DS_LOG_INFO("");
@ -704,7 +723,8 @@ DeviceStorageStatics::Observe(nsISupports* aSubject,
#endif
dsf = new DeviceStorageFile(NS_LITERAL_STRING(DEVICESTORAGE_SDCARD), volName, path);
} else if (!strcmp(aTopic, kFileWatcherNotify)) {
} else if (!strcmp(aTopic, kFileWatcherNotify) ||
!strcmp(aTopic, kMtpWatcherNotify)) {
dsf = static_cast<DeviceStorageFile*>(aSubject);
} else {
DS_LOG_WARN("unhandled topic '%s'", aTopic);
@ -755,6 +775,11 @@ DeviceStorageStatics::Observe(nsISupports* aSubject,
} else {
obs->NotifyObservers(dsf, kFileWatcherUpdate, aData);
}
if (strcmp(aTopic, kMtpWatcherNotify)) {
// Only send mtp-watcher-updates out if the MTP Server wasn't the one
// telling us about the change.
obs->NotifyObservers(dsf, kMtpWatcherUpdate, aData);
}
return NS_OK;
}

View File

@ -34,6 +34,8 @@ MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE(ScopedCloseDir, PRDir, PR_CloseDir)
BEGIN_MTP_NAMESPACE
static const char* kMtpWatcherNotify = "mtp-watcher-notify";
#if 0
// Some debug code for figuring out deadlocks, if you happen to run into
// that scenario
@ -253,10 +255,10 @@ MozMtpDatabase::UpdateEntry(MtpObjectHandle aHandle, DeviceStorageFile* aFile)
}
class FileWatcherNotifyRunnable final : public nsRunnable
class MtpWatcherNotifyRunnable final : public nsRunnable
{
public:
FileWatcherNotifyRunnable(nsACString& aStorageName,
MtpWatcherNotifyRunnable(nsACString& aStorageName,
nsACString& aPath,
const char* aEventType)
: mStorageName(aStorageName),
@ -277,10 +279,10 @@ public:
NS_ConvertUTF8toUTF16 eventType(mEventType);
nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
MTP_DBG("Sending file-watcher-notify %s %s %s",
MTP_DBG("Sending mtp-watcher-notify %s %s %s",
mEventType.get(), mStorageName.get(), mPath.get());
obs->NotifyObservers(dsf, "file-watcher-notify", eventType.get());
obs->NotifyObservers(dsf, kMtpWatcherNotify, eventType.get());
return NS_OK;
}
@ -290,10 +292,10 @@ private:
nsCString mEventType;
};
// FileWatcherNotify is used to tell DeviceStorage when a file was changed
// MtpWatcherNotify is used to tell DeviceStorage when a file was changed
// through the MTP server.
void
MozMtpDatabase::FileWatcherNotify(DbEntry* aEntry, const char* aEventType)
MozMtpDatabase::MtpWatcherNotify(DbEntry* aEntry, const char* aEventType)
{
// This function gets called from the MozMtpServer::mServerThread
MOZ_ASSERT(!NS_IsMainThread());
@ -320,19 +322,19 @@ MozMtpDatabase::FileWatcherNotify(DbEntry* aEntry, const char* aEventType)
nsAutoCString relPath(Substring(aEntry->mPath,
storageEntry->mStoragePath.Length() + 1));
nsRefPtr<FileWatcherNotifyRunnable> r =
new FileWatcherNotifyRunnable(storageEntry->mStorageName, relPath, aEventType);
nsRefPtr<MtpWatcherNotifyRunnable> r =
new MtpWatcherNotifyRunnable(storageEntry->mStorageName, relPath, aEventType);
DebugOnly<nsresult> rv = NS_DispatchToMainThread(r);
MOZ_ASSERT(NS_SUCCEEDED(rv));
}
// Called to tell the MTP server about new or deleted files,
void
MozMtpDatabase::FileWatcherUpdate(RefCountedMtpServer* aMtpServer,
MozMtpDatabase::MtpWatcherUpdate(RefCountedMtpServer* aMtpServer,
DeviceStorageFile* aFile,
const nsACString& aEventType)
{
// Runs on the FileWatcherUpdate->mIOThread (see MozMtpServer.cpp)
// Runs on the MtpWatcherUpdate->mIOThread (see MozMtpServer.cpp)
MOZ_ASSERT(!NS_IsMainThread());
// Figure out which storage the belongs to (if any)
@ -557,7 +559,7 @@ MozMtpDatabase::StorageArray::index_type
MozMtpDatabase::FindStorage(MtpStorageID aStorageID)
{
// Currently, this routine is called from MozMtpDatabase::RemoveStorage
// and MozMtpDatabase::FileWatcherNotify, which both hold mMutex.
// and MozMtpDatabase::MtpWatcherNotify, which both hold mMutex.
StorageArray::size_type numStorages = mStorage.Length();
StorageArray::index_type storageIndex;
@ -788,7 +790,7 @@ MozMtpDatabase::endSendObject(const char* aPath,
new_times.modtime = entry->mDateModified;
utime(entry->mPath.get(), &new_times);
FileWatcherNotify(entry, "modified");
MtpWatcherNotify(entry, "modified");
}
} else {
RemoveEntry(aHandle);
@ -1422,7 +1424,7 @@ MozMtpDatabase::deleteFile(MtpObjectHandle aHandle)
RemoveEntry(aHandle);
// Tell Device Storage that the file is gone.
FileWatcherNotify(entry, "deleted");
MtpWatcherNotify(entry, "deleted");
return MTP_RESPONSE_OK;
}

View File

@ -115,7 +115,7 @@ public:
void AddStorage(MtpStorageID aStorageID, const char* aPath, const char *aName);
void RemoveStorage(MtpStorageID aStorageID);
void FileWatcherUpdate(RefCountedMtpServer* aMtpServer,
void MtpWatcherUpdate(RefCountedMtpServer* aMtpServer,
DeviceStorageFile* aFile,
const nsACString& aEventType);
@ -270,7 +270,7 @@ private:
StorageArray::index_type FindStorage(MtpStorageID aStorageID);
MtpStorageID FindStorageIDFor(const nsACString& aPath, nsCSubstring& aRemainder);
void FileWatcherNotify(DbEntry* aEntry, const char* aEventType);
void MtpWatcherNotify(DbEntry* aEntry, const char* aEventType);
// We need a mutex to protext mDb and mStorage. The MTP server runs on a
// dedicated thread, and it updates/accesses mDb. When files are updated

View File

@ -39,10 +39,12 @@ using namespace android;
using namespace mozilla;
BEGIN_MTP_NAMESPACE
class FileWatcherUpdateRunnable final : public nsRunnable
static const char* kMtpWatcherUpdate = "mtp-watcher-update";
class MtpWatcherUpdateRunnable final : public nsRunnable
{
public:
FileWatcherUpdateRunnable(MozMtpDatabase* aMozMtpDatabase,
MtpWatcherUpdateRunnable(MozMtpDatabase* aMozMtpDatabase,
RefCountedMtpServer* aMtpServer,
DeviceStorageFile* aFile,
const nsACString& aEventType)
@ -54,10 +56,10 @@ public:
NS_IMETHOD Run()
{
// Runs on the FileWatcherUpdate->mIOThread
// Runs on the MtpWatcherUpdate->mIOThread
MOZ_ASSERT(!NS_IsMainThread());
mMozMtpDatabase->FileWatcherUpdate(mMtpServer, mFile, mEventType);
mMozMtpDatabase->MtpWatcherUpdate(mMtpServer, mFile, mEventType);
return NS_OK;
}
@ -68,24 +70,24 @@ private:
nsCString mEventType;
};
// The FileWatcherUpdate class listens for file-watcher-update events
// and tells the MtpServer about the changes.
class FileWatcherUpdate final : public nsIObserver
// The MtpWatcherUpdate class listens for mtp-watcher-update events
// and tells the MtpServer about changes made in device storage.
class MtpWatcherUpdate final : public nsIObserver
{
public:
NS_DECL_THREADSAFE_ISUPPORTS
FileWatcherUpdate(MozMtpServer* aMozMtpServer)
MtpWatcherUpdate(MozMtpServer* aMozMtpServer)
: mMozMtpServer(aMozMtpServer)
{
MOZ_ASSERT(NS_IsMainThread());
mIOThread = new LazyIdleThread(
DEFAULT_THREAD_TIMEOUT_MS,
NS_LITERAL_CSTRING("MTP FileWatcherUpdate"));
NS_LITERAL_CSTRING("MtpWatcherUpdate"));
nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
obs->AddObserver(this, "file-watcher-update", false);
obs->AddObserver(this, kMtpWatcherUpdate, false);
}
NS_IMETHOD
@ -93,8 +95,8 @@ public:
{
MOZ_ASSERT(NS_IsMainThread());
if (strcmp(aTopic, "file-watcher-update")) {
// We're only interested in file-watcher-update events
if (strcmp(aTopic, kMtpWatcherUpdate)) {
// We're only interested in mtp-watcher-update events
return NS_OK;
}
@ -106,8 +108,8 @@ public:
}
DeviceStorageFile* file = static_cast<DeviceStorageFile*>(aSubject);
file->Dump("file-watcher-update");
MTP_LOG("file-watcher-update: file %s %s",
file->Dump(kMtpWatcherUpdate);
MTP_LOG("%s: file %s %s", kMtpWatcherUpdate,
NS_LossyConvertUTF16toASCII(file->mPath).get(),
eventType.get());
@ -117,33 +119,33 @@ public:
// We're not supposed to perform I/O on the main thread, so punt the
// notification (which will write to /dev/mtp_usb) to an I/O Thread.
nsRefPtr<FileWatcherUpdateRunnable> r =
new FileWatcherUpdateRunnable(mozMtpDatabase, mtpServer, file, eventType);
nsRefPtr<MtpWatcherUpdateRunnable> r =
new MtpWatcherUpdateRunnable(mozMtpDatabase, mtpServer, file, eventType);
mIOThread->Dispatch(r, NS_DISPATCH_NORMAL);
return NS_OK;
}
protected:
~FileWatcherUpdate()
~MtpWatcherUpdate()
{
MOZ_ASSERT(NS_IsMainThread());
nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
obs->RemoveObserver(this, "file-watcher-update");
obs->RemoveObserver(this, kMtpWatcherUpdate);
}
private:
nsRefPtr<MozMtpServer> mMozMtpServer;
nsCOMPtr<nsIThread> mIOThread;
};
NS_IMPL_ISUPPORTS(FileWatcherUpdate, nsIObserver)
static StaticRefPtr<FileWatcherUpdate> sFileWatcherUpdate;
NS_IMPL_ISUPPORTS(MtpWatcherUpdate, nsIObserver)
static StaticRefPtr<MtpWatcherUpdate> sMtpWatcherUpdate;
class AllocFileWatcherUpdateRunnable final : public nsRunnable
class AllocMtpWatcherUpdateRunnable final : public nsRunnable
{
public:
AllocFileWatcherUpdateRunnable(MozMtpServer* aMozMtpServer)
AllocMtpWatcherUpdateRunnable(MozMtpServer* aMozMtpServer)
: mMozMtpServer(aMozMtpServer)
{}
@ -151,17 +153,17 @@ public:
{
MOZ_ASSERT(NS_IsMainThread());
sFileWatcherUpdate = new FileWatcherUpdate(mMozMtpServer);
sMtpWatcherUpdate = new MtpWatcherUpdate(mMozMtpServer);
return NS_OK;
}
private:
nsRefPtr<MozMtpServer> mMozMtpServer;
};
class FreeFileWatcherUpdateRunnable final : public nsRunnable
class FreeMtpWatcherUpdateRunnable final : public nsRunnable
{
public:
FreeFileWatcherUpdateRunnable(MozMtpServer* aMozMtpServer)
FreeMtpWatcherUpdateRunnable(MozMtpServer* aMozMtpServer)
: mMozMtpServer(aMozMtpServer)
{}
@ -169,7 +171,7 @@ public:
{
MOZ_ASSERT(NS_IsMainThread());
sFileWatcherUpdate = nullptr;
sMtpWatcherUpdate = nullptr;
return NS_OK;
}
private:
@ -190,7 +192,7 @@ public:
nsRefPtr<RefCountedMtpServer> server = mMozMtpServer->GetMtpServer();
DebugOnly<nsresult> rv =
NS_DispatchToMainThread(new AllocFileWatcherUpdateRunnable(mMozMtpServer));
NS_DispatchToMainThread(new AllocMtpWatcherUpdateRunnable(mMozMtpServer));
MOZ_ASSERT(NS_SUCCEEDED(rv));
MTP_LOG("MozMtpServer started");
@ -200,7 +202,7 @@ public:
// server->run will have closed the file descriptor.
mMtpUsbFd.forget();
rv = NS_DispatchToMainThread(new FreeFileWatcherUpdateRunnable(mMozMtpServer));
rv = NS_DispatchToMainThread(new FreeMtpWatcherUpdateRunnable(mMozMtpServer));
MOZ_ASSERT(NS_SUCCEEDED(rv));
return NS_OK;