Bug 1469126 - GetAllocationSize should report unique blobs when dealing with nested blobs, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 18 Jun 2018 11:35:46 -0400
changeset 422832 4a137fd2fcaf910648a4eb61cf9b446be336f76b
parent 422831 a9ead3b8d077094d9e58f8676133114bccdb5ec6
child 422833 9dc030b148fa37e4e0b849c3ca10c98e02963fe7
push id104365
push useramarchesini@mozilla.com
push dateMon, 18 Jun 2018 15:36:14 +0000
treeherdermozilla-inbound@9dc030b148fa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1469126
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1469126 - GetAllocationSize should report unique blobs when dealing with nested blobs, r=smaug
dom/file/BaseBlobImpl.h
dom/file/BlobImpl.h
dom/file/BlobSet.cpp
dom/file/BlobSet.h
dom/file/MemoryBlobImpl.h
dom/file/MultipartBlobImpl.cpp
dom/file/MultipartBlobImpl.h
dom/file/StreamBlobImpl.h
dom/file/StringBlobImpl.h
dom/indexedDB/FileSnapshot.h
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
--- a/dom/file/BaseBlobImpl.h
+++ b/dom/file/BaseBlobImpl.h
@@ -97,16 +97,21 @@ public:
 
   virtual void GetType(nsAString& aType) override;
 
   size_t GetAllocationSize() const override
   {
     return 0;
   }
 
+  size_t GetAllocationSize(FallibleTArray<BlobImpl*>& aVisitedBlobImpls) const override
+  {
+    return GetAllocationSize();
+  }
+
   virtual uint64_t GetSerialNumber() const override { return mSerialNumber; }
 
   virtual already_AddRefed<BlobImpl>
   CreateSlice(uint64_t aStart, uint64_t aLength,
               const nsAString& aContentType, ErrorResult& aRv) override
   {
     return nullptr;
   }
--- a/dom/file/BlobImpl.h
+++ b/dom/file/BlobImpl.h
@@ -47,16 +47,17 @@ public:
 
   virtual void GetMozFullPathInternal(nsAString& aFileName, ErrorResult& aRv) const = 0;
 
   virtual uint64_t GetSize(ErrorResult& aRv) = 0;
 
   virtual void GetType(nsAString& aType) = 0;
 
   virtual size_t GetAllocationSize() const = 0;
+  virtual size_t GetAllocationSize(FallibleTArray<BlobImpl*>& aVisitedBlobImpls) const  = 0;
 
   /**
    * An effectively-unique serial number identifying this instance of FileImpl.
    *
    * Implementations should obtain a serial number from
    * FileImplBase::NextSerialNumber().
    */
   virtual uint64_t GetSerialNumber() const = 0;
--- a/dom/file/BlobSet.cpp
+++ b/dom/file/BlobSet.cpp
@@ -24,19 +24,17 @@ BlobSet::AppendVoidPtr(const void* aData
   void* data = malloc(aLength);
   if (!data) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   memcpy((char*)data, aData, aLength);
 
   RefPtr<BlobImpl> blobImpl = new MemoryBlobImpl(data, aLength, EmptyString());
-  mBlobImpls.AppendElement(blobImpl);
-
-  return NS_OK;
+  return AppendBlobImpl(blobImpl);
 }
 
 nsresult
 BlobSet::AppendString(const nsAString& aString, bool nativeEOL)
 {
   nsCString utf8Str;
   if (NS_WARN_IF(!AppendUTF16toUTF8(aString, utf8Str, mozilla::fallible))) {
     return NS_ERROR_OUT_OF_MEMORY;
@@ -56,14 +54,16 @@ BlobSet::AppendString(const nsAString& a
     StringBlobImpl::Create(utf8Str, EmptyString());
   return AppendBlobImpl(blobImpl);
 }
 
 nsresult
 BlobSet::AppendBlobImpl(BlobImpl* aBlobImpl)
 {
   NS_ENSURE_ARG_POINTER(aBlobImpl);
-  mBlobImpls.AppendElement(aBlobImpl);
+  if (NS_WARN_IF(!mBlobImpls.AppendElement(aBlobImpl, fallible))) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
   return NS_OK;
 }
 
 } // dom namespace
 } // mozilla namespace
--- a/dom/file/BlobSet.h
+++ b/dom/file/BlobSet.h
@@ -15,24 +15,24 @@
 namespace mozilla {
 namespace dom {
 
 class BlobImpl;
 
 class BlobSet final
 {
 public:
-  nsresult AppendVoidPtr(const void* aData, uint32_t aLength);
+  MOZ_MUST_USE nsresult AppendVoidPtr(const void* aData, uint32_t aLength);
 
-  nsresult AppendString(const nsAString& aString, bool nativeEOL);
+  MOZ_MUST_USE nsresult AppendString(const nsAString& aString, bool nativeEOL);
 
-  nsresult AppendBlobImpl(BlobImpl* aBlobImpl);
+  MOZ_MUST_USE nsresult AppendBlobImpl(BlobImpl* aBlobImpl);
 
-  nsTArray<RefPtr<BlobImpl>>& GetBlobImpls() { return mBlobImpls; }
+  FallibleTArray<RefPtr<BlobImpl>>& GetBlobImpls() { return mBlobImpls; }
 
 private:
-  nsTArray<RefPtr<BlobImpl>> mBlobImpls;
+  FallibleTArray<RefPtr<BlobImpl>> mBlobImpls;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_BlobSet_h
--- a/dom/file/MemoryBlobImpl.h
+++ b/dom/file/MemoryBlobImpl.h
@@ -53,16 +53,21 @@ public:
     return true;
   }
 
   size_t GetAllocationSize() const override
   {
     return mLength;
   }
 
+  size_t GetAllocationSize(FallibleTArray<BlobImpl*>& aVisitedBlobImpls) const override
+  {
+    return GetAllocationSize();
+  }
+
   class DataOwner final : public mozilla::LinkedListElement<DataOwner>
   {
   public:
     NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DataOwner)
     DataOwner(void* aMemoryBuffer, uint64_t aLength)
       : mData(aMemoryBuffer)
       , mLength(aLength)
     {
--- a/dom/file/MultipartBlobImpl.cpp
+++ b/dom/file/MultipartBlobImpl.cpp
@@ -177,17 +177,20 @@ MultipartBlobImpl::InitializeBlob(const 
   mContentType = aContentType;
   BlobSet blobSet;
 
   for (uint32_t i = 0, len = aData.Length(); i < len; ++i) {
     const Blob::BlobPart& data = aData[i];
 
     if (data.IsBlob()) {
       RefPtr<Blob> blob = data.GetAsBlob().get();
-      blobSet.AppendBlobImpl(blob->Impl());
+      aRv = blobSet.AppendBlobImpl(blob->Impl());
+      if (aRv.Failed()) {
+        return;
+      }
     }
 
     else if (data.IsUSVString()) {
       aRv = blobSet.AppendString(data.GetAsUSVString(), aNativeEOL);
       if (aRv.Failed()) {
         return;
       }
     }
@@ -378,17 +381,21 @@ MultipartBlobImpl::InitializeChromeFile(
   }
 
   // XXXkhuey this is terrible
   if (mContentType.IsEmpty()) {
     blob->GetType(mContentType);
   }
 
   BlobSet blobSet;
-  blobSet.AppendBlobImpl(static_cast<File*>(blob.get())->Impl());
+  rv = blobSet.AppendBlobImpl(static_cast<File*>(blob.get())->Impl());
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
   mBlobImpls = blobSet.GetBlobImpls();
 
   SetLengthAndModifiedDate(error);
   if (NS_WARN_IF(error.Failed())) {
     return error.StealNSResult();
   }
 
   if (aLastModifiedPassed) {
@@ -407,15 +414,36 @@ MultipartBlobImpl::MayBeClonedToOtherThr
     }
   }
 
   return true;
 }
 
 size_t MultipartBlobImpl::GetAllocationSize() const
 {
+  FallibleTArray<BlobImpl*> visitedBlobs;
+
+  // We want to report the unique blob allocation, avoiding duplicated blobs in
+  // the multipart blob tree.
   size_t total = 0;
   for (uint32_t i = 0; i < mBlobImpls.Length(); ++i) {
-    total += mBlobImpls[i]->GetAllocationSize();
+    total += mBlobImpls[i]->GetAllocationSize(visitedBlobs);
   }
 
   return total;
 }
+
+size_t MultipartBlobImpl::GetAllocationSize(FallibleTArray<BlobImpl*>& aVisitedBlobs) const
+{
+  FallibleTArray<BlobImpl*> visitedBlobs;
+
+  size_t total = 0;
+  for (BlobImpl* blobImpl : mBlobImpls) {
+    if (!aVisitedBlobs.Contains(blobImpl)) {
+      if (NS_WARN_IF(!aVisitedBlobs.AppendElement(blobImpl, fallible))) {
+        return 0;
+      }
+      total += blobImpl->GetAllocationSize(aVisitedBlobs);
+    }
+  }
+
+  return total;
+}
--- a/dom/file/MultipartBlobImpl.h
+++ b/dom/file/MultipartBlobImpl.h
@@ -88,16 +88,17 @@ public:
   void SetName(const nsAString& aName)
   {
     mName = aName;
   }
 
   virtual bool MayBeClonedToOtherThreads() const override;
 
   size_t GetAllocationSize() const override;
+  size_t GetAllocationSize(FallibleTArray<BlobImpl*>& aVisitedBlobImpls) const override;
 
 protected:
   MultipartBlobImpl(nsTArray<RefPtr<BlobImpl>>&& aBlobImpls,
                     const nsAString& aName,
                     const nsAString& aContentType)
     : BaseBlobImpl(aName, aContentType, UINT64_MAX),
       mBlobImpls(std::move(aBlobImpls)),
       mIsFromNsIFile(false)
--- a/dom/file/StreamBlobImpl.h
+++ b/dom/file/StreamBlobImpl.h
@@ -74,16 +74,21 @@ public:
 
   bool IsDirectory() const override
   {
     return mIsDirectory;
   }
 
   size_t GetAllocationSize() const override;
 
+  size_t GetAllocationSize(FallibleTArray<BlobImpl*>& aVisitedBlobImpls) const override
+  {
+    return GetAllocationSize();
+  }
+
 private:
   StreamBlobImpl(already_AddRefed<nsIInputStream> aInputStream,
                  const nsAString& aContentType,
                  uint64_t aLength);
 
   StreamBlobImpl(already_AddRefed<nsIInputStream> aInputStream,
                  const nsAString& aName,
                  const nsAString& aContentType,
--- a/dom/file/StringBlobImpl.h
+++ b/dom/file/StringBlobImpl.h
@@ -32,16 +32,21 @@ public:
   CreateSlice(uint64_t aStart, uint64_t aLength,
               const nsAString& aContentType, ErrorResult& aRv) override;
 
   size_t GetAllocationSize() const override
   {
     return mData.Length();
   }
 
+  size_t GetAllocationSize(FallibleTArray<BlobImpl*>& aVisitedBlobImpls) const override
+  {
+    return GetAllocationSize();
+  }
+
 private:
   StringBlobImpl(const nsACString& aData, const nsAString& aContentType);
 
   ~StringBlobImpl();
 
   nsCString mData;
 };
 
--- a/dom/indexedDB/FileSnapshot.h
+++ b/dom/indexedDB/FileSnapshot.h
@@ -109,16 +109,22 @@ private:
   }
 
   size_t
   GetAllocationSize() const override
   {
     return mBlobImpl->GetAllocationSize();
   }
 
+  size_t
+  GetAllocationSize(FallibleTArray<BlobImpl*>& aVisitedBlobs) const override
+  {
+    return mBlobImpl->GetAllocationSize(aVisitedBlobs);
+  }
+
   virtual uint64_t
   GetSerialNumber() const override
   {
     return mBlobImpl->GetSerialNumber();
   }
 
   virtual already_AddRefed<BlobImpl>
   CreateSlice(uint64_t aStart,
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -8,17 +8,16 @@
 
 #include <algorithm>
 #ifndef XP_WIN
 #include <unistd.h>
 #endif
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/CheckedInt.h"
 #include "mozilla/dom/BlobBinding.h"
-#include "mozilla/dom/BlobSet.h"
 #include "mozilla/dom/DocGroup.h"
 #include "mozilla/dom/DOMString.h"
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/FileBinding.h"
 #include "mozilla/dom/FileCreatorHelper.h"
 #include "mozilla/dom/FetchUtil.h"
 #include "mozilla/dom/FormData.h"
 #include "mozilla/dom/MutableBlobStorage.h"
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -56,17 +56,16 @@
 
 class nsIJARChannel;
 class nsILoadGroup;
 class nsIJSID;
 
 namespace mozilla {
 namespace dom {
 
-class BlobSet;
 class DOMString;
 class XMLHttpRequestUpload;
 struct OriginAttributesDictionary;
 
 // A helper for building up an ArrayBuffer object's data
 // before creating the ArrayBuffer itself.  Will do doubling
 // based reallocation, up to an optional maximum growth given.
 //