storage: shareable file ref should not be a data handle

This is just cleanup in preparation for larger data handle changes.
There's no need for ShareableFileReference to subclass
BlobDataItem::DataHandle as it doesn't implement any of its interfaces.
Its goal is to preserve the lifetime of the ScopedFile, and it's clearer
to do this by holding onto the ShareableFileReference in BlobDataItem
directly.

Bug: 1012869
Change-Id: Ieaad64adede4358d734aff8e3cc21bd9038566cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1850198
Commit-Queue: enne <enne@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Auto-Submit: enne <enne@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704357}

storage/browser/blob/blob_builder_from_stream.cc

@@ -495,8 +495,7 @@ void BlobBuilderFromStream::AllocateMoreFileSpace(
       items_.back()->item()->length() < kMaxFileSize) {
     auto item = items_.back()->item();
     uint64_t old_file_size = item->length();
-    scoped_refptr<ShareableFileReference> file_reference =
-        static_cast<ShareableFileReference*>(item->data_handle());
+    scoped_refptr<ShareableFileReference> file_reference = item->file_ref_;
     DCHECK(file_reference);
     auto file_size_delta = std::min(kMaxFileSize - old_file_size, length_hint);
     context_->mutable_memory_controller()->GrowFileAllocation(
@@ -618,7 +617,7 @@ void BlobBuilderFromStream::DidWriteToExtendedFile(
   DCHECK(!items_.empty());
   auto item = items_.back()->item();
   DCHECK_EQ(item->type(), BlobDataItem::Type::kFile);
-  DCHECK_EQ(item->data_handle(), file_reference.get());
+  DCHECK_EQ(item->file_ref_, file_reference.get());
 
   item->SetFileModificationTime(modification_time);
   current_total_size_ += bytes_written;

storage/browser/blob/blob_data_builder.cc

@@ -281,8 +281,7 @@ void BlobDataBuilder::SliceBlob(const BlobEntry* source,
       case BlobDataItem::Type::kFile: {
         data_item = BlobDataItem::CreateFile(
             source_item->path(), source_item->offset() + item_offset, read_size,
-            source_item->expected_modification_time(),
-            source_item->data_handle_);
+            source_item->expected_modification_time(), source_item->file_ref_);
 
         if (source_item->IsFutureFileItem()) {
           // The source file isn't a real file yet (path is fake), so store the

storage/browser/blob/blob_data_item.cc

@@ -78,12 +78,12 @@ scoped_refptr<BlobDataItem> BlobDataItem::CreateFile(
     uint64_t offset,
     uint64_t length,
     base::Time expected_modification_time,
-    scoped_refptr<DataHandle> data_handle) {
+    scoped_refptr<ShareableFileReference> file_ref) {
   auto item =
       base::WrapRefCounted(new BlobDataItem(Type::kFile, offset, length));
   item->path_ = std::move(path);
   item->expected_modification_time_ = std::move(expected_modification_time);
-  item->data_handle_ = std::move(data_handle);
+  item->file_ref_ = std::move(file_ref);
   // TODO(mek): DCHECK(!item->IsFutureFileItem()) when BlobDataBuilder has some
   // other way of slicing a future file.
   return item;
@@ -171,14 +171,15 @@ void BlobDataItem::ShrinkBytes(size_t new_length) {
   bytes_.resize(length_);
 }
 
-void BlobDataItem::PopulateFile(base::FilePath path,
-                                base::Time expected_modification_time,
-                                scoped_refptr<DataHandle> data_handle) {
+void BlobDataItem::PopulateFile(
+    base::FilePath path,
+    base::Time expected_modification_time,
+    scoped_refptr<ShareableFileReference> file_ref) {
   DCHECK_EQ(type_, Type::kFile);
   DCHECK(IsFutureFileItem());
   path_ = std::move(path);
   expected_modification_time_ = std::move(expected_modification_time);
-  data_handle_ = std::move(data_handle);
+  file_ref_ = std::move(file_ref);
 }
 
 void BlobDataItem::ShrinkFile(uint64_t new_length) {

storage/browser/blob/blob_data_item.h

@@ -16,6 +16,7 @@
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "net/base/io_buffer.h"
+#include "storage/browser/blob/shareable_file_reference.h"
 #include "url/gurl.h"
 
 namespace storage {
@@ -102,7 +103,7 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobDataItem
       uint64_t offset,
       uint64_t length,
       base::Time expected_modification_time = base::Time(),
-      scoped_refptr<DataHandle> data_handle = nullptr);
+      scoped_refptr<ShareableFileReference> file_ref = nullptr);
   static scoped_refptr<BlobDataItem> CreateFutureFile(uint64_t offset,
                                                       uint64_t length,
                                                       uint64_t file_id);
@@ -148,8 +149,7 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobDataItem
   }
 
   DataHandle* data_handle() const {
-    DCHECK(type_ == Type::kFile || type_ == Type::kReadableDataHandle)
-        << static_cast<int>(type_);
+    DCHECK_EQ(type_, Type::kReadableDataHandle) << static_cast<int>(type_);
     return data_handle_.get();
   }
 
@@ -180,7 +180,7 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobDataItem
 
   void PopulateFile(base::FilePath path,
                     base::Time expected_modification_time,
-                    scoped_refptr<DataHandle> data_handle);
+                    scoped_refptr<ShareableFileReference> file_ref);
   void ShrinkFile(uint64_t new_length);
   void GrowFile(uint64_t new_length);
   void SetFileModificationTime(base::Time time) {
@@ -198,8 +198,8 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobDataItem
   base::Time
       expected_modification_time_;  // For Type::kFile and kFileFilesystem.
 
-  scoped_refptr<DataHandle>
-      data_handle_;  // For Type::kFile and kReadableDataHandle.
+  scoped_refptr<DataHandle> data_handle_;           // For kReadableDataHandle.
+  scoped_refptr<ShareableFileReference> file_ref_;  // For Type::kFile
 
   scoped_refptr<FileSystemContext>
       file_system_context_;  // For Type::kFileFilesystem.

storage/browser/blob/blob_storage_context.cc

@@ -520,7 +520,7 @@ void BlobStorageContext::FinishBuilding(BlobEntry* entry) {
               source_item->path(),
               source_item->offset() + copy.source_item_offset, dest_size,
               source_item->expected_modification_time(),
-              source_item->data_handle_);
+              source_item->file_ref_);
           copy.dest_item->set_item(std::move(new_item));
           break;
         }

storage/browser/blob/shareable_file_reference.h

@@ -7,7 +7,6 @@
 
 #include "base/component_export.h"
 #include "base/macros.h"
-#include "storage/browser/blob/blob_data_item.h"
 #include "storage/browser/blob/scoped_file.h"
 
 namespace storage {
@@ -17,7 +16,7 @@ namespace storage {
 // This class is non-thread-safe and all methods must be called on a single
 // thread.
 class COMPONENT_EXPORT(STORAGE_BROWSER) ShareableFileReference
-    : public BlobDataItem::DataHandle {
+    : public base::RefCounted<ShareableFileReference> {
  public:
   using FinalReleaseCallback = ScopedFile::ScopeOutCallback;
 
@@ -64,8 +63,10 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) ShareableFileReference
   void AddFinalReleaseCallback(FinalReleaseCallback callback);
 
  private:
+  friend class base::RefCounted<ShareableFileReference>;
+
   ShareableFileReference(ScopedFile scoped_file);
-  ~ShareableFileReference() override;
+  ~ShareableFileReference();
 
   ScopedFile scoped_file_;