Skip to content

Commit

Permalink
[Storage] Blob Storage Refactoring pt 1:
Browse files Browse the repository at this point in the history
* Renaming classes to be more descriptive.
* Changing smart pointers to reflect strict ownership model.
* Adding pointers to facilitate future resource swapping.
* Remove renderer-side dependency on blob_data.h

This patch makes all of the far-reaching changes that effect
everyone that uses the blob storage context.  Subsequent
changes should only effect the blob infrastructure.

https://bit.ly/AutoBlobToDisk

BUG=375297

Review URL: https://codereview.chromium.org/810403004

Cr-Commit-Position: refs/heads/master@{#312800}
  • Loading branch information
dmurph authored and Commit bot committed Jan 23, 2015
1 parent 98d7ef5 commit bff2e53
Show file tree
Hide file tree
Showing 93 changed files with 884 additions and 553 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/drive/fileapi/async_file_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
#include "chrome/browser/chromeos/drive/fileapi/fileapi_worker.h"
#include "content/public/browser/browser_thread.h"
#include "google_apis/drive/task_util.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/file_system_operation_context.h"
#include "storage/browser/fileapi/file_system_url.h"
#include "storage/common/blob/shareable_file_reference.h"

using content::BrowserThread;

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/drive/fileapi/fileapi_worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
#include "base/callback_forward.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/chromeos/drive/file_errors.h"
#include "storage/common/blob/scoped_file.h"
#include "storage/browser/blob/scoped_file.h"

namespace base {
class FilePath;
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/file_manager/snapshot_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
#include "chrome/browser/profiles/profile.h"
#include "content/public/browser/browser_thread.h"
#include "google_apis/drive/task_util.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "storage/common/blob/shareable_file_reference.h"
#include "third_party/cros_system_api/constants/cryptohome.h"

namespace file_manager {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
#include "chrome/browser/chromeos/file_system_provider/mount_path_util.h"
#include "chrome/browser/chromeos/file_system_provider/provided_file_system_interface.h"
#include "content/public/browser/browser_thread.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/file_system_operation_context.h"
#include "storage/browser/fileapi/file_system_url.h"
#include "storage/common/blob/shareable_file_reference.h"

using content::BrowserThread;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
#include "content/public/test/test_browser_thread_bundle.h"
#include "content/public/test/test_file_system_context.h"
#include "extensions/browser/extension_registry.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/async_file_util.h"
#include "storage/browser/fileapi/external_mount_points.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "storage/browser/fileapi/file_system_url.h"
#include "storage/common/blob/shareable_file_reference.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace chromeos {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@
#include "extensions/common/switches.h"
#include "extensions/grit/extensions_browser_resources.h"
#include "net/base/net_util.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/external_mount_points.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "storage/browser/fileapi/file_system_operation.h"
#include "storage/browser/fileapi/file_system_operation_runner.h"
#include "storage/browser/fileapi/isolated_context.h"
#include "storage/common/blob/shareable_file_reference.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/base/webui/web_ui_util.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "base/memory/ref_counted.h"
#include "chrome/browser/extensions/chrome_extension_function.h"
#include "chrome/common/extensions/api/page_capture.h"
#include "storage/common/blob/shareable_file_reference.h"
#include "storage/browser/blob/shareable_file_reference.h"

namespace base {
class FilePath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
#include "chrome/browser/media_galleries/fileapi/readahead_file_stream_reader.h"
#include "content/public/browser/browser_thread.h"
#include "storage/browser/blob/file_stream_reader.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "storage/browser/fileapi/file_system_operation_context.h"
#include "storage/browser/fileapi/file_system_url.h"
#include "storage/browser/fileapi/native_file_util.h"
#include "storage/common/blob/shareable_file_reference.h"

using storage::AsyncFileUtil;
using storage::FileSystemOperationContext;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/async_file_util.h"
#include "storage/common/blob/shareable_file_reference.h"

namespace storage {
class FileSystemOperationContext;
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/media_galleries/fileapi/iphoto_file_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
#include "chrome/browser/media_galleries/fileapi/media_path_filter.h"
#include "chrome/browser/media_galleries/imported_media_gallery_registry.h"
#include "content/public/browser/browser_thread.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/file_system_operation_context.h"
#include "storage/browser/fileapi/file_system_url.h"
#include "storage/browser/fileapi/native_file_util.h"
#include "storage/common/blob/shareable_file_reference.h"
#include "storage/common/fileapi/directory_entry.h"
#include "storage/common/fileapi/file_system_util.h"

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/media_galleries/fileapi/itunes_file_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
#include "chrome/browser/media_galleries/fileapi/media_path_filter.h"
#include "chrome/browser/media_galleries/imported_media_gallery_registry.h"
#include "content/public/browser/browser_thread.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/file_system_operation_context.h"
#include "storage/browser/fileapi/file_system_url.h"
#include "storage/browser/fileapi/native_file_util.h"
#include "storage/common/blob/shareable_file_reference.h"
#include "storage/common/fileapi/file_system_util.h"

using storage::DirectoryEntry;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
#include "content/public/browser/browser_thread.h"
#include "net/base/io_buffer.h"
#include "net/base/mime_sniffer.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "storage/browser/fileapi/file_system_operation_context.h"
#include "storage/browser/fileapi/native_file_util.h"
#include "storage/common/blob/shareable_file_reference.h"
#include "url/gurl.h"

namespace {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@
#include "content/public/test/mock_special_storage_policy.h"
#include "content/public/test/test_browser_thread.h"
#include "content/public/test/test_file_system_options.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/async_file_util.h"
#include "storage/browser/fileapi/external_mount_points.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "storage/browser/fileapi/file_system_operation_context.h"
#include "storage/browser/fileapi/file_system_operation_runner.h"
#include "storage/browser/fileapi/isolated_context.h"
#include "storage/common/blob/shareable_file_reference.h"
#include "testing/gtest/include/gtest/gtest.h"

using storage::FileSystemOperationContext;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include "chrome/browser/sync_file_system/drive_backend/metadata_database.pb.h"
#include "chrome/browser/sync_file_system/sync_status_code.h"
#include "google_apis/drive/gdata_errorcode.h"
#include "storage/common/blob/scoped_file.h"
#include "storage/browser/blob/scoped_file.h"

namespace google_apis {
class ChangeResource;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
#include "google_apis/drive/drive_api_url_generator.h"
#include "google_apis/drive/gdata_wapi_url_generator.h"
#include "net/url_request/url_request_context_getter.h"
#include "storage/common/blob/scoped_file.h"
#include "storage/browser/blob/scoped_file.h"
#include "storage/common/fileapi/file_system_util.h"

namespace sync_file_system {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@
#include "content/public/test/mock_blob_url_request_context.h"
#include "content/public/test/mock_special_storage_policy.h"
#include "content/public/test/test_file_system_options.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/external_mount_points.h"
#include "storage/browser/fileapi/file_system_backend.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "storage/browser/fileapi/file_system_operation_context.h"
#include "storage/browser/fileapi/file_system_operation_runner.h"
#include "storage/browser/quota/quota_manager.h"
#include "storage/common/blob/shareable_file_reference.h"
#include "testing/gtest/include/gtest/gtest.h"

using base::File;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@
#include "chrome/browser/sync_file_system/logger.h"
#include "chrome/browser/sync_file_system/sync_file_metadata.h"
#include "chrome/browser/sync_file_system/syncable_file_system_util.h"
#include "storage/browser/blob/scoped_file.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "storage/browser/fileapi/file_system_file_util.h"
#include "storage/browser/fileapi/file_system_operation_context.h"
#include "storage/browser/fileapi/file_system_operation_runner.h"
#include "storage/common/blob/scoped_file.h"
#include "storage/common/fileapi/file_system_util.h"

using storage::FileSystemContext;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/test/mock_blob_url_request_context.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "storage/browser/blob/scoped_file.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "storage/browser/fileapi/file_system_operation_runner.h"
#include "storage/browser/fileapi/isolated_context.h"
#include "storage/common/blob/scoped_file.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/leveldatabase/src/helpers/memenv/memenv.h"
#include "third_party/leveldatabase/src/include/leveldb/env.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
#include "content/public/browser/storage_partition.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/common/extension_set.h"
#include "storage/browser/blob/scoped_file.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "storage/browser/fileapi/file_system_url.h"
#include "storage/common/blob/scoped_file.h"
#include "url/gurl.h"

using content::BrowserThread;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@
#include "chrome/browser/sync_file_system/local/syncable_file_operation_runner.h"
#include "chrome/browser/sync_file_system/syncable_file_system_util.h"
#include "net/url_request/url_request.h"
#include "storage/browser/blob/shareable_file_reference.h"
#include "storage/browser/fileapi/file_system_context.h"
#include "storage/browser/fileapi/file_system_operation.h"
#include "storage/browser/fileapi/file_system_operation_context.h"
#include "storage/browser/fileapi/file_system_url.h"
#include "storage/browser/fileapi/file_writer_delegate.h"
#include "storage/common/blob/shareable_file_reference.h"

using storage::FileSystemURL;

Expand Down
52 changes: 40 additions & 12 deletions content/browser/fileapi/blob_storage_context_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,24 @@
#include "base/run_loop.h"
#include "base/time/time.h"
#include "content/browser/fileapi/blob_storage_host.h"
#include "storage/browser/blob/blob_data_builder.h"
#include "storage/browser/blob/blob_data_handle.h"
#include "storage/browser/blob/blob_data_snapshot.h"
#include "storage/browser/blob/blob_storage_context.h"
#include "testing/gtest/include/gtest/gtest.h"

using storage::BlobDataBuilder;
using storage::BlobDataHandle;
using storage::BlobDataSnapshot;
using storage::BlobStorageContext;
using storage::DataElement;

namespace content {

namespace {
void SetupBasicBlob(BlobStorageHost* host, const std::string& id) {
EXPECT_TRUE(host->StartBuildingBlob(id));
BlobData::Item item;
DataElement item;
item.SetToBytes("1", 1);
EXPECT_TRUE(host->AppendBlobDataItem(id, item));
EXPECT_TRUE(host->FinishBuildingBlob(id, "text/plain"));
Expand Down Expand Up @@ -66,6 +72,22 @@ TEST(BlobStorageContextTest, IncrementDecrementRef) {
EXPECT_FALSE(host.IncrementBlobRefCount(kId));
}

TEST(BlobStorageContextTest, CancelBuildingBlob) {
BlobStorageContext context;
BlobStorageHost host(&context);
base::MessageLoop fake_io_message_loop;

// Build up a basic blob.
const std::string kId("id");
EXPECT_TRUE(host.StartBuildingBlob(kId));
DataElement item;
item.SetToBytes("1", 1);
EXPECT_TRUE(host.AppendBlobDataItem(kId, item));
EXPECT_TRUE(host.CancelBuildingBlob(kId));
EXPECT_FALSE(host.FinishBuildingBlob(kId, "text/plain"));
EXPECT_TRUE(host.StartBuildingBlob(kId));
}

TEST(BlobStorageContextTest, BlobDataHandle) {
BlobStorageContext context;
BlobStorageHost host(&context);
Expand Down Expand Up @@ -111,19 +133,20 @@ TEST(BlobStorageContextTest, CompoundBlobs) {
base::Time::FromString("Tue, 15 Nov 1994, 12:45:26 GMT", &time1);
base::Time::FromString("Mon, 14 Nov 1994, 11:30:49 GMT", &time2);

scoped_refptr<BlobData> blob_data1(new BlobData(kId1));
scoped_ptr<BlobDataBuilder> blob_data1(new BlobDataBuilder(kId1));
blob_data1->AppendData("Data1");
blob_data1->AppendData("Data2");
blob_data1->AppendFile(base::FilePath(FILE_PATH_LITERAL("File1.txt")),
10, 1024, time1);

scoped_refptr<BlobData> blob_data2(new BlobData(kId2));
scoped_ptr<BlobDataBuilder> blob_data2(new BlobDataBuilder(kId2));
blob_data2->AppendData("Data3");
blob_data2->AppendBlob(kId1, 8, 100);
blob_data2->AppendFile(base::FilePath(FILE_PATH_LITERAL("File2.txt")),
0, 20, time2);

scoped_refptr<BlobData> canonicalized_blob_data2(new BlobData(kId2Prime));
scoped_ptr<BlobDataBuilder> canonicalized_blob_data2(
new BlobDataBuilder(kId2Prime));
canonicalized_blob_data2->AppendData("Data3");
canonicalized_blob_data2->AppendData("a2___", 2);
canonicalized_blob_data2->AppendFile(
Expand All @@ -136,14 +159,18 @@ TEST(BlobStorageContextTest, CompoundBlobs) {
scoped_ptr<BlobDataHandle> blob_data_handle;

// Test a blob referring to only data and a file.
blob_data_handle = context.AddFinishedBlob(blob_data1.get());
ASSERT_TRUE(blob_data_handle.get());
EXPECT_TRUE(*(blob_data_handle->data()) == *blob_data1.get());
blob_data_handle = context.AddFinishedBlob(*blob_data1.get());
ASSERT_TRUE(blob_data_handle);
scoped_ptr<BlobDataSnapshot> data = blob_data_handle->CreateSnapshot();
ASSERT_TRUE(blob_data_handle);
EXPECT_EQ(*data, *blob_data1);

// Test a blob composed in part with another blob.
blob_data_handle = context.AddFinishedBlob(blob_data2.get());
ASSERT_TRUE(blob_data_handle.get());
EXPECT_TRUE(*(blob_data_handle->data()) == *canonicalized_blob_data2.get());
blob_data_handle = context.AddFinishedBlob(*blob_data2.get());
data = blob_data_handle->CreateSnapshot();
ASSERT_TRUE(blob_data_handle);
ASSERT_TRUE(data);
EXPECT_EQ(*data, *canonicalized_blob_data2);

blob_data_handle.reset();
{ // Clean up for ASAN
Expand All @@ -167,7 +194,8 @@ TEST(BlobStorageContextTest, PublicBlobUrls) {
scoped_ptr<BlobDataHandle> blob_data_handle =
context.GetBlobDataFromPublicURL(kUrl);
ASSERT_TRUE(blob_data_handle.get());
EXPECT_EQ(kId, blob_data_handle->data()->uuid());
EXPECT_EQ(kId, blob_data_handle->uuid());
scoped_ptr<BlobDataSnapshot> data = blob_data_handle->CreateSnapshot();
blob_data_handle.reset();
{ // Clean up for ASAN
base::RunLoop run_loop;
Expand Down Expand Up @@ -222,7 +250,7 @@ TEST(BlobStorageContextTest, EarlyContextDeletion) {
const std::string kId("id");
GURL kUrl("blob:id");
EXPECT_FALSE(host.StartBuildingBlob(kId));
BlobData::Item item;
DataElement item;
item.SetToBytes("1", 1);
EXPECT_FALSE(host.AppendBlobDataItem(kId, item));
EXPECT_FALSE(host.FinishBuildingBlob(kId, "text/plain"));
Expand Down
5 changes: 2 additions & 3 deletions content/browser/fileapi/blob_storage_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@

#include "base/sequenced_task_runner.h"
#include "base/strings/string_util.h"
#include "storage/browser/blob/blob_data_handle.h"
#include "storage/browser/blob/blob_storage_context.h"
#include "url/gurl.h"

using storage::BlobStorageContext;
using storage::BlobData;

namespace content {

Expand Down Expand Up @@ -42,7 +40,8 @@ bool BlobStorageHost::StartBuildingBlob(const std::string& uuid) {
}

bool BlobStorageHost::AppendBlobDataItem(
const std::string& uuid, const BlobData::Item& data_item) {
const std::string& uuid,
const storage::DataElement& data_item) {
if (!context_.get() || !IsBeingBuiltInHost(uuid))
return false;
context_->AppendBlobDataItem(uuid, data_item);
Expand Down
Loading

0 comments on commit bff2e53

Please sign in to comment.