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

@@ -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;
   }

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

dom/file/FileBlobImpl.cpp

@@ -20,10 +20,10 @@ namespace dom {
 
 FileBlobImpl::FileBlobImpl(nsIFile* aFile)
     : BaseBlobImpl(NS_LITERAL_STRING("FileBlobImpl"), EmptyString(),
-                   EmptyString(), UINT64_MAX,
+                   EmptyString(),
-                   // We pass 0 as lastModified because FileblobImpl has a
+                   // We pass 0 as lastModified and length because FileblobImpl
-                   // different way to compute the value of this attribute
+                   // has a different way to compute those values.
-                   0),
+                   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 and length because FileblobImpl
-                   // We pass 0 as lastModified because FileblobImpl has a
+                   // has a different way to compute those values.
-                   // different way to compute the value of this attribute
+                   0, 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> slicedInputStream =
-      new SlicedInputStream(stream.forget(), mStart, mLength);
+      new SlicedInputStream(stream.forget(), mStart, mLength.value());
   slicedInputStream.forget(aStream);
 }
 

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<nsIFile> mFile;
   nsString mMozFullPath;
+  Maybe<uint64_t> mLength;
   Maybe<int64_t> mLastModified;
   int64_t mFileId;
   bool mWholeFile;

dom/file/MultipartBlobImpl.cpp

@@ -232,7 +232,7 @@ void MultipartBlobImpl::InitializeBlob(const Sequence<Blob::BlobPart>& 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<BlobImpl>& blob = mBlobImpls[index];
 
-#ifdef DEBUG
-    MOZ_ASSERT(!blob->IsSizeUnknown());
-#endif
-
     uint64_t subBlobLength = blob->GetSize(aRv);
     if (NS_WARN_IF(aRv.Failed())) {
       return;

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<RefPtr<BlobImpl>>&& 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<RefPtr<BlobImpl>>&& aBlobImpls,
                     const nsAString& aContentType)
       : BaseBlobImpl(NS_LITERAL_STRING("MultipartBlobImpl"), aContentType,
-                     UINT64_MAX),
+                     MULTIPARTBLOBIMPL_UNKNOWN_LENGTH),
         mBlobImpls(std::move(aBlobImpls)) {}
 
   virtual ~MultipartBlobImpl() = default;