From 8d8aabb99ba84217788b4ae0229ec72294dabe27 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Wed, 4 Mar 2020 21:26:21 +0000 Subject: [PATCH] Bug 1619518 - part 3 - Better File.length attribute handling, r=ssengupta,smaug Differential Revision: https://phabricator.services.mozilla.com/D65165 --HG-- extra : moz-landing-system : lando --- dom/file/BaseBlobImpl.h | 3 --- dom/file/BlobImpl.h | 2 -- dom/file/FileBlobImpl.cpp | 43 ++++++++++++++++++++++------------ dom/file/FileBlobImpl.h | 9 ++++--- dom/file/MultipartBlobImpl.cpp | 6 +---- dom/file/MultipartBlobImpl.h | 9 +++---- 6 files changed, 38 insertions(+), 34 deletions(-) diff --git a/dom/file/BaseBlobImpl.h b/dom/file/BaseBlobImpl.h index b248964bc436..aaf5bc233a0c 100644 --- a/dom/file/BaseBlobImpl.h +++ b/dom/file/BaseBlobImpl.h @@ -54,7 +54,6 @@ class BaseBlobImpl : public BlobImpl { mLength(aLength), mLastModificationDate(0), mSerialNumber(NextSerialNumber()) { - MOZ_ASSERT(aLength != UINT64_MAX, "Must know length when creating slice"); // Ensure non-null mContentType by default mContentType.SetIsVoid(false); } @@ -123,8 +122,6 @@ class BaseBlobImpl : public BlobImpl { virtual bool IsFile() const override { return mIsFile; } - virtual bool IsSizeUnknown() const override { return mLength == UINT64_MAX; } - virtual void GetBlobImplType(nsAString& aBlobImplType) const override { aBlobImplType = mBlobImplType; } diff --git a/dom/file/BlobImpl.h b/dom/file/BlobImpl.h index 8987a9c26854..503beede73b6 100644 --- a/dom/file/BlobImpl.h +++ b/dom/file/BlobImpl.h @@ -93,8 +93,6 @@ class BlobImpl : public nsISupports { virtual bool IsMemoryFile() const = 0; - virtual bool IsSizeUnknown() const = 0; - virtual bool IsFile() const = 0; // Returns true if the BlobImpl is backed by an nsIFile and the underlying diff --git a/dom/file/FileBlobImpl.cpp b/dom/file/FileBlobImpl.cpp index 7d66372b8f78..e9c46b07f1ad 100644 --- a/dom/file/FileBlobImpl.cpp +++ b/dom/file/FileBlobImpl.cpp @@ -20,10 +20,10 @@ namespace dom { FileBlobImpl::FileBlobImpl(nsIFile* aFile) : BaseBlobImpl(NS_LITERAL_STRING("FileBlobImpl"), EmptyString(), - EmptyString(), UINT64_MAX, - // We pass 0 as lastModified because FileblobImpl has a - // different way to compute the value of this attribute - 0), + EmptyString(), + // We pass 0 as lastModified and length because FileblobImpl + // has a different way to compute those values. + 0, 0), mMutex("FileBlobImpl::mMutex"), mFile(aFile), mFileId(-1), @@ -40,12 +40,12 @@ FileBlobImpl::FileBlobImpl(const nsAString& aName, const nsAString& aContentType, uint64_t aLength, nsIFile* aFile) : BaseBlobImpl(NS_LITERAL_STRING("FileBlobImpl"), aName, aContentType, - aLength, - // We pass 0 as lastModified because FileblobImpl has a - // different way to compute the value of this attribute - 0), + // We pass 0 as lastModified and length because FileblobImpl + // has a different way to compute those values. + 0, 0), mMutex("FileBlobImpl::mMutex"), mFile(aFile), + mLength(Some(aLength)), mFileId(-1), mWholeFile(true) { MOZ_ASSERT(mFile, "must have file"); @@ -57,9 +57,13 @@ FileBlobImpl::FileBlobImpl(const nsAString& aName, const nsAString& aContentType, uint64_t aLength, nsIFile* aFile, int64_t aLastModificationDate) : BaseBlobImpl(NS_LITERAL_STRING("FileBlobImpl"), aName, aContentType, - aLength, aLastModificationDate), + // We pass 0 as lastModified and length because FileblobImpl + // has a different way to compute those values. + 0, 0), mMutex("FileBlobImpl::mMutex"), mFile(aFile), + mLength(Some(aLength)), + mLastModified(Some(aLastModificationDate)), mFileId(-1), mWholeFile(true) { MOZ_ASSERT(mFile, "must have file"); @@ -70,7 +74,10 @@ FileBlobImpl::FileBlobImpl(const nsAString& aName, FileBlobImpl::FileBlobImpl(nsIFile* aFile, const nsAString& aName, const nsAString& aContentType, const nsAString& aBlobImplType) - : BaseBlobImpl(aBlobImplType, aName, aContentType, UINT64_MAX, 0), + : BaseBlobImpl(aBlobImplType, aName, aContentType, + // We pass 0 as lastModified and length because FileblobImpl + // has a different way to compute those values. + 0, 0), mMutex("FileBlobImpl::mMutex"), mFile(aFile), mFileId(-1), @@ -88,9 +95,13 @@ FileBlobImpl::FileBlobImpl(nsIFile* aFile, const nsAString& aName, FileBlobImpl::FileBlobImpl(const FileBlobImpl* aOther, uint64_t aStart, uint64_t aLength, const nsAString& aContentType) : BaseBlobImpl(NS_LITERAL_STRING("FileBlobImpl"), aContentType, - aOther->mStart + aStart, aLength), + aOther->mStart + aStart, + // We pass 0 as length because FileblobImpl has a different + // way to compute the value. + 0), mMutex("FileBlobImpl::mMutex"), mFile(aOther->mFile), + mLength(Some(aLength)), mFileId(-1), mWholeFile(false) { MOZ_ASSERT(mFile, "must have file"); @@ -128,7 +139,7 @@ void FileBlobImpl::GetMozFullPathInternal(nsAString& aFilename, uint64_t FileBlobImpl::GetSize(ErrorResult& aRv) { MutexAutoLock lock(mMutex); - if (BaseBlobImpl::IsSizeUnknown()) { + if (mLength.isNothing()) { MOZ_ASSERT(mWholeFile, "Should only use lazy size when using the whole file"); int64_t fileSize; @@ -142,10 +153,10 @@ uint64_t FileBlobImpl::GetSize(ErrorResult& aRv) { return 0; } - mLength = fileSize; + mLength.emplace(fileSize); } - return mLength; + return mLength.value(); } class FileBlobImpl::GetTypeRunnable final : public WorkerMainThreadRunnable { @@ -263,8 +274,10 @@ void FileBlobImpl::CreateInputStream(nsIInputStream** aStream, return; } + MOZ_ASSERT(mLength.isSome()); + RefPtr slicedInputStream = - new SlicedInputStream(stream.forget(), mStart, mLength); + new SlicedInputStream(stream.forget(), mStart, mLength.value()); slicedInputStream.forget(aStream); } diff --git a/dom/file/FileBlobImpl.h b/dom/file/FileBlobImpl.h index b861cdef02a3..778799cf9bc3 100644 --- a/dom/file/FileBlobImpl.h +++ b/dom/file/FileBlobImpl.h @@ -49,13 +49,11 @@ class FileBlobImpl : public BaseBlobImpl { virtual void SetLazyData(const nsAString& aName, const nsAString& aContentType, uint64_t aLength, int64_t aLastModifiedDate) override { - BaseBlobImpl::SetLazyData(aName, aContentType, aLength, 0); + BaseBlobImpl::SetLazyData(aName, aContentType, 0, 0); + mLength.emplace(aLength); mLastModified.emplace(aLastModifiedDate); } - // We always have size for this kind of blob. - virtual bool IsSizeUnknown() const override { return false; } - void SetName(const nsAString& aName) { mName = aName; } void SetType(const nsAString& aType) { mContentType = aType; } @@ -64,7 +62,7 @@ class FileBlobImpl : public BaseBlobImpl { void SetFileId(int64_t aFileId) { mFileId = aFileId; } - void SetEmptySize() { mLength = 0; } + void SetEmptySize() { mLength.emplace(0); } void SetMozFullPath(const nsAString& aPath) { mMozFullPath = aPath; } @@ -98,6 +96,7 @@ class FileBlobImpl : public BaseBlobImpl { nsCOMPtr mFile; nsString mMozFullPath; + Maybe mLength; Maybe mLastModified; int64_t mFileId; bool mWholeFile; diff --git a/dom/file/MultipartBlobImpl.cpp b/dom/file/MultipartBlobImpl.cpp index 0449ba48e15f..e608698098e6 100644 --- a/dom/file/MultipartBlobImpl.cpp +++ b/dom/file/MultipartBlobImpl.cpp @@ -232,7 +232,7 @@ void MultipartBlobImpl::InitializeBlob(const Sequence& aData, } void MultipartBlobImpl::SetLengthAndModifiedDate(ErrorResult& aRv) { - MOZ_ASSERT(mLength == UINT64_MAX); + MOZ_ASSERT(mLength == MULTIPARTBLOBIMPL_UNKNOWN_LENGTH); MOZ_ASSERT_IF(mIsFile, mLastModificationDate == MULTIPARTBLOBIMPL_UNKNOWN_LAST_MODIFIED); @@ -244,10 +244,6 @@ void MultipartBlobImpl::SetLengthAndModifiedDate(ErrorResult& aRv) { index++) { RefPtr& blob = mBlobImpls[index]; -#ifdef DEBUG - MOZ_ASSERT(!blob->IsSizeUnknown()); -#endif - uint64_t subBlobLength = blob->GetSize(aRv); if (NS_WARN_IF(aRv.Failed())) { return; diff --git a/dom/file/MultipartBlobImpl.h b/dom/file/MultipartBlobImpl.h index 02763434e87a..cedc23de04a9 100644 --- a/dom/file/MultipartBlobImpl.h +++ b/dom/file/MultipartBlobImpl.h @@ -19,6 +19,7 @@ namespace dom { // This is just a sentinel value to be sure that we don't call // SetLengthAndModifiedDate more than once. constexpr int64_t MULTIPARTBLOBIMPL_UNKNOWN_LAST_MODIFIED = INT64_MAX; +constexpr uint64_t MULTIPARTBLOBIMPL_UNKNOWN_LENGTH = UINT64_MAX; class MultipartBlobImpl final : public BaseBlobImpl { public: @@ -37,13 +38,13 @@ class MultipartBlobImpl final : public BaseBlobImpl { // Create as a file to be later initialized explicit MultipartBlobImpl(const nsAString& aName) : BaseBlobImpl(NS_LITERAL_STRING("MultipartBlobImpl"), aName, - EmptyString(), UINT64_MAX, + EmptyString(), MULTIPARTBLOBIMPL_UNKNOWN_LENGTH, MULTIPARTBLOBIMPL_UNKNOWN_LAST_MODIFIED) {} // Create as a blob to be later initialized MultipartBlobImpl() : BaseBlobImpl(NS_LITERAL_STRING("MultipartBlobImpl"), EmptyString(), - UINT64_MAX) {} + MULTIPARTBLOBIMPL_UNKNOWN_LENGTH) {} void InitializeBlob(ErrorResult& aRv); @@ -80,7 +81,7 @@ class MultipartBlobImpl final : public BaseBlobImpl { MultipartBlobImpl(nsTArray>&& aBlobImpls, const nsAString& aName, const nsAString& aContentType) : BaseBlobImpl(NS_LITERAL_STRING("MultipartBlobImpl"), aName, - aContentType, UINT64_MAX, + aContentType, MULTIPARTBLOBIMPL_UNKNOWN_LENGTH, MULTIPARTBLOBIMPL_UNKNOWN_LAST_MODIFIED), mBlobImpls(std::move(aBlobImpls)) {} @@ -88,7 +89,7 @@ class MultipartBlobImpl final : public BaseBlobImpl { MultipartBlobImpl(nsTArray>&& aBlobImpls, const nsAString& aContentType) : BaseBlobImpl(NS_LITERAL_STRING("MultipartBlobImpl"), aContentType, - UINT64_MAX), + MULTIPARTBLOBIMPL_UNKNOWN_LENGTH), mBlobImpls(std::move(aBlobImpls)) {} virtual ~MultipartBlobImpl() = default;