[BlobAsync] Moving async handling into BlobStorageContext & quota out.

storage/browser/blob/blob_async_builder_host.h

Index: storage/browser/blob/blob_async_builder_host.h
diff --git a/storage/browser/blob/blob_async_builder_host.h b/storage/browser/blob/blob_async_builder_host.h
index b7e82fb3dd7dd97b6422b70e301f881dbab93f5d..9619d07380807f8e048698af52cb7f56372c01d5 100644
--- a/storage/browser/blob/blob_async_builder_host.h
+++ b/storage/browser/blob/blob_async_builder_host.h
@@ -22,7 +22,7 @@
 #include "base/memory/weak_ptr.h"
 #include "storage/browser/blob/blob_async_transport_request_builder.h"
 #include "storage/browser/blob/blob_data_builder.h"
-#include "storage/browser/blob/blob_transport_result.h"
+#include "storage/browser/blob/blob_memory_controller.h"
 #include "storage/browser/storage_browser_export.h"
 #include "storage/common/blob_storage/blob_item_bytes_request.h"
 #include "storage/common/blob_storage/blob_item_bytes_response.h"
@@ -37,63 +37,38 @@ namespace storage {
 class BlobDataHandle;
 class BlobStorageContext;
 
-// This class
-// * holds all blobs that are currently being built asynchronously for a child
-//   process,
-// * sends memory requests through the given callback in |StartBuildingBlob|,
-//   and uses the BlobTransportResult return value to signify other results,
-// * includes all logic for deciding which async transport strategy to use, and
-// * handles all blob construction communication with the BlobStorageContext.
-// The method |CancelAll| must be called by the consumer, it is not called on
-// destruction.
+// This class facilitates moving memory from the renderer to the browser.
 class STORAGE_EXPORT BlobAsyncBuilderHost {

[michaeln, 2016/10/28 19:38:29]

maybe call this BlobTransportHost so it more obviously pairs up with
BlobTransportController in the child process

[dmurph, 2016/10/28 22:13:56]

Nice call! Done.

  public:
-  using RequestMemoryCallback = base::Callback<void(
-      std::unique_ptr<std::vector<storage::BlobItemBytesRequest>>,
-      std::unique_ptr<std::vector<base::SharedMemoryHandle>>,
-      std::unique_ptr<std::vector<base::File>>)>;
+  // One is expected to use std::move when calling this callback.
+  using RequestMemoryCallback =
+      base::Callback<void(std::vector<storage::BlobItemBytesRequest>,
+                          std::vector<base::SharedMemoryHandle>,
+                          std::vector<base::File>)>;
+
   BlobAsyncBuilderHost();
   ~BlobAsyncBuilderHost();
 
-  // This registers the given blob internally and adds it to the storage.
-  // Calling this method also guarentees that the referenced blobs are kept
-  // alive for the duration of the construction of this blob.
-  // We return
-  // * BAD_IPC if we already have the blob registered or if we reference ourself
-  //   in the referenced_blob_uuids.
-  // * CANCEL_REFERENCED_BLOB_BROKEN if one of the referenced blobs is broken or
-  //   doesn't exist. We store the blob in the context as broken  with code
-  //   REFERENCED_BLOB_BROKEN.
-  // * DONE if we successfully registered the blob.
-  BlobTransportResult RegisterBlobUUID(
-      const std::string& uuid,
-      const std::string& content_type,
-      const std::string& content_disposition,
-      const std::set<std::string>& referenced_blob_uuids,
-      BlobStorageContext* context);
-
-  // This method begins the construction of the blob given the descriptions. The
-  // blob uuid MUST be building in this object.
-  // When we return:
-  // * DONE: The blob is finished transfering right away, and is now
-  //   successfully saved in the context.
-  // * PENDING_RESPONSES: The async builder host is waiting for responses from
-  //   the renderer. It has called |request_memory| for these responses.
-  // * CANCEL_*: We have to cancel the blob construction. This function clears
-  //   the blob's internal state and marks the blob as broken in the context
-  //   before returning.
-  // * BAD_IPC: The arguments were invalid/bad. This marks the blob as broken in
-  //   the context before returning.
-  BlobTransportResult StartBuildingBlob(
-      const std::string& uuid,
-      const std::vector<DataElement>& elements,
-      size_t memory_available,
-      BlobStorageContext* context,
-      const RequestMemoryCallback& request_memory);
+  // This registers the given blob internally and adds it to the storage with a
+  // refcount of 1. |completion_callback| is called synchronously or
+  // asynchronously with:
+  // * INVALID_CONSTRUCTION_ARGUMENTS if we have invalid input arguments/data.
+  //   We treat this as a critical error, and don't bother registering the blob
+  //   in the BlobStorageContext.
+  // * REFERENCED_BLOB_BROKEN if one of the referenced blobs is broken or
+  //   doesn't exist.
+  // * DONE if we don't need any more data transported and we can clean up.
+  void RegisterBlob(const std::string& uuid,

[michaeln, 2016/10/28 19:38:29]

it might make more sense to name this StartBuildingBlob so its obvious the
CancelBuildingBlob method balances it

[dmurph, 2016/10/28 22:13:56]

Done.

+                    const std::string& content_type,
+                    const std::string& content_disposition,
+                    const std::vector<DataElement>& elements,
+                    BlobStorageContext* context,
+                    const RequestMemoryCallback& request_memory,
+                    const BlobStatusCallback& completion_callback);
 
   // This is called when we have responses from the Renderer to our calls to
   // the request_memory callback above. See above for return value meaning.
-  BlobTransportResult OnMemoryResponses(
+  BlobStatus OnMemoryResponses(
       const std::string& uuid,
       const std::vector<BlobItemBytesResponse>& responses,
       BlobStorageContext* context);
@@ -105,7 +80,7 @@ class STORAGE_EXPORT BlobAsyncBuilderHost {
   // Note: if the blob isn't in the context (renderer dereferenced it before we
   // finished constructing), then we don't bother touching the context.
   void CancelBuildingBlob(const std::string& uuid,
-                          IPCBlobCreationCancelCode code,
+                          BlobStatus code,
                           BlobStorageContext* context);
 
   // This clears this object of pending construction. It also handles marking
@@ -122,30 +97,22 @@ class STORAGE_EXPORT BlobAsyncBuilderHost {
     return async_blob_map_.find(key) != async_blob_map_.end();
   }
 
-  // For testing use only.  Must be called before StartBuildingBlob.
-  void SetMemoryConstantsForTesting(size_t max_ipc_memory_size,
-                                    size_t max_shared_memory_size,
-                                    uint64_t max_file_size) {
-    max_ipc_memory_size_ = max_ipc_memory_size;
-    max_shared_memory_size_ = max_shared_memory_size;
-    max_file_size_ = max_file_size;
-  }
-
  private:
   struct BlobBuildingState {

[michaeln, 2016/10/28 19:38:29]

given the scope and context, this could be a shorter name, maybe State or
TransportState?

[dmurph, 2016/10/28 22:13:56]

Done.

-    // |refernced_blob_handles| should be all handles generated from the set
-    // of |refernced_blob_uuids|.
-    BlobBuildingState(
-        const std::string& uuid,
-        std::set<std::string> referenced_blob_uuids,
-        std::vector<std::unique_ptr<BlobDataHandle>>* referenced_blob_handles);
+    explicit BlobBuildingState(const std::string& uuid);
     ~BlobBuildingState();
 
+    IPCBlobItemRequestStrategy strategy = IPCBlobItemRequestStrategy::UNKNOWN;
     BlobAsyncTransportRequestBuilder request_builder;

[michaeln, 2016/11/07 21:47:04]

shorter naming nit: BlobTransportRequestBuilder 

[dmurph, 2016/11/08 21:19:58]

Done.

     BlobDataBuilder data_builder;
     std::vector<bool> request_received;
-    size_t next_request = 0;
     size_t num_fulfilled_requests = 0;
+
+    RequestMemoryCallback request_memory_callback;
+    BlobStatusCallback completion_callback;
+
+    // Used by shared memory strategy.
+    size_t next_request = 0;
     std::unique_ptr<base::SharedMemory> shared_memory_block;
     // This is the number of requests that have been sent to populate the above
     // shared data. We won't ask for more data in shared memory until all
@@ -154,36 +121,52 @@ class STORAGE_EXPORT BlobAsyncBuilderHost {
     // Only relevant if num_shared_memory_requests is > 0
     size_t current_shared_memory_handle_index = 0;
 
-    // We save these to double check that the RegisterBlob and StartBuildingBlob
-    // messages are in sync.
-    std::set<std::string> referenced_blob_uuids;
-    // These are the blobs that are referenced in the newly constructed blob.
-    // We use these to make sure they stay alive while we create the new blob,
-    // and to wait until any blobs that are not done building are fully
-    // constructed.
-    std::vector<std::unique_ptr<BlobDataHandle>> referenced_blob_handles;
-
-    // These are the number of blobs we're waiting for before we can start
-    // building.
-    size_t num_referenced_blobs_building = 0;
-
-    BlobAsyncBuilderHost::RequestMemoryCallback request_memory_callback;
+    // Used by file strategy.
+    std::vector<scoped_refptr<ShareableFileReference>> files;
   };
 
   typedef std::map<std::string, std::unique_ptr<BlobBuildingState>>
       AsyncBlobMap;
 
+  BlobStatus StartRequests(
+      const std::string& uuid,
+      BlobBuildingState* state,
+      BlobStorageContext* context,
+      std::vector<BlobMemoryController::FileCreationInfo> file_infos);
+
+  void OnCanStartBuildingBlob(

[michaeln, 2016/10/28 19:38:29]

Please line up the names across layers, this is wired to the (too long of a name
to remember right now, involving the term populate) callback

[dmurph, 2016/10/28 22:13:56]

Done.

+      const std::string& uuid,
+      base::WeakPtr<BlobStorageContext> context,
+      BlobStatus status,
+      std::vector<BlobMemoryController::FileCreationInfo> file_infos);
+
+  void SendIPCRequests(BlobBuildingState* state, BlobStorageContext* context);
+  BlobStatus OnIPCResponses(const std::string& uuid,
+                            BlobBuildingState* state,
+                            const std::vector<BlobItemBytesResponse>& responses,
+                            BlobStorageContext* context);
+
   // This is the 'main loop' of our memory requests to the renderer.
-  BlobTransportResult ContinueBlobMemoryRequests(const std::string& uuid,
-                                                 BlobStorageContext* context);
+  BlobStatus ContinueSharedMemoryRequests(const std::string& uuid,
+                                          BlobBuildingState* state,
+                                          BlobStorageContext* context);
 
-  // This is our callback for when we want to finish the blob and we're waiting
-  // for blobs we reference to be built. When the last callback occurs, we
-  // complete the blob and erase our internal state.
-  void ReferencedBlobFinished(const std::string& uuid,
-                              base::WeakPtr<BlobStorageContext> context,
-                              bool construction_success,
-                              IPCBlobCreationCancelCode reason);
+  BlobStatus OnSharedMemoryResponses(
+      const std::string& uuid,
+      BlobBuildingState* state,
+      const std::vector<BlobItemBytesResponse>& responses,
+      BlobStorageContext* context);
+
+  void SendFileRequests(
+      BlobBuildingState* state,
+      BlobStorageContext* context,
+      std::vector<BlobMemoryController::FileCreationInfo> files);
+
+  BlobStatus OnFileResponses(
+      const std::string& uuid,
+      BlobBuildingState* state,
+      const std::vector<BlobItemBytesResponse>& responses,
+      BlobStorageContext* context);
 
   // This finishes creating the blob in the context, decrements blob references
   // that we were holding during construction, and erases our state.
@@ -191,12 +174,6 @@ class STORAGE_EXPORT BlobAsyncBuilderHost {
                           BlobStorageContext* context);
 
   AsyncBlobMap async_blob_map_;
-
-  // Here for testing.
-  size_t max_ipc_memory_size_ = kBlobStorageIPCThresholdBytes;
-  size_t max_shared_memory_size_ = kBlobStorageMaxSharedMemoryBytes;
-  uint64_t max_file_size_ = kBlobStorageMaxFileSizeBytes;
-
   base::WeakPtrFactory<BlobAsyncBuilderHost> ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BlobAsyncBuilderHost);