From d828d1c6e0c49427fecedc3cdae83f10b7917ae9 Mon Sep 17 00:00:00 2001 From: Jonathan Watt Date: Tue, 8 Oct 2013 18:47:08 +0100 Subject: [PATCH] Bug 923922, part 2 - Allow DirPickerBuildFileListTasks to be cancelled, and cancel any in-progress DirPickerBuildFileListTasks when the user picks a new directory. r=smaug --- content/html/content/src/HTMLInputElement.cpp | 187 ++++++++++++------ content/html/content/src/HTMLInputElement.h | 31 +-- 2 files changed, 133 insertions(+), 85 deletions(-) diff --git a/content/html/content/src/HTMLInputElement.cpp b/content/html/content/src/HTMLInputElement.cpp index 1b8a11586b55..554df0be7b0c 100644 --- a/content/html/content/src/HTMLInputElement.cpp +++ b/content/html/content/src/HTMLInputElement.cpp @@ -477,58 +477,6 @@ private: NS_IMPL_ISUPPORTS1(DirPickerRecursiveFileEnumerator, nsISimpleEnumerator) -class DirPickerFileListBuilderTask MOZ_FINAL - : public nsRunnable -{ -public: - DirPickerFileListBuilderTask(HTMLInputElement* aInput, nsIFile* aTopDir) - : mInput(aInput) - , mTopDir(aTopDir) - {} - - NS_IMETHOD Run() { - if (!NS_IsMainThread()) { - // Build up list of nsDOMFileFile objects on this dedicated thread: - nsCOMPtr iter = - new DirPickerRecursiveFileEnumerator(mTopDir); - bool hasMore = true; - nsCOMPtr tmp; - while (NS_SUCCEEDED(iter->HasMoreElements(&hasMore)) && hasMore) { - iter->GetNext(getter_AddRefs(tmp)); - nsCOMPtr domFile = do_QueryInterface(tmp); - MOZ_ASSERT(domFile); - mFileList.AppendElement(domFile); - mInput->SetFileListProgress(mFileList.Length()); - } - return NS_DispatchToMainThread(this); - } - - // Now back on the main thread, set the list on our HTMLInputElement: - if (mFileList.IsEmpty()) { - return NS_OK; - } - // The text control frame (if there is one) isn't going to send a change - // event because it will think this is done by a script. - // So, we can safely send one by ourself. - mInput->SetFiles(mFileList, true); - mInput->MaybeDispatchProgressEvent(true); // last progress event - nsresult rv = - nsContentUtils::DispatchTrustedEvent(mInput->OwnerDoc(), - static_cast(mInput.get()), - NS_LITERAL_STRING("change"), true, - false); - // Clear mInput to make sure that it can't lose its last strong ref off the - // main thread (which may happen if our dtor runs off the main thread)! - mInput = nullptr; - return rv; - } - -private: - nsRefPtr mInput; - nsCOMPtr mTopDir; - nsTArray > mFileList; -}; - /** * This may return nullptr if aDomFile's implementation of * nsIDOMFile::mozFullPathInternal does not successfully return a non-empty @@ -554,6 +502,108 @@ DOMFileToLocalFile(nsIDOMFile* aDomFile) } // anonymous namespace +class DirPickerFileListBuilderTask MOZ_FINAL + : public nsRunnable +{ +public: + DirPickerFileListBuilderTask(HTMLInputElement* aInput, nsIFile* aTopDir) + : mPreviousFileListLength(0) + , mInput(aInput) + , mTopDir(aTopDir) + , mFileListLength(0) + , mCanceled(0) + {} + + NS_IMETHOD Run() { + if (!NS_IsMainThread()) { + // Build up list of nsDOMFileFile objects on this dedicated thread: + nsCOMPtr iter = + new DirPickerRecursiveFileEnumerator(mTopDir); + bool hasMore = true; + nsCOMPtr tmp; + while (NS_SUCCEEDED(iter->HasMoreElements(&hasMore)) && hasMore) { + iter->GetNext(getter_AddRefs(tmp)); + nsCOMPtr domFile = do_QueryInterface(tmp); + MOZ_ASSERT(domFile); + mFileList.AppendElement(domFile); + mFileListLength = mFileList.Length(); + if (mCanceled) { + NS_ASSERTION(!mInput, "This is bad - how did this happen?"); + // There's no point dispatching to the main thread (that doesn't + // guarantee that we'll be destroyed there). + return NS_OK; + } + } + return NS_DispatchToMainThread(this); + } + + // Now back on the main thread, set the list on our HTMLInputElement: + if (mCanceled || mFileList.IsEmpty()) { + return NS_OK; + } + MOZ_ASSERT(mInput->mDirPickerFileListBuilderTask, + "But we aren't canceled!"); + if (mInput->mProgressTimer) { + mInput->mProgressTimerIsActive = false; + mInput->mProgressTimer->Cancel(); + } + + // The text control frame (if there is one) isn't going to send a change + // event because it will think this is done by a script. + // So, we can safely send one by ourself. + mInput->SetFiles(mFileList, true); + mInput->MaybeDispatchProgressEvent(true); // Last progress event. + mInput->mDirPickerFileListBuilderTask = nullptr; // Now null out. + nsresult rv = + nsContentUtils::DispatchTrustedEvent(mInput->OwnerDoc(), + static_cast(mInput.get()), + NS_LITERAL_STRING("change"), true, + false); + // Clear mInput to make sure that it can't lose its last strong ref off the + // main thread (which may happen if our dtor runs off the main thread)! + mInput = nullptr; + return rv; + } + + void Cancel() + { + MOZ_ASSERT(NS_IsMainThread() && !mCanceled); + // Clear mInput to make sure that it can't lose its last strong ref off the + // main thread (which may happen if our dtor runs off the main thread)! + mInput = nullptr; + mCanceled = 1; // true + } + + uint32_t GetFileListLength() const + { + return mFileListLength; + } + + /** + * The number of files added to the FileList at the time the last progress + * event was fired. + * + * This is only read/set by HTMLInputElement on the main thread. The reason + * that this member is stored here rather than on HTMLInputElement is so that + * we don't increase the size of HTMLInputElement for something that's rarely + * used. + */ + uint32_t mPreviousFileListLength; + +private: + nsRefPtr mInput; + nsCOMPtr mTopDir; + nsTArray > mFileList; + + // We access the list length on both threads, so we need the indirection of + // this atomic member to make the access thread safe: + mozilla::Atomic mFileListLength; + + // We'd prefer this member to be bool, but we don't support Atomic. + mozilla::Atomic mCanceled; +}; + + NS_IMETHODIMP HTMLInputElement::nsFilePickerShownCallback::Done(int16_t aResult) { @@ -561,6 +611,8 @@ HTMLInputElement::nsFilePickerShownCallback::Done(int16_t aResult) return NS_OK; } + mInput->CancelDirectoryPickerScanIfRunning(); + int16_t mode; mFilePicker->GetMode(&mode); @@ -590,14 +642,14 @@ HTMLInputElement::nsFilePickerShownCallback::Done(int16_t aResult) = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID); NS_ASSERTION(target, "Must have stream transport service"); - mInput->ResetProgressCounters(); mInput->StartProgressEventTimer(); // DirPickerFileListBuilderTask takes care of calling SetFiles() and // dispatching the "change" event. - nsRefPtr event = + mInput->mDirPickerFileListBuilderTask = new DirPickerFileListBuilderTask(mInput.get(), pickedDir.get()); - return target->Dispatch(event, NS_DISPATCH_NORMAL); + return target->Dispatch(mInput->mDirPickerFileListBuilderTask, + NS_DISPATCH_NORMAL); } // Collect new selected filenames @@ -1020,8 +1072,6 @@ static nsresult FireEventForAccessibility(nsIDOMHTMLInputElement* aTarget, HTMLInputElement::HTMLInputElement(already_AddRefed aNodeInfo, FromParser aFromParser) : nsGenericHTMLFormElementWithState(aNodeInfo) - , mFileListProgress(0) - , mLastFileListProgress(0) , mType(kInputDefaultType->value) , mDisabledChanged(false) , mValueChanged(false) @@ -2463,6 +2513,20 @@ HTMLInputElement::OpenDirectoryPicker(ErrorResult& aRv) InitFilePicker(FILE_PICKER_DIRECTORY); } +void +HTMLInputElement::CancelDirectoryPickerScanIfRunning() +{ + if (!mDirPickerFileListBuilderTask) { + return; + } + if (mProgressTimer) { + mProgressTimerIsActive = false; + mProgressTimer->Cancel(); + } + mDirPickerFileListBuilderTask->Cancel(); + mDirPickerFileListBuilderTask = nullptr; +} + void HTMLInputElement::StartProgressEventTimer() { @@ -2507,8 +2571,10 @@ HTMLInputElement::MaybeDispatchProgressEvent(bool aFinalProgress) mProgressTimer->Cancel(); } + uint32_t fileListLength = mDirPickerFileListBuilderTask->GetFileListLength(); + if (mProgressTimerIsActive || - mFileListProgress == mLastFileListProgress) { + fileListLength == mDirPickerFileListBuilderTask->mPreviousFileListLength) { return; } @@ -2516,10 +2582,11 @@ HTMLInputElement::MaybeDispatchProgressEvent(bool aFinalProgress) StartProgressEventTimer(); } - mLastFileListProgress = mFileListProgress; + mDirPickerFileListBuilderTask->mPreviousFileListLength = fileListLength; DispatchProgressEvent(NS_LITERAL_STRING(PROGRESS_STR), - false, mLastFileListProgress, + false, + mDirPickerFileListBuilderTask->mPreviousFileListLength, 0); } diff --git a/content/html/content/src/HTMLInputElement.h b/content/html/content/src/HTMLInputElement.h index 49bea99249f0..713cdb408190 100644 --- a/content/html/content/src/HTMLInputElement.h +++ b/content/html/content/src/HTMLInputElement.h @@ -32,6 +32,7 @@ namespace mozilla { namespace dom { class Date; +class DirPickerFileListBuilderTask; class UploadLastDir MOZ_FINAL : public nsIObserver, public nsSupportsWeakReference { public: @@ -87,6 +88,8 @@ class HTMLInputElement MOZ_FINAL : public nsGenericHTMLFormElementWithState, public nsITimerCallback, public nsIConstraintValidation { + friend class DirPickerFileListBuilderTask; + public: using nsIConstraintValidation::GetValidationMessage; using nsIConstraintValidation::CheckValidity; @@ -402,12 +405,8 @@ public: nsDOMFileList* GetFiles(); void OpenDirectoryPicker(ErrorResult& aRv); + void CancelDirectoryPickerScanIfRunning(); - void ResetProgressCounters() - { - mFileListProgress = 0; - mLastFileListProgress = 0; - } void StartProgressEventTimer(); void MaybeDispatchProgressEvent(bool aFinalProgress); void DispatchProgressEvent(const nsAString& aType, @@ -677,13 +676,6 @@ public: // XPCOM GetPhonetic() is OK - void SetFileListProgress(uint32_t mFileCount) - { - MOZ_ASSERT(!NS_IsMainThread(), - "Why are we calling this on the main thread?"); - mFileListProgress = mFileCount; - } - protected: virtual JSObject* WrapNode(JSContext* aCx, JS::Handle aScope) MOZ_OVERRIDE; @@ -1162,6 +1154,8 @@ protected: nsRefPtr mFileList; + nsRefPtr mDirPickerFileListBuilderTask; + nsString mStaticDocFileList; /** @@ -1202,19 +1196,6 @@ protected: // Float value returned by GetStep() when the step attribute is set to 'any'. static const Decimal kStepAny; - /** - * The number of files added to the FileList being built off-main-thread when - * mType == NS_FORM_INPUT_FILE and the user selects a directory. This is set - * off the main thread, read on main thread. - */ - mozilla::Atomic mFileListProgress; - - /** - * The number of files added to the FileList at the time the last progress - * event was fired. - */ - uint32_t mLastFileListProgress; - /** * The type of this input () as an integer. * @see nsIFormControl.h (specifically NS_FORM_INPUT_*)