diff options
author | michaeln@google.com <michaeln@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-17 23:00:51 +0000 |
---|---|---|
committer | michaeln@google.com <michaeln@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-17 23:00:51 +0000 |
commit | a3c71e8edf494687af2e67bc8fe8b013577bcbb6 (patch) | |
tree | ee46aaaf7a90f4b57065f6af59c2b6a647d92c00 | |
parent | 378429dc295cf26b11d20f9e52bc568e4dbf3b4e (diff) | |
download | chromium_src-a3c71e8edf494687af2e67bc8fe8b013577bcbb6.zip chromium_src-a3c71e8edf494687af2e67bc8fe8b013577bcbb6.tar.gz chromium_src-a3c71e8edf494687af2e67bc8fe8b013577bcbb6.tar.bz2 |
A few improvements to Blob handling.
* Break large blobs into multiple ipcs during creation.
* Use shared memory blocks for the xfer.
* Rename some methods and IPCs for readability.
* Cap memory usage in the browser process at 1G.
BUG=97221
Review URL: http://codereview.chromium.org/7974011
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@105950 0039d316-1c4b-4281-b951-d872f2087c98
19 files changed, 493 insertions, 444 deletions
diff --git a/content/browser/renderer_host/blob_message_filter.cc b/content/browser/renderer_host/blob_message_filter.cc index 829c025..6c09451 100644 --- a/content/browser/renderer_host/blob_message_filter.cc +++ b/content/browser/renderer_host/blob_message_filter.cc @@ -11,6 +11,8 @@ #include "webkit/blob/blob_data.h" #include "webkit/blob/blob_storage_controller.h" +using webkit_blob::BlobData; + BlobMessageFilter::BlobMessageFilter( int process_id, ChromeBlobStorageContext* blob_storage_context) @@ -28,7 +30,7 @@ void BlobMessageFilter::OnChannelClosing() { // process. for (base::hash_set<std::string>::const_iterator iter = blob_urls_.begin(); iter != blob_urls_.end(); ++iter) { - blob_storage_context_->controller()->UnregisterBlobUrl(GURL(*iter)); + blob_storage_context_->controller()->RemoveBlob(GURL(*iter)); } } @@ -38,48 +40,70 @@ bool BlobMessageFilter::OnMessageReceived(const IPC::Message& message, bool handled = true; IPC_BEGIN_MESSAGE_MAP_EX(BlobMessageFilter, message, *message_was_ok) - IPC_MESSAGE_HANDLER(BlobHostMsg_RegisterBlobUrl, OnRegisterBlobUrl) - IPC_MESSAGE_HANDLER(BlobHostMsg_RegisterBlobUrlFrom, OnRegisterBlobUrlFrom) - IPC_MESSAGE_HANDLER(BlobHostMsg_UnregisterBlobUrl, OnUnregisterBlobUrl) + IPC_MESSAGE_HANDLER(BlobHostMsg_StartBuildingBlob, OnStartBuildingBlob) + IPC_MESSAGE_HANDLER(BlobHostMsg_AppendBlobDataItem, OnAppendBlobDataItem) + IPC_MESSAGE_HANDLER(BlobHostMsg_SyncAppendSharedMemory, + OnAppendSharedMemory) + IPC_MESSAGE_HANDLER(BlobHostMsg_FinishBuildingBlob, OnFinishBuildingBlob) + IPC_MESSAGE_HANDLER(BlobHostMsg_CloneBlob, OnCloneBlob) + IPC_MESSAGE_HANDLER(BlobHostMsg_RemoveBlob, OnRemoveBlob) IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP() return handled; } -// Check if the child process has been granted permission to register the files. -bool BlobMessageFilter::CheckPermission( - webkit_blob::BlobData* blob_data) const { - ChildProcessSecurityPolicy* policy = - ChildProcessSecurityPolicy::GetInstance(); - for (std::vector<webkit_blob::BlobData::Item>::const_iterator iter = - blob_data->items().begin(); - iter != blob_data->items().end(); ++iter) { - if (iter->type() == webkit_blob::BlobData::TYPE_FILE) { - if (!policy->CanReadFile(process_id_, iter->file_path())) - return false; - } - } - return true; +void BlobMessageFilter::OnStartBuildingBlob(const GURL& url) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + blob_storage_context_->controller()->StartBuildingBlob(url); + blob_urls_.insert(url.spec()); } -void BlobMessageFilter::OnRegisterBlobUrl( - const GURL& url, const scoped_refptr<webkit_blob::BlobData>& blob_data) { +void BlobMessageFilter::OnAppendBlobDataItem( + const GURL& url, const BlobData::Item& item) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - if (!CheckPermission(blob_data.get())) + if (item.type == BlobData::TYPE_FILE && + !ChildProcessSecurityPolicy::GetInstance()->CanReadFile( + process_id_, item.file_path)) { + OnRemoveBlob(url); return; - blob_storage_context_->controller()->RegisterBlobUrl(url, blob_data); - blob_urls_.insert(url.spec()); + } + blob_storage_context_->controller()->AppendBlobDataItem(url, item); +} + +void BlobMessageFilter::OnAppendSharedMemory( + const GURL& url, base::SharedMemoryHandle handle, size_t buffer_size) { + DCHECK(base::SharedMemory::IsHandleValid(handle)); +#if defined(OS_WIN) + base::SharedMemory shared_memory(handle, true, peer_handle()); +#else + base::SharedMemory shared_memory(handle, true); +#endif + if (!shared_memory.Map(buffer_size)) { + OnRemoveBlob(url); + return; + } + + BlobData::Item item; + item.SetToDataExternal(static_cast<char*>(shared_memory.memory()), + buffer_size); + blob_storage_context_->controller()->AppendBlobDataItem(url, item); +} + +void BlobMessageFilter::OnFinishBuildingBlob( + const GURL& url, const std::string& content_type) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + blob_storage_context_->controller()->FinishBuildingBlob(url, content_type); } -void BlobMessageFilter::OnRegisterBlobUrlFrom( +void BlobMessageFilter::OnCloneBlob( const GURL& url, const GURL& src_url) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - blob_storage_context_->controller()->RegisterBlobUrlFrom(url, src_url); + blob_storage_context_->controller()->CloneBlob(url, src_url); blob_urls_.insert(url.spec()); } -void BlobMessageFilter::OnUnregisterBlobUrl(const GURL& url) { +void BlobMessageFilter::OnRemoveBlob(const GURL& url) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - blob_storage_context_->controller()->UnregisterBlobUrl(url); + blob_storage_context_->controller()->RemoveBlob(url); blob_urls_.erase(url.spec()); } diff --git a/content/browser/renderer_host/blob_message_filter.h b/content/browser/renderer_host/blob_message_filter.h index ace01db..24758ac 100644 --- a/content/browser/renderer_host/blob_message_filter.h +++ b/content/browser/renderer_host/blob_message_filter.h @@ -6,7 +6,9 @@ #define CONTENT_BROWSER_RENDERER_HOST_BLOB_MESSAGE_FILTER_H_ #include "base/hash_tables.h" +#include "base/shared_memory.h" #include "content/browser/browser_message_filter.h" +#include "webkit/blob/blob_data.h" class ChromeBlobStorageContext; class GURL; @@ -15,10 +17,6 @@ namespace IPC { class Message; } -namespace webkit_blob { -class BlobData; -} - class BlobMessageFilter : public BrowserMessageFilter { public: BlobMessageFilter(int process_id, @@ -31,12 +29,14 @@ class BlobMessageFilter : public BrowserMessageFilter { bool* message_was_ok); private: - void OnRegisterBlobUrl(const GURL& url, - const scoped_refptr<webkit_blob::BlobData>& blob_data); - void OnRegisterBlobUrlFrom(const GURL& url, const GURL& src_url); - void OnUnregisterBlobUrl(const GURL& url); - - bool CheckPermission(webkit_blob::BlobData* blob_data) const; + void OnStartBuildingBlob(const GURL& url); + void OnAppendBlobDataItem(const GURL& url, + const webkit_blob::BlobData::Item& item); + void OnAppendSharedMemory(const GURL& url, base::SharedMemoryHandle handle, + size_t buffer_size); + void OnFinishBuildingBlob(const GURL& url, const std::string& content_type); + void OnCloneBlob(const GURL& url, const GURL& src_url); + void OnRemoveBlob(const GURL& url); int process_id_; scoped_refptr<ChromeBlobStorageContext> blob_storage_context_; diff --git a/content/common/webblob_messages.h b/content/common/webblob_messages.h index 2cbb2b7..6fcd61f 100644 --- a/content/common/webblob_messages.h +++ b/content/common/webblob_messages.h @@ -11,19 +11,47 @@ #define IPC_MESSAGE_START BlobMsgStart +IPC_ENUM_TRAITS(webkit_blob::BlobData::Type) + +IPC_STRUCT_TRAITS_BEGIN(webkit_blob::BlobData::Item) + IPC_STRUCT_TRAITS_MEMBER(type) + IPC_STRUCT_TRAITS_MEMBER(data) + IPC_STRUCT_TRAITS_MEMBER(file_path) + IPC_STRUCT_TRAITS_MEMBER(blob_url) + IPC_STRUCT_TRAITS_MEMBER(offset) + IPC_STRUCT_TRAITS_MEMBER(length) + IPC_STRUCT_TRAITS_MEMBER(expected_modification_time) +IPC_STRUCT_TRAITS_END() + // Blob messages sent from the renderer to the browser. -// Registers a blob URL referring to the specified blob data. -IPC_MESSAGE_CONTROL2(BlobHostMsg_RegisterBlobUrl, + +// Registers a blob as being built. +IPC_MESSAGE_CONTROL1(BlobHostMsg_StartBuildingBlob, + GURL /* url */) + +// Appends data to a blob being built. +IPC_MESSAGE_CONTROL2(BlobHostMsg_AppendBlobDataItem, + GURL /* url */, + webkit_blob::BlobData::Item) + +// Appends data to a blob being built. +IPC_SYNC_MESSAGE_CONTROL3_0(BlobHostMsg_SyncAppendSharedMemory, + GURL /* url */, + base::SharedMemoryHandle, + size_t /* buffer size */) + +// Finishes building a blob. +IPC_MESSAGE_CONTROL2(BlobHostMsg_FinishBuildingBlob, GURL /* url */, - scoped_refptr<webkit_blob::BlobData> /* blob_data */) + std::string /* content_type */) -// Registers a blob URL referring to the blob data identified by the specified -// source URL. -IPC_MESSAGE_CONTROL2(BlobHostMsg_RegisterBlobUrlFrom, +// Creates a new blob that's a clone of an existing src blob. +// The source blob must be fully built. +IPC_MESSAGE_CONTROL2(BlobHostMsg_CloneBlob, GURL /* url */, GURL /* src_url */) -// Unregister a blob URL. -IPC_MESSAGE_CONTROL1(BlobHostMsg_UnregisterBlobUrl, +// Removes a blob. +IPC_MESSAGE_CONTROL1(BlobHostMsg_RemoveBlob, GURL /* url */) diff --git a/content/common/webblobregistry_impl.cc b/content/common/webblobregistry_impl.cc index 7a6a1ea..53d7a74 100644 --- a/content/common/webblobregistry_impl.cc +++ b/content/common/webblobregistry_impl.cc @@ -5,18 +5,21 @@ #include "content/common/webblobregistry_impl.h" #include "base/memory/ref_counted.h" +#include "base/shared_memory.h" +#include "content/common/child_thread.h" #include "content/common/webblob_messages.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebBlobData.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebString.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebURL.h" #include "webkit/blob/blob_data.h" +#include "webkit/glue/webkit_glue.h" using WebKit::WebBlobData; using WebKit::WebString; using WebKit::WebURL; -WebBlobRegistryImpl::WebBlobRegistryImpl(IPC::Message::Sender* sender) - : sender_(sender) { +WebBlobRegistryImpl::WebBlobRegistryImpl(ChildThread* child_thread) + : child_thread_(child_thread) { } WebBlobRegistryImpl::~WebBlobRegistryImpl() { @@ -24,16 +27,72 @@ WebBlobRegistryImpl::~WebBlobRegistryImpl() { void WebBlobRegistryImpl::registerBlobURL( const WebURL& url, WebBlobData& data) { - scoped_refptr<webkit_blob::BlobData> blob_data( - new webkit_blob::BlobData(data)); - sender_->Send(new BlobHostMsg_RegisterBlobUrl(url, blob_data)); + const size_t kLargeThresholdBytes = 250 * 1024; + const size_t kMaxSharedMemoryBytes = 10 * 1024 * 1024; + + child_thread_->Send(new BlobHostMsg_StartBuildingBlob(url)); + size_t i = 0; + WebBlobData::Item data_item; + while (data.itemAt(i++, data_item)) { + webkit_blob::BlobData::Item item; + switch (data_item.type) { + case WebBlobData::Item::TypeData: { + // WebBlobData does not allow partial data items. + DCHECK(!data_item.offset && data_item.length == -1); + if (data_item.data.size() < kLargeThresholdBytes) { + item.SetToData(data_item.data.data(), data_item.data.size()); + child_thread_->Send(new BlobHostMsg_AppendBlobDataItem(url, item)); + } else { + // We handle larger amounts of data via SharedMemory instead of + // writing it directly to the IPC channel. + size_t data_size = data_item.data.size(); + const char* data_ptr = data_item.data.data(); + size_t shared_memory_size = std::min( + data_size, kMaxSharedMemoryBytes); + scoped_ptr<base::SharedMemory> shared_memory( + child_thread_->AllocateSharedMemory(shared_memory_size)); + CHECK(shared_memory.get()); + while (data_size) { + size_t chunk_size = std::min(data_size, shared_memory_size); + memcpy(shared_memory->memory(), data_ptr, chunk_size); + child_thread_->Send(new BlobHostMsg_SyncAppendSharedMemory( + url, shared_memory->handle(), chunk_size)); + data_size -= chunk_size; + data_ptr += chunk_size; + } + } + break; + } + case WebBlobData::Item::TypeFile: + item.SetToFile( + webkit_glue::WebStringToFilePath(data_item.filePath), + static_cast<uint64>(data_item.offset), + static_cast<uint64>(data_item.length), + base::Time::FromDoubleT(data_item.expectedModificationTime)); + child_thread_->Send(new BlobHostMsg_AppendBlobDataItem(url, item)); + break; + case WebBlobData::Item::TypeBlob: + if (data_item.length) { + item.SetToBlob( + data_item.blobURL, + static_cast<uint64>(data_item.offset), + static_cast<uint64>(data_item.length)); + } + child_thread_->Send(new BlobHostMsg_AppendBlobDataItem(url, item)); + break; + default: + NOTREACHED(); + } + } + child_thread_->Send(new BlobHostMsg_FinishBuildingBlob( + url, data.contentType().utf8().data())); } void WebBlobRegistryImpl::registerBlobURL( const WebURL& url, const WebURL& src_url) { - sender_->Send(new BlobHostMsg_RegisterBlobUrlFrom(url, src_url)); + child_thread_->Send(new BlobHostMsg_CloneBlob(url, src_url)); } void WebBlobRegistryImpl::unregisterBlobURL(const WebURL& url) { - sender_->Send(new BlobHostMsg_UnregisterBlobUrl(url)); + child_thread_->Send(new BlobHostMsg_RemoveBlob(url)); } diff --git a/content/common/webblobregistry_impl.h b/content/common/webblobregistry_impl.h index 73395d9..6b05683 100644 --- a/content/common/webblobregistry_impl.h +++ b/content/common/webblobregistry_impl.h @@ -6,19 +6,23 @@ #define CHROME_COMMON_WEBBLOBREGISTRY_IMPL_H_ #pragma once -#include "ipc/ipc_message.h" +//#include "ipc/ipc_message.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebBlobRegistry.h" +class ChildThread; + +namespace base { +class SharedMemory; +} + namespace WebKit { class WebBlobData; -class WebBlobStorageData; -class WebString; class WebURL; } class WebBlobRegistryImpl : public WebKit::WebBlobRegistry { public: - explicit WebBlobRegistryImpl(IPC::Message::Sender* sender); + explicit WebBlobRegistryImpl(ChildThread* child_thread); virtual ~WebBlobRegistryImpl(); // See WebBlobRegistry.h for documentation on these functions. @@ -29,7 +33,7 @@ class WebBlobRegistryImpl : public WebKit::WebBlobRegistry { virtual void unregisterBlobURL(const WebKit::WebURL& url); private: - IPC::Message::Sender* sender_; + ChildThread* child_thread_; }; #endif // CHROME_COMMON_WEBBLOBREGISTRY_IMPL_H_ diff --git a/content/common/webkit_param_traits.cc b/content/common/webkit_param_traits.cc index bed2f26..a10eaa8 100644 --- a/content/common/webkit_param_traits.cc +++ b/content/common/webkit_param_traits.cc @@ -150,106 +150,6 @@ void ParamTraits<scoped_refptr<webkit_glue::ResourceDevToolsInfo> >::Log( l->append(")"); } -// Only the webkit_blob::BlobData ParamTraits<> definition needs this -// definition, so keep this in the implementation file so we can forward declare -// BlobData in the header. -template <> -struct ParamTraits<webkit_blob::BlobData::Item> { - typedef webkit_blob::BlobData::Item param_type; - static void Write(Message* m, const param_type& p) { - WriteParam(m, static_cast<int>(p.type())); - if (p.type() == webkit_blob::BlobData::TYPE_DATA) { - WriteParam(m, p.data()); - } else if (p.type() == webkit_blob::BlobData::TYPE_FILE) { - WriteParam(m, p.file_path()); - WriteParam(m, p.offset()); - WriteParam(m, p.length()); - WriteParam(m, p.expected_modification_time()); - } else { - WriteParam(m, p.blob_url()); - WriteParam(m, p.offset()); - WriteParam(m, p.length()); - } - } - static bool Read(const Message* m, void** iter, param_type* r) { - int type; - if (!ReadParam(m, iter, &type)) - return false; - if (type == webkit_blob::BlobData::TYPE_DATA) { - std::string data; - if (!ReadParam(m, iter, &data)) - return false; - r->SetToData(data); - } else if (type == webkit_blob::BlobData::TYPE_FILE) { - FilePath file_path; - uint64 offset, length; - base::Time expected_modification_time; - if (!ReadParam(m, iter, &file_path)) - return false; - if (!ReadParam(m, iter, &offset)) - return false; - if (!ReadParam(m, iter, &length)) - return false; - if (!ReadParam(m, iter, &expected_modification_time)) - return false; - r->SetToFile(file_path, offset, length, expected_modification_time); - } else { - DCHECK(type == webkit_blob::BlobData::TYPE_BLOB); - GURL blob_url; - uint64 offset, length; - if (!ReadParam(m, iter, &blob_url)) - return false; - if (!ReadParam(m, iter, &offset)) - return false; - if (!ReadParam(m, iter, &length)) - return false; - r->SetToBlob(blob_url, offset, length); - } - return true; - } - static void Log(const param_type& p, std::string* l) { - l->append("<BlobData::Item>"); - } -}; - -void ParamTraits<scoped_refptr<webkit_blob::BlobData> >::Write( - Message* m, const param_type& p) { - WriteParam(m, p.get() != NULL); - if (p) { - WriteParam(m, p->items()); - WriteParam(m, p->content_type()); - WriteParam(m, p->content_disposition()); - } -} - -bool ParamTraits<scoped_refptr<webkit_blob::BlobData> >::Read( - const Message* m, void** iter, param_type* r) { - bool has_object; - if (!ReadParam(m, iter, &has_object)) - return false; - if (!has_object) - return true; - std::vector<webkit_blob::BlobData::Item> items; - if (!ReadParam(m, iter, &items)) - return false; - std::string content_type; - if (!ReadParam(m, iter, &content_type)) - return false; - std::string content_disposition; - if (!ReadParam(m, iter, &content_disposition)) - return false; - *r = new webkit_blob::BlobData; - (*r)->swap_items(&items); - (*r)->set_content_type(content_type); - (*r)->set_content_disposition(content_disposition); - return true; -} - -void ParamTraits<scoped_refptr<webkit_blob::BlobData> >::Log( - const param_type& p, std::string* l) { - l->append("<webkit_blob::BlobData>"); -} - void ParamTraits<NPVariant_Param>::Write(Message* m, const param_type& p) { WriteParam(m, static_cast<int>(p.type)); if (p.type == NPVARIANT_PARAM_BOOL) { diff --git a/content/common/webkit_param_traits.h b/content/common/webkit_param_traits.h index 9808e59..8bf9973 100644 --- a/content/common/webkit_param_traits.h +++ b/content/common/webkit_param_traits.h @@ -90,14 +90,6 @@ struct ParamTraits<scoped_refptr<webkit_glue::ResourceDevToolsInfo> > { }; template <> -struct ParamTraits<scoped_refptr<webkit_blob::BlobData > > { - typedef scoped_refptr<webkit_blob::BlobData> param_type; - static void Write(Message* m, const param_type& p); - static bool Read(const Message* m, void** iter, param_type* r); - static void Log(const param_type& p, std::string* l); -}; - -template <> struct ParamTraits<NPVariant_Param> { typedef NPVariant_Param param_type; static void Write(Message* m, const param_type& p); diff --git a/content/renderer/renderer_webkitplatformsupport_impl.cc b/content/renderer/renderer_webkitplatformsupport_impl.cc index 9c71370..6ed042b 100644 --- a/content/renderer/renderer_webkitplatformsupport_impl.cc +++ b/content/renderer/renderer_webkitplatformsupport_impl.cc @@ -604,9 +604,9 @@ RendererWebKitPlatformSupportImpl::signedPublicKeyAndChallengeString( //------------------------------------------------------------------------------ WebBlobRegistry* RendererWebKitPlatformSupportImpl::blobRegistry() { - // RenderThreadImpl::current can be NULL when running some tests. - if (!blob_registry_.get() && RenderThreadImpl::current()) { - blob_registry_.reset(new WebBlobRegistryImpl(RenderThreadImpl::Get())); + // ChildThread::current can be NULL when running some tests. + if (!blob_registry_.get() && ChildThread::current()) { + blob_registry_.reset(new WebBlobRegistryImpl(ChildThread::current())); } return blob_registry_.get(); } diff --git a/webkit/blob/blob_data.cc b/webkit/blob/blob_data.cc index 2366a00..b73a139 100644 --- a/webkit/blob/blob_data.cc +++ b/webkit/blob/blob_data.cc @@ -18,9 +18,10 @@ using WebKit::WebString; namespace webkit_blob { BlobData::Item::Item() - : type_(TYPE_DATA), - offset_(0), - length_(0) { + : type(TYPE_DATA), + data_external(NULL), + offset(0), + length(0) { } BlobData::Item::~Item() {} diff --git a/webkit/blob/blob_data.h b/webkit/blob/blob_data.h index 587b60b..697591e 100644 --- a/webkit/blob/blob_data.h +++ b/webkit/blob/blob_data.h @@ -24,82 +24,70 @@ class BlobData : public base::RefCounted<BlobData> { public: enum Type { TYPE_DATA, + TYPE_DATA_EXTERNAL, TYPE_FILE, TYPE_BLOB }; - class Item { - public: + struct Item { Item(); ~Item(); - Type type() const { return type_; } - const std::string& data() const { return data_; } - const FilePath& file_path() const { return file_path_; } - const GURL& blob_url() const { return blob_url_; } - uint64 offset() const { return offset_; } - uint64 length() const { return length_; } - const base::Time& expected_modification_time() const { - return expected_modification_time_; + void SetToData(const std::string& data) { + SetToData(data.c_str(), data.size()); } - void SetToData(const std::string& data) { - SetToData(data, 0, static_cast<uint32>(data.size())); + void SetToData(const char* data, size_t length) { + type = TYPE_DATA; + this->data.assign(data, length); + this->offset = 0; + this->length = length; } - void SetToData(const std::string& data, uint32 offset, uint32 length) { - // TODO(jianli): Need to implement ref-counting vector storage. - type_ = TYPE_DATA; - data_ = data; - offset_ = offset; - length_ = length; + void SetToDataExternal(const char* data, size_t length) { + type = TYPE_DATA_EXTERNAL; + this->data_external = data; + this->offset = 0; + this->length = length; } void SetToFile(const FilePath& file_path, uint64 offset, uint64 length, const base::Time& expected_modification_time) { - type_ = TYPE_FILE; - file_path_ = file_path; - offset_ = offset; - length_ = length; - expected_modification_time_ = expected_modification_time; + type = TYPE_FILE; + this->file_path = file_path; + this->offset = offset; + this->length = length; + this->expected_modification_time = expected_modification_time; } void SetToBlob(const GURL& blob_url, uint64 offset, uint64 length) { - type_ = TYPE_BLOB; - blob_url_ = blob_url; - offset_ = offset; - length_ = length; + type = TYPE_BLOB; + this->blob_url = blob_url; + this->offset = offset; + this->length = length; } - private: - Type type_; - - // For Data type. - std::string data_; - - // For File type. - FilePath file_path_; - - // For Blob typ. - GURL blob_url_; - - uint64 offset_; - uint64 length_; - base::Time expected_modification_time_; + Type type; + std::string data; // For Data type. + const char* data_external; // For DataExternal type. + GURL blob_url; // For Blob type. + FilePath file_path; // For File type. + base::Time expected_modification_time; // Also for File type. + uint64 offset; + uint64 length; }; BlobData(); explicit BlobData(const WebKit::WebBlobData& data); void AppendData(const std::string& data) { - // TODO(jianli): Consider writing the big data to the disk. - AppendData(data, 0, static_cast<uint32>(data.size())); + AppendData(data.c_str(), data.size()); } - void AppendData(const std::string& data, uint32 offset, uint32 length) { + void AppendData(const char* data, size_t length) { if (length > 0) { items_.push_back(Item()); - items_.back().SetToData(data, offset, length); + items_.back().SetToData(data, length); } } @@ -133,9 +121,14 @@ class BlobData : public base::RefCounted<BlobData> { content_disposition_ = content_disposition; } - // Should only be called by the IPC ParamTraits for this class. - void swap_items(std::vector<Item>* items) { - items_.swap(*items); + int64 GetMemoryUsage() const { + int64 memory = 0; + for (std::vector<Item>::const_iterator iter = items_.begin(); + iter != items_.end(); ++iter) { + if (iter->type == TYPE_DATA) + memory += iter->data.size(); + } + return memory; } private: @@ -153,23 +146,23 @@ class BlobData : public base::RefCounted<BlobData> { #if defined(UNIT_TEST) inline bool operator==(const BlobData::Item& a, const BlobData::Item& b) { - if (a.type() != b.type()) + if (a.type != b.type) return false; - if (a.type() == BlobData::TYPE_DATA) { - return a.data() == b.data() && - a.offset() == b.offset() && - a.length() == b.length(); + if (a.type == BlobData::TYPE_DATA) { + return a.data == b.data && + a.offset == b.offset && + a.length == b.length; } - if (a.type() == BlobData::TYPE_FILE) { - return a.file_path() == b.file_path() && - a.offset() == b.offset() && - a.length() == b.length() && - a.expected_modification_time() == b.expected_modification_time(); + if (a.type == BlobData::TYPE_FILE) { + return a.file_path == b.file_path && + a.offset == b.offset && + a.length == b.length && + a.expected_modification_time == b.expected_modification_time; } - if (a.type() == BlobData::TYPE_BLOB) { - return a.blob_url() == b.blob_url() && - a.offset() == b.offset() && - a.length() == b.length(); + if (a.type == BlobData::TYPE_BLOB) { + return a.blob_url == b.blob_url && + a.offset == b.offset && + a.length == b.length; } return false; } diff --git a/webkit/blob/blob_storage_controller.cc b/webkit/blob/blob_storage_controller.cc index 4fb8018..6490c66 100644 --- a/webkit/blob/blob_storage_controller.cc +++ b/webkit/blob/blob_storage_controller.cc @@ -29,22 +29,36 @@ GURL ClearBlobUrlRef(const GURL& url) { return GURL(url.spec().substr(0, hash_pos)); } +static const int64 kMaxMemoryUsage = 1024 * 1024 * 1024; // 1G + } // namespace -BlobStorageController::BlobStorageController() { +BlobStorageController::BlobStorageController() + : memory_usage_(0) { } BlobStorageController::~BlobStorageController() { } -void BlobStorageController::RegisterBlobUrl( - const GURL& url, const BlobData* blob_data) { +void BlobStorageController::StartBuildingBlob(const GURL& url) { DCHECK(url.SchemeIs("blob")); DCHECK(!BlobUrlHasRef(url)); + BlobData* blob_data = new BlobData; + unfinalized_blob_map_[url.spec()] = blob_data; + IncrementBlobDataUsage(blob_data); +} - scoped_refptr<BlobData> target_blob_data(new BlobData()); - target_blob_data->set_content_type(blob_data->content_type()); - target_blob_data->set_content_disposition(blob_data->content_disposition()); +void BlobStorageController::AppendBlobDataItem( + const GURL& url, const BlobData::Item& item) { + DCHECK(url.SchemeIs("blob")); + DCHECK(!BlobUrlHasRef(url)); + BlobMap::iterator found = unfinalized_blob_map_.find(url.spec()); + if (found == unfinalized_blob_map_.end()) + return; + BlobData* target_blob_data = found->second; + DCHECK(target_blob_data); + + memory_usage_ -= target_blob_data->GetMemoryUsage(); // The blob data is stored in the "canonical" way. That is, it only contains a // list of Data and File items. @@ -54,40 +68,67 @@ void BlobStorageController::RegisterBlobUrl( // All the Blob items in the passing blob data are resolved and expanded into // a set of Data and File items. - for (std::vector<BlobData::Item>::const_iterator iter = - blob_data->items().begin(); - iter != blob_data->items().end(); ++iter) { - switch (iter->type()) { - case BlobData::TYPE_DATA: { - // WebBlobData does not allow partial data. - DCHECK(!(iter->offset()) && iter->length() == iter->data().size()); - target_blob_data->AppendData(iter->data()); - break; - } - case BlobData::TYPE_FILE: - AppendFileItem(target_blob_data, - iter->file_path(), - iter->offset(), - iter->length(), - iter->expected_modification_time()); - break; - case BlobData::TYPE_BLOB: { - BlobData* src_blob_data = GetBlobDataFromUrl(iter->blob_url()); - DCHECK(src_blob_data); - if (src_blob_data) - AppendStorageItems(target_blob_data.get(), - src_blob_data, - iter->offset(), - iter->length()); - break; - } - } + switch (item.type) { + case BlobData::TYPE_DATA: + // WebBlobData does not allow partial data. + DCHECK(!(item.offset) && item.length == item.data.size()); + target_blob_data->AppendData(item.data.c_str(), item.data.size()); + break; + case BlobData::TYPE_DATA_EXTERNAL: + DCHECK(!item.offset); + target_blob_data->AppendData(item.data_external, item.length); + break; + case BlobData::TYPE_FILE: + AppendFileItem(target_blob_data, + item.file_path, + item.offset, + item.length, + item.expected_modification_time); + break; + case BlobData::TYPE_BLOB: + BlobData* src_blob_data = GetBlobDataFromUrl(item.blob_url); + DCHECK(src_blob_data); + if (src_blob_data) + AppendStorageItems(target_blob_data, + src_blob_data, + item.offset, + item.length); + break; } - blob_map_[url.spec()] = target_blob_data; + memory_usage_ += target_blob_data->GetMemoryUsage(); + + // If we're using too much memory, drop this blob. + // TODO(michaeln): Blob memory storage does not yet spill over to disk, + // until it does, we'll prevent memory usage over a max amount. + if (memory_usage_ > kMaxMemoryUsage) + RemoveBlob(url); +} + +void BlobStorageController::FinishBuildingBlob( + const GURL& url, const std::string& content_type) { + DCHECK(url.SchemeIs("blob")); + DCHECK(!BlobUrlHasRef(url)); + BlobMap::iterator found = unfinalized_blob_map_.find(url.spec()); + if (found == unfinalized_blob_map_.end()) + return; + found->second->set_content_type(content_type); + blob_map_[url.spec()] = found->second; + unfinalized_blob_map_.erase(found); } -void BlobStorageController::RegisterBlobUrlFrom( +void BlobStorageController::AddFinishedBlob(const GURL& url, + const BlobData* data) { + StartBuildingBlob(url); + for (std::vector<BlobData::Item>::const_iterator iter = + data->items().begin(); + iter != data->items().end(); ++iter) { + AppendBlobDataItem(url, *iter); + } + FinishBuildingBlob(url, data->content_type()); +} + +void BlobStorageController::CloneBlob( const GURL& url, const GURL& src_url) { DCHECK(url.SchemeIs("blob")); DCHECK(!BlobUrlHasRef(url)); @@ -98,12 +139,29 @@ void BlobStorageController::RegisterBlobUrlFrom( return; blob_map_[url.spec()] = blob_data; + IncrementBlobDataUsage(blob_data); } -void BlobStorageController::UnregisterBlobUrl(const GURL& url) { - blob_map_.erase(url.spec()); +void BlobStorageController::RemoveBlob(const GURL& url) { + DCHECK(url.SchemeIs("blob")); + DCHECK(!BlobUrlHasRef(url)); + + if (!RemoveFromMapHelper(&unfinalized_blob_map_, url)) + RemoveFromMapHelper(&blob_map_, url); +} + +bool BlobStorageController::RemoveFromMapHelper( + BlobMap* map, const GURL& url) { + BlobMap::iterator found = map->find(url.spec()); + if (found == map->end()) + return false; + if (DecrementBlobDataUsage(found->second)) + memory_usage_ -= found->second->GetMemoryUsage(); + map->erase(found); + return true; } + BlobData* BlobStorageController::GetBlobDataFromUrl(const GURL& url) { BlobMap::iterator found = blob_map_.find( BlobUrlHasRef(url) ? ClearBlobUrlRef(url).spec() : url.spec()); @@ -123,7 +181,7 @@ void BlobStorageController::ResolveBlobReferencesInUploadData( } // Find the referred blob data. - webkit_blob::BlobData* blob_data = GetBlobDataFromUrl(iter->blob_url()); + BlobData* blob_data = GetBlobDataFromUrl(iter->blob_url()); DCHECK(blob_data); if (!blob_data) { // TODO(jianli): We should probably fail uploading the data @@ -146,22 +204,22 @@ void BlobStorageController::ResolveBlobReferencesInUploadData( for (size_t i = blob_data->items().size(); i > 0; --i) { iter = uploads->insert(iter, net::UploadData::Element()); - const webkit_blob::BlobData::Item& item = blob_data->items().at(i - 1); - switch (item.type()) { - case webkit_blob::BlobData::TYPE_DATA: + const BlobData::Item& item = blob_data->items().at(i - 1); + switch (item.type) { + case BlobData::TYPE_DATA: // TODO(jianli): Figure out how to avoid copying the data. iter->SetToBytes( - &item.data().at(0) + static_cast<int>(item.offset()), - static_cast<int>(item.length())); + &item.data.at(0) + static_cast<int>(item.offset), + static_cast<int>(item.length)); break; - case webkit_blob::BlobData::TYPE_FILE: + case BlobData::TYPE_FILE: // TODO(michaeln): Ensure that any temp files survive till the // net::URLRequest is done with the upload. iter->SetToFilePathRange( - item.file_path(), - item.offset(), - item.length(), - item.expected_modification_time()); + item.file_path, + item.offset, + item.length, + item.expected_modification_time); break; default: NOTREACHED(); @@ -181,27 +239,27 @@ void BlobStorageController::AppendStorageItems( src_blob_data->items().begin(); if (offset) { for (; iter != src_blob_data->items().end(); ++iter) { - if (offset >= iter->length()) - offset -= iter->length(); + if (offset >= iter->length) + offset -= iter->length; else break; } } for (; iter != src_blob_data->items().end() && length > 0; ++iter) { - uint64 current_length = iter->length() - offset; + uint64 current_length = iter->length - offset; uint64 new_length = current_length > length ? length : current_length; - if (iter->type() == BlobData::TYPE_DATA) { - target_blob_data->AppendData(iter->data(), - static_cast<uint32>(iter->offset() + offset), - static_cast<uint32>(new_length)); + if (iter->type == BlobData::TYPE_DATA) { + target_blob_data->AppendData( + iter->data.c_str() + static_cast<size_t>(iter->offset + offset), + static_cast<uint32>(new_length)); } else { - DCHECK(iter->type() == BlobData::TYPE_FILE); + DCHECK(iter->type == BlobData::TYPE_FILE); AppendFileItem(target_blob_data, - iter->file_path(), - iter->offset() + offset, + iter->file_path, + iter->offset + offset, new_length, - iter->expected_modification_time()); + iter->expected_modification_time); } length -= new_length; offset = 0; @@ -222,4 +280,17 @@ void BlobStorageController::AppendFileItem( target_blob_data->AttachDeletableFileReference(deletable_file); } +void BlobStorageController::IncrementBlobDataUsage(BlobData* blob_data) { + blob_data_usage_count_[blob_data] += 1; +} + +bool BlobStorageController::DecrementBlobDataUsage(BlobData* blob_data) { + BlobDataUsageMap::iterator found = blob_data_usage_count_.find(blob_data); + DCHECK(found != blob_data_usage_count_.end()); + if (--(found->second)) + return false; // Still in use + blob_data_usage_count_.erase(found); + return true; +} + } // namespace webkit_blob diff --git a/webkit/blob/blob_storage_controller.h b/webkit/blob/blob_storage_controller.h index 7f8d45c..7dbb6f7 100644 --- a/webkit/blob/blob_storage_controller.h +++ b/webkit/blob/blob_storage_controller.h @@ -5,9 +5,13 @@ #ifndef WEBKIT_BLOB_BLOB_STORAGE_CONTROLLER_H_ #define WEBKIT_BLOB_BLOB_STORAGE_CONTROLLER_H_ +#include <map> +#include <string> + #include "base/hash_tables.h" #include "base/memory/ref_counted.h" #include "base/process.h" +#include "webkit/blob/blob_data.h" class GURL; class FilePath; @@ -21,17 +25,18 @@ class UploadData; namespace webkit_blob { -class BlobData; - // This class handles the logistics of blob Storage within the browser process. class BlobStorageController { public: BlobStorageController(); ~BlobStorageController(); - void RegisterBlobUrl(const GURL& url, const BlobData* blob_data); - void RegisterBlobUrlFrom(const GURL& url, const GURL& src_url); - void UnregisterBlobUrl(const GURL& url); + void StartBuildingBlob(const GURL& url); + void AppendBlobDataItem(const GURL& url, const BlobData::Item& data_item); + void FinishBuildingBlob(const GURL& url, const std::string& content_type); + void AddFinishedBlob(const GURL& url, const BlobData* blob_data); + void CloneBlob(const GURL& url, const GURL& src_url); + void RemoveBlob(const GURL& url); BlobData* GetBlobDataFromUrl(const GURL& url); // If there is any blob reference in the upload data, it will get resolved @@ -41,6 +46,9 @@ class BlobStorageController { private: friend class ViewBlobInternalsJob; + typedef base::hash_map<std::string, scoped_refptr<BlobData> > BlobMap; + typedef std::map<BlobData*, int> BlobDataUsageMap; + void AppendStorageItems(BlobData* target_blob_data, BlobData* src_blob_data, uint64 offset, @@ -49,8 +57,23 @@ class BlobStorageController { const FilePath& file_path, uint64 offset, uint64 length, const base::Time& expected_modification_time); - typedef base::hash_map<std::string, scoped_refptr<BlobData> > BlobMap; + bool RemoveFromMapHelper(BlobMap* map, const GURL& url); + + void IncrementBlobDataUsage(BlobData* blob_data); + // Returns true if no longer in use. + bool DecrementBlobDataUsage(BlobData* blob_data); + BlobMap blob_map_; + BlobMap unfinalized_blob_map_; + + // Used to keep track of how much memory is being utitlized for blob data, + // we count only the items of TYPE_DATA which are held in memory and not + // items of TYPE_FILE. + int64 memory_usage_; + + // Multiple urls can refer to the same blob data, this map keeps track of + // how many urls refer to a BlobData. + BlobDataUsageMap blob_data_usage_count_; DISALLOW_COPY_AND_ASSIGN(BlobStorageController); }; diff --git a/webkit/blob/blob_storage_controller_unittest.cc b/webkit/blob/blob_storage_controller_unittest.cc index 27beb44..f98f449 100644 --- a/webkit/blob/blob_storage_controller_unittest.cc +++ b/webkit/blob/blob_storage_controller_unittest.cc @@ -35,45 +35,44 @@ TEST(BlobStorageControllerTest, RegisterBlobUrl) { scoped_refptr<BlobData> canonicalized_blob_data2(new BlobData()); canonicalized_blob_data2->AppendData("Data3"); - canonicalized_blob_data2->AppendData("Data2", 3, 2); + canonicalized_blob_data2->AppendData("a2___", 2); canonicalized_blob_data2->AppendFile(FilePath(FILE_PATH_LITERAL("File1.txt")), 10, 98, time1); canonicalized_blob_data2->AppendFile(FilePath(FILE_PATH_LITERAL("File2.txt")), 0, 20, time2); - scoped_ptr<BlobStorageController> blob_storage_controller( - new BlobStorageController()); + BlobStorageController blob_storage_controller; // Test registering a blob URL referring to the blob data containing only // data and file. GURL blob_url1("blob://url_1"); - blob_storage_controller->RegisterBlobUrl(blob_url1, blob_data1); + blob_storage_controller.AddFinishedBlob(blob_url1, blob_data1); BlobData* blob_data_found = - blob_storage_controller->GetBlobDataFromUrl(blob_url1); + blob_storage_controller.GetBlobDataFromUrl(blob_url1); ASSERT_TRUE(blob_data_found != NULL); EXPECT_TRUE(*blob_data_found == *blob_data1); // Test registering a blob URL referring to the blob data containing data, // file and blob. GURL blob_url2("blob://url_2"); - blob_storage_controller->RegisterBlobUrl(blob_url2, blob_data2); + blob_storage_controller.AddFinishedBlob(blob_url2, blob_data2); - blob_data_found = blob_storage_controller->GetBlobDataFromUrl(blob_url2); + blob_data_found = blob_storage_controller.GetBlobDataFromUrl(blob_url2); ASSERT_TRUE(blob_data_found != NULL); EXPECT_TRUE(*blob_data_found == *canonicalized_blob_data2); // Test registering a blob URL referring to existent blob URL. GURL blob_url3("blob://url_3"); - blob_storage_controller->RegisterBlobUrlFrom(blob_url3, blob_url1); + blob_storage_controller.CloneBlob(blob_url3, blob_url1); - blob_data_found = blob_storage_controller->GetBlobDataFromUrl(blob_url3); + blob_data_found = blob_storage_controller.GetBlobDataFromUrl(blob_url3); ASSERT_TRUE(blob_data_found != NULL); EXPECT_TRUE(*blob_data_found == *blob_data1); // Test unregistering a blob URL. - blob_storage_controller->UnregisterBlobUrl(blob_url3); - blob_data_found = blob_storage_controller->GetBlobDataFromUrl(blob_url3); + blob_storage_controller.RemoveBlob(blob_url3); + blob_data_found = blob_storage_controller.GetBlobDataFromUrl(blob_url3); EXPECT_TRUE(!blob_data_found); } @@ -83,38 +82,36 @@ TEST(BlobStorageControllerTest, ResolveBlobReferencesInUploadData) { 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_ptr<BlobStorageController> blob_storage_controller( - new BlobStorageController()); - + BlobStorageController blob_storage_controller; scoped_refptr<BlobData> blob_data(new BlobData()); GURL blob_url0("blob://url_0"); - blob_storage_controller->RegisterBlobUrl(blob_url0, blob_data); + blob_storage_controller.AddFinishedBlob(blob_url0, blob_data); blob_data->AppendData("BlobData"); blob_data->AppendFile( FilePath(FILE_PATH_LITERAL("BlobFile.txt")), 0, 20, time1); GURL blob_url1("blob://url_1"); - blob_storage_controller->RegisterBlobUrl(blob_url1, blob_data); + blob_storage_controller.AddFinishedBlob(blob_url1, blob_data); GURL blob_url2("blob://url_2"); - blob_storage_controller->RegisterBlobUrlFrom(blob_url2, blob_url1); + blob_storage_controller.CloneBlob(blob_url2, blob_url1); GURL blob_url3("blob://url_3"); - blob_storage_controller->RegisterBlobUrlFrom(blob_url3, blob_url2); + blob_storage_controller.CloneBlob(blob_url3, blob_url2); // Setup upload data elements for comparison. UploadData::Element blob_element1, blob_element2; blob_element1.SetToBytes( - blob_data->items().at(0).data().c_str() + - static_cast<int>(blob_data->items().at(0).offset()), - static_cast<int>(blob_data->items().at(0).length())); + blob_data->items().at(0).data.c_str() + + static_cast<int>(blob_data->items().at(0).offset), + static_cast<int>(blob_data->items().at(0).length)); blob_element2.SetToFilePathRange( - blob_data->items().at(1).file_path(), - blob_data->items().at(1).offset(), - blob_data->items().at(1).length(), - blob_data->items().at(1).expected_modification_time()); + blob_data->items().at(1).file_path, + blob_data->items().at(1).offset, + blob_data->items().at(1).length, + blob_data->items().at(1).expected_modification_time); UploadData::Element upload_element1, upload_element2; upload_element1.SetToBytes("Hello", 5); @@ -132,7 +129,7 @@ TEST(BlobStorageControllerTest, ResolveBlobReferencesInUploadData) { upload_element2.file_range_length(), upload_element2.expected_file_modification_time()); - blob_storage_controller->ResolveBlobReferencesInUploadData(upload_data.get()); + blob_storage_controller.ResolveBlobReferencesInUploadData(upload_data.get()); ASSERT_EQ(upload_data->elements()->size(), 2U); EXPECT_TRUE(upload_data->elements()->at(0) == upload_element1); EXPECT_TRUE(upload_data->elements()->at(1) == upload_element2); @@ -141,14 +138,14 @@ TEST(BlobStorageControllerTest, ResolveBlobReferencesInUploadData) { upload_data = new UploadData(); upload_data->AppendBlob(blob_url0); - blob_storage_controller->ResolveBlobReferencesInUploadData(upload_data.get()); + blob_storage_controller.ResolveBlobReferencesInUploadData(upload_data.get()); ASSERT_EQ(upload_data->elements()->size(), 0U); // Test having only one blob reference. upload_data = new UploadData(); upload_data->AppendBlob(blob_url1); - blob_storage_controller->ResolveBlobReferencesInUploadData(upload_data.get()); + blob_storage_controller.ResolveBlobReferencesInUploadData(upload_data.get()); ASSERT_EQ(upload_data->elements()->size(), 2U); EXPECT_TRUE(upload_data->elements()->at(0) == blob_element1); EXPECT_TRUE(upload_data->elements()->at(1) == blob_element2); @@ -165,7 +162,7 @@ TEST(BlobStorageControllerTest, ResolveBlobReferencesInUploadData) { upload_element2.file_range_length(), upload_element2.expected_file_modification_time()); - blob_storage_controller->ResolveBlobReferencesInUploadData(upload_data.get()); + blob_storage_controller.ResolveBlobReferencesInUploadData(upload_data.get()); ASSERT_EQ(upload_data->elements()->size(), 4U); EXPECT_TRUE(upload_data->elements()->at(0) == blob_element1); EXPECT_TRUE(upload_data->elements()->at(1) == blob_element2); @@ -184,7 +181,7 @@ TEST(BlobStorageControllerTest, ResolveBlobReferencesInUploadData) { upload_element2.expected_file_modification_time()); upload_data->AppendBlob(blob_url1); - blob_storage_controller->ResolveBlobReferencesInUploadData(upload_data.get()); + blob_storage_controller.ResolveBlobReferencesInUploadData(upload_data.get()); ASSERT_EQ(upload_data->elements()->size(), 4U); EXPECT_TRUE(upload_data->elements()->at(0) == upload_element1); EXPECT_TRUE(upload_data->elements()->at(1) == upload_element2); @@ -203,7 +200,7 @@ TEST(BlobStorageControllerTest, ResolveBlobReferencesInUploadData) { upload_element2.file_range_length(), upload_element2.expected_file_modification_time()); - blob_storage_controller->ResolveBlobReferencesInUploadData(upload_data.get()); + blob_storage_controller.ResolveBlobReferencesInUploadData(upload_data.get()); ASSERT_EQ(upload_data->elements()->size(), 4U); EXPECT_TRUE(upload_data->elements()->at(0) == upload_element1); EXPECT_TRUE(upload_data->elements()->at(1) == blob_element1); @@ -224,7 +221,7 @@ TEST(BlobStorageControllerTest, ResolveBlobReferencesInUploadData) { upload_element2.file_range_length(), upload_element2.expected_file_modification_time()); - blob_storage_controller->ResolveBlobReferencesInUploadData(upload_data.get()); + blob_storage_controller.ResolveBlobReferencesInUploadData(upload_data.get()); ASSERT_EQ(upload_data->elements()->size(), 8U); EXPECT_TRUE(upload_data->elements()->at(0) == blob_element1); EXPECT_TRUE(upload_data->elements()->at(1) == blob_element2); diff --git a/webkit/blob/blob_url_request_job.cc b/webkit/blob/blob_url_request_job.cc index 312c264..09491e8 100644 --- a/webkit/blob/blob_url_request_job.cc +++ b/webkit/blob/blob_url_request_job.cc @@ -135,10 +135,10 @@ void BlobURLRequestJob::DidResolve(base::PlatformFileError rv, // Note that the expected modification time from WebKit is based on // time_t precision. So we have to convert both to time_t to compare. const BlobData::Item& item = blob_data_->items().at(item_index_); - DCHECK(item.type() == BlobData::TYPE_FILE); + DCHECK(item.type == BlobData::TYPE_FILE); - if (!item.expected_modification_time().is_null() && - item.expected_modification_time().ToTimeT() != + if (!item.expected_modification_time.is_null() && + item.expected_modification_time.ToTimeT() != file_info.last_modified.ToTimeT()) { NotifyFailure(net::ERR_FILE_NOT_FOUND); return; @@ -146,7 +146,7 @@ void BlobURLRequestJob::DidResolve(base::PlatformFileError rv, // If item length is -1, we need to use the file size being resolved // in the real time. - int64 item_length = static_cast<int64>(item.length()); + int64 item_length = static_cast<int64>(item.length); if (item_length == -1) item_length = file_info.size; @@ -162,11 +162,11 @@ void BlobURLRequestJob::DidResolve(base::PlatformFileError rv, void BlobURLRequestJob::CountSize() { for (; item_index_ < blob_data_->items().size(); ++item_index_) { const BlobData::Item& item = blob_data_->items().at(item_index_); - int64 item_length = static_cast<int64>(item.length()); + int64 item_length = static_cast<int64>(item.length); // If there is a file item, do the resolving. - if (item.type() == BlobData::TYPE_FILE) { - ResolveFile(item.file_path()); + if (item.type == BlobData::TYPE_FILE) { + ResolveFile(item.file_path); return; } @@ -286,7 +286,7 @@ bool BlobURLRequestJob::ReadItem() { // Do the reading. const BlobData::Item& item = blob_data_->items().at(item_index_); - switch (item.type()) { + switch (item.type) { case BlobData::TYPE_DATA: return ReadBytes(item); case BlobData::TYPE_FILE: @@ -301,7 +301,7 @@ bool BlobURLRequestJob::ReadBytes(const BlobData::Item& item) { DCHECK(read_buf_remaining_bytes_ >= bytes_to_read_); memcpy(read_buf_->data() + read_buf_offset_, - &item.data().at(0) + item.offset() + current_item_offset_, + &item.data.at(0) + item.offset + current_item_offset_, bytes_to_read_); AdvanceBytesRead(bytes_to_read_); @@ -314,7 +314,7 @@ bool BlobURLRequestJob::DispatchReadFile(const BlobData::Item& item) { return ReadFile(item); base::FileUtilProxy::CreateOrOpen( - file_thread_proxy_, item.file_path(), kFileOpenFlags, + file_thread_proxy_, item.file_path, kFileOpenFlags, base::Bind(&BlobURLRequestJob::DidOpen, weak_factory_.GetWeakPtr())); SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0)); return false; @@ -335,7 +335,7 @@ void BlobURLRequestJob::DidOpen(base::PlatformFileError rv, { // stream_.Seek() blocks the IO thread, see http://crbug.com/75548. base::ThreadRestrictions::ScopedAllowIO allow_io; - int64 offset = current_item_offset_ + static_cast<int64>(item.offset()); + int64 offset = current_item_offset_ + static_cast<int64>(item.offset); if (offset > 0 && offset != stream_->Seek(net::FROM_BEGIN, offset)) { NotifyFailure(net::ERR_FAILED); return; diff --git a/webkit/blob/blob_url_request_job_unittest.cc b/webkit/blob/blob_url_request_job_unittest.cc index e263ca9..07632db 100644 --- a/webkit/blob/blob_url_request_job_unittest.cc +++ b/webkit/blob/blob_url_request_job_unittest.cc @@ -317,13 +317,6 @@ class BlobURLRequestJobTest : public testing::Test { TestErrorRequest(blob_data, 404); } - void TestGetSlicedDataRequest() { - scoped_refptr<BlobData> blob_data(new BlobData()); - blob_data->AppendData(kTestData2, 2, 4); - std::string result(kTestData2 + 2, 4); - TestSuccessRequest(blob_data, result); - } - void TestGetSlicedFileRequest() { scoped_refptr<BlobData> blob_data(new BlobData()); blob_data->AppendFile(temp_file1_, 2, 4, temp_file_modification_time1_); @@ -333,9 +326,9 @@ class BlobURLRequestJobTest : public testing::Test { scoped_refptr<BlobData> BuildComplicatedData(std::string* expected_result) { scoped_refptr<BlobData> blob_data(new BlobData()); - blob_data->AppendData(kTestData1, 1, 2); + blob_data->AppendData(kTestData1 + 1, 2); blob_data->AppendFile(temp_file1_, 2, 3, temp_file_modification_time1_); - blob_data->AppendData(kTestData2, 3, 4); + blob_data->AppendData(kTestData2 + 3, 4); blob_data->AppendFile(temp_file2_, 4, 5, temp_file_modification_time2_); *expected_result = std::string(kTestData1 + 1, 2); *expected_result += std::string(kTestFileData1 + 2, 3); @@ -431,10 +424,6 @@ TEST_F(BlobURLRequestJobTest, TestGetLargeFileRequest) { RunTestOnIOThread(&BlobURLRequestJobTest::TestGetLargeFileRequest); } -TEST_F(BlobURLRequestJobTest, TestGetSlicedDataRequest) { - RunTestOnIOThread(&BlobURLRequestJobTest::TestGetSlicedDataRequest); -} - TEST_F(BlobURLRequestJobTest, TestGetSlicedFileRequest) { RunTestOnIOThread(&BlobURLRequestJobTest::TestGetSlicedFileRequest); } diff --git a/webkit/blob/view_blob_internals_job.cc b/webkit/blob/view_blob_internals_job.cc index 753a6af..dbd491e 100644 --- a/webkit/blob/view_blob_internals_job.cc +++ b/webkit/blob/view_blob_internals_job.cc @@ -135,7 +135,7 @@ void ViewBlobInternalsJob::DoWorkAsync() { std::string blob_url = request_->url().query().substr(strlen("remove=")); blob_url = net::UnescapeURLComponent(blob_url, UnescapeRule::NORMAL | UnescapeRule::URL_SPECIAL_CHARS); - blob_storage_controller_->UnregisterBlobUrl(GURL(blob_url)); + blob_storage_controller_->RemoveBlob(GURL(blob_url)); } StartAsync(); @@ -190,37 +190,38 @@ void ViewBlobInternalsJob::GenerateHTMLForBlobData(const BlobData& blob_data, } const BlobData::Item& item = blob_data.items().at(i); - switch (item.type()) { + switch (item.type) { case BlobData::TYPE_DATA: + case BlobData::TYPE_DATA_EXTERNAL: AddHTMLListItem(kType, "data", out); break; case BlobData::TYPE_FILE: AddHTMLListItem(kType, "file", out); AddHTMLListItem(kPath, #if defined(OS_WIN) - net::EscapeForHTML(WideToUTF8(item.file_path().value())), + net::EscapeForHTML(WideToUTF8(item.file_path.value())), #else - net::EscapeForHTML(item.file_path().value()), + net::EscapeForHTML(item.file_path.value()), #endif out); - if (!item.expected_modification_time().is_null()) { + if (!item.expected_modification_time.is_null()) { AddHTMLListItem(kModificationTime, UTF16ToUTF8( - TimeFormatFriendlyDateAndTime(item.expected_modification_time())), + TimeFormatFriendlyDateAndTime(item.expected_modification_time)), out); } break; case BlobData::TYPE_BLOB: AddHTMLListItem(kType, "blob", out); - AddHTMLListItem(kURL, item.blob_url().spec(), out); + AddHTMLListItem(kURL, item.blob_url.spec(), out); break; } - if (item.offset()) { + if (item.offset) { AddHTMLListItem(kOffset, UTF16ToUTF8(base::FormatNumber( - static_cast<int64>(item.offset()))), out); + static_cast<int64>(item.offset))), out); } - if (static_cast<int64>(item.length()) != -1) { + if (static_cast<int64>(item.length) != -1) { AddHTMLListItem(kLength, UTF16ToUTF8(base::FormatNumber( - static_cast<int64>(item.length()))), out); + static_cast<int64>(item.length))), out); } if (has_multi_items) diff --git a/webkit/fileapi/file_system_operation_write_unittest.cc b/webkit/fileapi/file_system_operation_write_unittest.cc index f434042..8a00927 100644 --- a/webkit/fileapi/file_system_operation_write_unittest.cc +++ b/webkit/fileapi/file_system_operation_write_unittest.cc @@ -8,6 +8,8 @@ // TYPE_UI, which URLRequest doesn't allow. // +#include <vector> + #include "base/memory/scoped_ptr.h" #include "base/message_loop.h" #include "base/message_loop.h" @@ -223,14 +225,14 @@ TEST_F(FileSystemOperationWriteTest, TestWriteSuccess) { scoped_refptr<TestURLRequestContext> url_request_context( new TestURLRequestContext()); - url_request_context->blob_storage_controller()-> - RegisterBlobUrl(blob_url, blob_data); + url_request_context->blob_storage_controller()->AddFinishedBlob( + blob_url, blob_data); operation()->Write(url_request_context, URLForPath(virtual_path_), blob_url, 0); MessageLoop::current()->Run(); - url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); + url_request_context->blob_storage_controller()->RemoveBlob(blob_url); EXPECT_EQ(14, bytes_written()); EXPECT_EQ(base::PLATFORM_FILE_OK, status()); @@ -244,14 +246,14 @@ TEST_F(FileSystemOperationWriteTest, TestWriteZero) { scoped_refptr<TestURLRequestContext> url_request_context( new TestURLRequestContext()); - url_request_context->blob_storage_controller()-> - RegisterBlobUrl(blob_url, blob_data); + url_request_context->blob_storage_controller()->AddFinishedBlob( + blob_url, blob_data); operation()->Write(url_request_context, URLForPath(virtual_path_), blob_url, 0); MessageLoop::current()->Run(); - url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); + url_request_context->blob_storage_controller()->RemoveBlob(blob_url); EXPECT_EQ(0, bytes_written()); EXPECT_EQ(base::PLATFORM_FILE_OK, status()); @@ -278,15 +280,15 @@ TEST_F(FileSystemOperationWriteTest, TestWriteInvalidFile) { scoped_refptr<TestURLRequestContext> url_request_context( new TestURLRequestContext()); - url_request_context->blob_storage_controller()-> - RegisterBlobUrl(blob_url, blob_data); + url_request_context->blob_storage_controller()->AddFinishedBlob( + blob_url, blob_data); operation()->Write(url_request_context, URLForPath(FilePath(FILE_PATH_LITERAL("nonexist"))), blob_url, 0); MessageLoop::current()->Run(); - url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); + url_request_context->blob_storage_controller()->RemoveBlob(blob_url); EXPECT_EQ(0, bytes_written()); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); @@ -306,14 +308,14 @@ TEST_F(FileSystemOperationWriteTest, TestWriteDir) { scoped_refptr<TestURLRequestContext> url_request_context( new TestURLRequestContext()); - url_request_context->blob_storage_controller()-> - RegisterBlobUrl(blob_url, blob_data); + url_request_context->blob_storage_controller()->AddFinishedBlob( + blob_url, blob_data); operation()->Write(url_request_context, URLForPath(virtual_subdir_path), blob_url, 0); MessageLoop::current()->Run(); - url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); + url_request_context->blob_storage_controller()->RemoveBlob(blob_url); EXPECT_EQ(0, bytes_written()); EXPECT_EQ(base::PLATFORM_FILE_ERROR_ACCESS_DENIED, status()); @@ -327,15 +329,15 @@ TEST_F(FileSystemOperationWriteTest, TestWriteFailureByQuota) { scoped_refptr<TestURLRequestContext> url_request_context( new TestURLRequestContext()); - url_request_context->blob_storage_controller()-> - RegisterBlobUrl(blob_url, blob_data); + url_request_context->blob_storage_controller()->AddFinishedBlob( + blob_url, blob_data); quota_manager_->set_quota(10); operation()->Write(url_request_context, URLForPath(virtual_path_), blob_url, 0); MessageLoop::current()->Run(); - url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); + url_request_context->blob_storage_controller()->RemoveBlob(blob_url); EXPECT_EQ(10, bytes_written()); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NO_SPACE, status()); diff --git a/webkit/tools/test_shell/test_shell_webblobregistry_impl.cc b/webkit/tools/test_shell/test_shell_webblobregistry_impl.cc index 6ecdc4f..c376055 100644 --- a/webkit/tools/test_shell/test_shell_webblobregistry_impl.cc +++ b/webkit/tools/test_shell/test_shell_webblobregistry_impl.cc @@ -6,35 +6,21 @@ #include "base/bind.h" #include "base/message_loop.h" -#include "base/task.h" #include "googleurl/src/gurl.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebBlobData.h" -#include "third_party/WebKit/Source/WebKit/chromium/public/WebCString.h" -#include "third_party/WebKit/Source/WebKit/chromium/public/WebString.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebURL.h" #include "webkit/blob/blob_data.h" #include "webkit/blob/blob_storage_controller.h" using WebKit::WebBlobData; -using WebKit::WebString; using WebKit::WebURL; +using webkit_blob::BlobData; namespace { MessageLoop* g_io_thread; webkit_blob::BlobStorageController* g_blob_storage_controller; -// WebURL contains a WebCString object that is ref-counted, -// but not thread-safe ref-counted. -// "Normal" copying of WebURL results in a copy that is not thread-safe. -// This method creates a deep copy of WebURL. -WebURL GetWebURLThreadsafeCopy(const WebURL& source) { - const WebKit::WebCString spec(source.spec().data(), source.spec().length()); - const url_parse::Parsed& parsed(source.parsed()); - const bool is_valid = source.isValid(); - return WebURL(spec, parsed, is_valid); -} - } // namespace /* static */ @@ -56,63 +42,43 @@ TestShellWebBlobRegistryImpl::TestShellWebBlobRegistryImpl() { void TestShellWebBlobRegistryImpl::registerBlobURL( const WebURL& url, WebBlobData& data) { DCHECK(g_io_thread); - base::Closure task; - { - scoped_refptr<webkit_blob::BlobData> blob_data( - new webkit_blob::BlobData(data)); - WebURL url_copy = GetWebURLThreadsafeCopy(url); - task = base::Bind(&TestShellWebBlobRegistryImpl::DoRegisterBlobUrl, this, - url_copy, blob_data); - // After this block exits, url_copy is disposed, and - // the underlying WebCString will have a refcount=1 and will - // only be accessible from the task object. - } - g_io_thread->PostTask(FROM_HERE, task); + GURL thread_safe_url = url; // WebURL uses refcounted strings. + g_io_thread->PostTask(FROM_HERE, base::Bind( + &TestShellWebBlobRegistryImpl::AddFinishedBlob, this, + thread_safe_url, make_scoped_refptr(new BlobData(data)))); } void TestShellWebBlobRegistryImpl::registerBlobURL( const WebURL& url, const WebURL& src_url) { DCHECK(g_io_thread); - base::Closure task; - { - WebURL url_copy = GetWebURLThreadsafeCopy(url); - WebURL src_url_copy = GetWebURLThreadsafeCopy(src_url); - task = base::Bind(&TestShellWebBlobRegistryImpl::DoRegisterBlobUrlFrom, - this, url_copy, src_url_copy); - // After this block exits, url_copy and src_url_copy are disposed, and - // the underlying WebCStrings will have a refcount=1 and will - // only be accessible from the task object. - } - g_io_thread->PostTask(FROM_HERE, task); + GURL thread_safe_url = url; + GURL thread_safe_src_url = src_url; + g_io_thread->PostTask(FROM_HERE, base::Bind( + &TestShellWebBlobRegistryImpl::CloneBlob, this, + thread_safe_url, thread_safe_src_url)); } void TestShellWebBlobRegistryImpl::unregisterBlobURL(const WebURL& url) { DCHECK(g_io_thread); - base::Closure task; - { - WebURL url_copy = GetWebURLThreadsafeCopy(url); - task = base::Bind(&TestShellWebBlobRegistryImpl::DoUnregisterBlobUrl, this, - url_copy); - // After this block exits, url_copy is disposed, and - // the underlying WebCString will have a refcount=1 and will - // only be accessible from the task object. - } - g_io_thread->PostTask(FROM_HERE, task); + GURL thread_safe_url = url; + g_io_thread->PostTask(FROM_HERE, base::Bind( + &TestShellWebBlobRegistryImpl::RemoveBlob, this, + thread_safe_url)); } -void TestShellWebBlobRegistryImpl::DoRegisterBlobUrl( - const GURL& url, webkit_blob::BlobData* blob_data) { +void TestShellWebBlobRegistryImpl::AddFinishedBlob( + const GURL& url, BlobData* blob_data) { DCHECK(g_blob_storage_controller); - g_blob_storage_controller->RegisterBlobUrl(url, blob_data); + g_blob_storage_controller->AddFinishedBlob(url, blob_data); } -void TestShellWebBlobRegistryImpl::DoRegisterBlobUrlFrom( +void TestShellWebBlobRegistryImpl::CloneBlob( const GURL& url, const GURL& src_url) { DCHECK(g_blob_storage_controller); - g_blob_storage_controller->RegisterBlobUrlFrom(url, src_url); + g_blob_storage_controller->CloneBlob(url, src_url); } -void TestShellWebBlobRegistryImpl::DoUnregisterBlobUrl(const GURL& url) { +void TestShellWebBlobRegistryImpl::RemoveBlob(const GURL& url) { DCHECK(g_blob_storage_controller); - g_blob_storage_controller->UnregisterBlobUrl(url); + g_blob_storage_controller->RemoveBlob(url); } diff --git a/webkit/tools/test_shell/test_shell_webblobregistry_impl.h b/webkit/tools/test_shell/test_shell_webblobregistry_impl.h index 090a4e9..d5fa96c 100644 --- a/webkit/tools/test_shell/test_shell_webblobregistry_impl.h +++ b/webkit/tools/test_shell/test_shell_webblobregistry_impl.h @@ -32,15 +32,14 @@ class TestShellWebBlobRegistryImpl const WebKit::WebURL& src_url); virtual void unregisterBlobURL(const WebKit::WebURL& url); - // Run on I/O thread. - void DoRegisterBlobUrl(const GURL& url, webkit_blob::BlobData* blob_data); - void DoRegisterBlobUrlFrom(const GURL& url, const GURL& src_url); - void DoUnregisterBlobUrl(const GURL& url); - - protected: + private: friend class base::RefCountedThreadSafe<TestShellWebBlobRegistryImpl>; - private: + // Run on I/O thread. + void AddFinishedBlob(const GURL& url, webkit_blob::BlobData* blob_data); + void CloneBlob(const GURL& url, const GURL& src_url); + void RemoveBlob(const GURL& url); + DISALLOW_COPY_AND_ASSIGN(TestShellWebBlobRegistryImpl); }; |