[BlobAsync] Disk support for blob storage

storage/browser/blob/blob_storage_context.h

Index: storage/browser/blob/blob_storage_context.h
diff --git a/storage/browser/blob/blob_storage_context.h b/storage/browser/blob/blob_storage_context.h
index 8553d0266d2d8b6338a176f4e9d9ce9d7675fa85..b800d32b045e27d705b87e4e63dff935e5720398 100644
--- a/storage/browser/blob/blob_storage_context.h
+++ b/storage/browser/blob/blob_storage_context.h
@@ -19,17 +19,17 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "storage/browser/blob/blob_data_handle.h"
+#include "storage/browser/blob/blob_memory_controller.h"
 #include "storage/browser/blob/blob_storage_registry.h"
-#include "storage/browser/blob/internal_blob_data.h"
 #include "storage/browser/storage_browser_export.h"
 #include "storage/common/blob_storage/blob_storage_constants.h"
-#include "storage/common/data_element.h"
 
 class GURL;
 
 namespace base {
 class FilePath;
 class Time;
+class TaskRunner;
 }
 
 namespace content {
@@ -40,6 +40,7 @@ class BlobDispatcherHostTest;
 namespace storage {
 
 class BlobDataBuilder;
+class BlobDataHandle;
 class BlobDataItem;
 class BlobDataSnapshot;
 class ShareableBlobDataItem;
@@ -57,8 +58,8 @@ class STORAGE_EXPORT BlobStorageContext
   std::unique_ptr<BlobDataHandle> GetBlobDataFromUUID(const std::string& uuid);
   std::unique_ptr<BlobDataHandle> GetBlobDataFromPublicURL(const GURL& url);
 
-  // Useful for coining blobs from within the browser process. If the
-  // blob cannot be added due to memory consumption, returns NULL.
+  // Useful for coining blobs from within the browser process. Use the
+  // BlobDataHandle::GetBlobStatus call to check if there was an error.
   // A builder should not be used twice to create blobs, as the internal

[kinuko, 2016/07/17 16:15:47]

This comment about builder should probably be mentioned in blob_data_builder.h
instead?

[dmurph, 2016/07/19 02:26:28]

Done.

   // resources are refcounted instead of copied for memory efficiency.
   // To cleanly use a builder multiple times, please call Clone() on the
@@ -71,30 +72,66 @@ class STORAGE_EXPORT BlobStorageContext
   std::unique_ptr<BlobDataHandle> AddFinishedBlob(
       const BlobDataBuilder* builder);
 
+  std::unique_ptr<BlobDataHandle> AddBrokenBlob(
+      const std::string& uuid,
+      const std::string& content_type,
+      const std::string& content_disposition,
+      BlobStatus reason);
+
   // Useful for coining blob urls from within the browser process.
   bool RegisterPublicBlobURL(const GURL& url, const std::string& uuid);
   void RevokePublicBlobURL(const GURL& url);
 
-  size_t memory_usage() const { return memory_usage_; }
   size_t blob_count() const { return registry_.blob_count(); }
-  size_t memory_available() const {
-    return kBlobStorageMaxMemoryUsage - memory_usage_;
-  }
 
   const BlobStorageRegistry& registry() { return registry_; }
 
+  void EnableDisk(const base::FilePath& storage_directory,
+                  scoped_refptr<base::TaskRunner> file_runner);
+
+  // If there is data present that needs further population then we will call

[kinuko, 2016/07/17 16:15:47]

I think the method comment should start with what the method is supposed to do.

[dmurph, 2016/07/19 02:26:28]

Done.

+  // |can_populate_memory| when we're ready for the user data to be populated
+  // with the PENDING_DATA_POPULATION status. This can happen synchronously or
+  // asynchronously. Otherwise |can_populate_memory| should be null.
+  // In the further population case, the caller must call either
+  // FinishedPopulatingPendingBlob or BreakAndFinishPendingBlob after
+  // |can_populate_memory| is called to signify the data is finished populating
+  // or an error occurred (respectively).
+  // If the returned handle is broken, then the possible error cases are:
+  // * OUT_OF_MEMORY if we don't have enough memory to store the blob,
+  // * REFERENCED_BLOB_BROKEN if a referenced blob is broken or we're
+  //   referencing ourself.
+  std::unique_ptr<BlobDataHandle> BuildBlob(
+      const BlobDataBuilder& content,
+      const BlobStatusCallback& can_populate_memory);
+
+  // Note: Any callbacks waiting on this blob, including the
+  // |can_populate_memory| callback given to BuildBlob, will be called with this
+  // status code.
+  void BreakAndFinishPendingBlob(const std::string& uuid, BlobStatus code);
+
+  // This notifies the construction system that the unpopulated data in the
+  // given blob has been populated. Caller must have all pending items populated
+  // in the original builder |content| given in BuildBlob or we'll check-fail.
+  void FinishedPopulatingPendingBlob(const std::string& uuid);
+
+  const BlobMemoryController& memory_controller() { return memory_controller_; }
+
  private:
   using BlobRegistryEntry = BlobStorageRegistry::Entry;
-  using BlobConstructedCallback = BlobStorageRegistry::BlobConstructedCallback;
   friend class content::BlobDispatcherHost;
+  friend class content::BlobDispatcherHostTest;
   friend class BlobAsyncBuilderHost;
   friend class BlobAsyncBuilderHostTest;
   friend class BlobDataHandle;
   friend class BlobDataHandle::BlobDataHandleShared;
+  friend class BlobFlattenerTest;
   friend class BlobReaderTest;
   FRIEND_TEST_ALL_PREFIXES(BlobReaderTest, HandleBeforeAsyncCancel);
   FRIEND_TEST_ALL_PREFIXES(BlobReaderTest, ReadFromIncompleteBlob);
   friend class BlobStorageContextTest;
+  FRIEND_TEST_ALL_PREFIXES(BlobStorageContextTest, BuildBlobAsync);
+  FRIEND_TEST_ALL_PREFIXES(BlobStorageContextTest, BuildBlobAndCancel);
   FRIEND_TEST_ALL_PREFIXES(BlobStorageContextTest, IncrementDecrementRef);
   FRIEND_TEST_ALL_PREFIXES(BlobStorageContextTest, OnCancelBuildingBlob);
   FRIEND_TEST_ALL_PREFIXES(BlobStorageContextTest, PublicBlobUrls);
@@ -102,18 +139,23 @@ class STORAGE_EXPORT BlobStorageContext
                            TestUnknownBrokenAndBuildingBlobReference);
   friend class ViewBlobInternalsJob;

[kinuko, 2016/07/17 16:15:47]

Can we list up non-test friend classes first and then test ones next?

We're having so many non-test friend classes, should we maybe review if we need
all these friends / some private methods should be public?

[dmurph, 2016/07/19 02:26:28]

Good idea. I'll explore this in the next patch, need to head home now.

 
-  // CompletePendingBlob or CancelPendingBlob should be called after this.
-  void CreatePendingBlob(const std::string& uuid,
-                         const std::string& content_type,
-                         const std::string& content_disposition);
+  BlobMemoryController* mutable_memory_controller() {
+    return &memory_controller_;
+  }
+
+  std::unique_ptr<BlobDataHandle> CreateHandle(const std::string& uuid,
+                                               BlobRegistryEntry* entry);
 
-  // This includes resolving blob references in the builder. This will run the
-  // callbacks given in RunOnConstructionComplete.
-  void CompletePendingBlob(const BlobDataBuilder& external_builder);
+  bool CanFinishBuilding(BlobRegistryEntry* entry);
 
-  // This will run the callbacks given in RunOnConstructionComplete.
-  void CancelPendingBlob(const std::string& uuid,
-                         IPCBlobCreationCancelCode reason);
+  void FinishBuilding(BlobRegistryEntry* entry);
+
+  void OnEnoughSizeForBlobData(const std::string& uuid, bool can_fit);
+
+  void OnDependentBlobFinished(const std::string& owning_blob_uuid,
+                               BlobStatus reason);
+
+  void ClearAndFreeMemory(const std::string& uuid, BlobRegistryEntry* entry);
 
   void IncrementBlobRefCount(const std::string& uuid);
   void DecrementBlobRefCount(const std::string& uuid);
@@ -123,45 +165,15 @@ class STORAGE_EXPORT BlobStorageContext
   // TODO(dmurph): After we make the snapshot method in BlobHandle private, then
   // make this DCHECK on the blob not being complete.
   std::unique_ptr<BlobDataSnapshot> CreateSnapshot(const std::string& uuid);
-  bool IsBroken(const std::string& uuid) const;
-  bool IsBeingBuilt(const std::string& uuid) const;
+  BlobStatus GetBlobStatus(const std::string& uuid) const;
   // Runs |done| when construction completes, with true if it was successful,
   // and false if there was an error, which is reported in the second argument
   // of the callback.
   void RunOnConstructionComplete(const std::string& uuid,
-                                 const BlobConstructedCallback& done);
-
-  // Appends the given blob item to the blob builder.  The new blob
-  // retains ownership of data_item if applicable, and returns false if there
-  // was an error and pouplates the error_code.  We can either have an error of:
-  // OUT_OF_MEMORY: We are out of memory to store this blob.
-  // REFERENCED_BLOB_BROKEN: One of the referenced blobs is broken.
-  bool AppendAllocatedBlobItem(const std::string& target_blob_uuid,
-                               scoped_refptr<BlobDataItem> data_item,
-                               InternalBlobData::Builder* target_blob_data,
-                               IPCBlobCreationCancelCode* error_code);
-
-  // Allocates a shareable blob data item, with blob references resolved.  If
-  // there isn't enough memory, then a nullptr is returned.
-  scoped_refptr<ShareableBlobDataItem> AllocateShareableBlobDataItem(
-      const std::string& target_blob_uuid,
-      scoped_refptr<BlobDataItem> data_item);
-
-  // Deconstructs the blob and appends it's contents to the target blob.  Items
-  // are shared if possible, and copied if the given offset and length
-  // have to split an item.
-  bool AppendBlob(const std::string& target_blob_uuid,
-                  const InternalBlobData& blob,
-                  uint64_t offset,
-                  uint64_t length,
-                  InternalBlobData::Builder* target_blob_data);
+                                 const BlobStatusCallback& done);
 
   BlobStorageRegistry registry_;
-
-  // Used to keep track of how much memory is being utilized for blob data,
-  // we count only the items of TYPE_DATA which are held in memory and not
-  // items of TYPE_FILE.
-  size_t memory_usage_;
+  BlobMemoryController memory_controller_;
 
   DISALLOW_COPY_AND_ASSIGN(BlobStorageContext);
 };