[BlobAsync] Disk support for blob storage

storage/browser/blob/blob_slice.h

Index: storage/browser/blob/blob_slice.h
diff --git a/storage/browser/blob/blob_slice.h b/storage/browser/blob/blob_slice.h
new file mode 100644
index 0000000000000000000000000000000000000000..7d83510a9d4e618a6dfe676d225c13e0154d8173
--- /dev/null
+++ b/storage/browser/blob/blob_slice.h
@@ -0,0 +1,49 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef STORAGE_BROWSER_BLOB_BLOB_SLICE_H_
+#define STORAGE_BROWSER_BLOB_BLOB_SLICE_H_
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "storage/browser/storage_browser_export.h"
+
+namespace storage {
+class InternalBlobData;
+class ShareableBlobDataItem;
+
+// Here we try to handle the cases where the slice partially intersects with
+// either a data item (which we want to copy instead of share when it's partial
+// intersection) or a 'future file item', which isn't populated yet in the
+// source blob so we need to copy it over later when it's completed.

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

Is this used only in BlobFlattener other than in its own unittest?  Again, I'm
worried a bit about having many utility structs just for one-time
computation/operation.  Also.. this struct's name feels somewhat generic 'data
item' type class, while it is rather for doing specific slicing operation in a
certain context.  The reason we have these in separate files are for unit
testing?  Would it make sense to make this a static method that can be called
like:

SliceBlobItems(source, slice_offset, slice_size, &dest_items, &copies,
&copying_memory_size);

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

If you're ok with having 5 output arguments then sure. I was concerned that was
too many output arguments, so I made it a struct to hold them all. We just need
all 5.

[michaeln, 2016/08/15 22:44:42]

I feel similarly about "BlobFlatterner". The addition of these structs to the
top-level namespace add cognitive complexity.

Maybe the BlobFlatterner struct could be nested into the storage context class,
i hadn't noticed the extent of usage for the BlobSlice struct yet. If it can be
removed from the top-level namespace that would help too.

+struct STORAGE_EXPORT BlobSlice {
+  BlobSlice(const InternalBlobData& source,
+            uint64_t slice_offset,
+            uint64_t slice_size);
+  ~BlobSlice();
+
+  // The is the amount of memory we'll be copying from the source blob, as we
+  // can't share a partial data item. This can happen on the first and/or last
+  // item our slice is intersecting.
+  size_t copying_memory_size = 0;
+
+  size_t first_item_slice_offset = 0;
+  // Populated if our first slice item is a temporary item that we'll copy to
+  // later from this |first_source_item|, at offset |first_item_slice_offset|.
+  scoped_refptr<ShareableBlobDataItem> first_source_item;
+  // Populated if our last slice item is a temporary item that we'll copy to
+  // later from this |last_source_item|.
+  scoped_refptr<ShareableBlobDataItem> last_source_item;
+
+  std::vector<scoped_refptr<ShareableBlobDataItem>> dest_items;
+};
+
+}  // namespace storage
+#endif  // STORAGE_BROWSER_BLOB_BLOB_SLICE_H_