Create blobs from Disk Cache entries.

storage/browser/blob/shareable_file_reference.h

Index: storage/browser/blob/shareable_file_reference.h
diff --git a/storage/browser/blob/shareable_file_reference.h b/storage/browser/blob/shareable_file_reference.h
index e2739dcf8604654772991a11d14239c1c20cba23..eb674ef669f9ff064b11667431a095181a1b731d 100644
--- a/storage/browser/blob/shareable_file_reference.h
+++ b/storage/browser/blob/shareable_file_reference.h
@@ -7,6 +7,7 @@
 
 #include <vector>
 
+#include "storage/browser/blob/blob_data_item.h"
 #include "storage/browser/blob/scoped_file.h"
 #include "storage/browser/storage_browser_export.h"
 
@@ -16,8 +17,7 @@ namespace storage {
 // same path if it already exists in its internal map.
 // This class is non-thread-safe and all methods must be called on a single
 // thread.
-class STORAGE_EXPORT ShareableFileReference
-    : public base::RefCounted<ShareableFileReference> {
+class STORAGE_EXPORT ShareableFileReference : public BlobDataItem::DataHandle {

[dmurph, 2015/06/05 01:17:23]

Can you actually keep this the way it was?  And then have:
* DataHandle no longer be refcounted
* Create these static methods in BlobDataHandle:
    template <typename T>
    DataHandle* CreateDataHandleForResource(scoped_refptr<T> resource)
    template <typename T>
    DataHandle* CreateDataHandleForResource(scoped_ptr<T> resource)
  Both of which use private implementations of DataHandle to wrap those
resources.

This makes the DataHandle a little cleaner as a simple container, and makes it
easy to wrap generic resources that are either refcounted or should be owned by
that element.

Does this make sense?  I'm also available to VC early tomorrow, which might be
easier, as I might be making this more complex than we need.

[gavinp, 2015/06/05 15:10:34]

I'm totally confused. If we are going to allow splicing, how can any kind of
scoped resource be held on to by a DataHandle unless the data handle is itself
ref counted (which is what's in the CL that's uploaded)?? So all DataHandles
will be refcounted. So why not make it refcounted if it is exactly a ref counted
object?

Irrelevant but complicating detail: a disk_cache::ScopedEntryPtr is not
scoped_ptr<disk_cache::Entry>, for awkward and terrible historic reasons.
Instead it's a scoped_ptr<disk_cache::Entry, MagicClassThatDestructsIt>.

I wonder if you're objecting to the layer-removing optimization of making the
ShareableFileReference just be a DataHandle. I considered implementing it as a
container that just contained a scoped_refptr<ShareableFileReference>, but it
seemed silly to have a class which was just a single scoped_refptr<> when that
class exists only to be the target of a scoped_refptr...

I agree on the VC. I'll put one on your calendar for early Friday so we can ref
this hopefully quickly.

  public:
   typedef ScopedFile::ScopeOutCallback FinalReleaseCallback;
 
@@ -60,10 +60,8 @@ class STORAGE_EXPORT ShareableFileReference
   void AddFinalReleaseCallback(const FinalReleaseCallback& callback);
 
  private:
-  friend class base::RefCounted<ShareableFileReference>;
-
   ShareableFileReference(ScopedFile scoped_file);
-  ~ShareableFileReference();
+  ~ShareableFileReference() override;
 
   ScopedFile scoped_file_;