Add Stream support to WebBlobRegistry and FileAPIMessageFilter.

content/browser/fileapi/fileapi_message_filter.cc

Index: content/browser/fileapi/fileapi_message_filter.cc
diff --git a/content/browser/fileapi/fileapi_message_filter.cc b/content/browser/fileapi/fileapi_message_filter.cc
index 13d887d81f8bda4e8b3e7fd0477a4f890121ca23..1b808355897d9390ef57761988c1780f0e39407f 100644
--- a/content/browser/fileapi/fileapi_message_filter.cc
+++ b/content/browser/fileapi/fileapi_message_filter.cc
@@ -12,16 +12,19 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/platform_file.h"
 #include "base/sequenced_task_runner.h"
+#include "base/strings/string_util.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/fileapi/browser_file_system_helper.h"
 #include "content/browser/fileapi/chrome_blob_storage_context.h"
+#include "content/browser/streams/stream_registry.h"
 #include "content/common/fileapi/file_system_messages.h"
 #include "content/common/fileapi/webblob_messages.h"
 #include "content/public/browser/user_metrics.h"
 #include "googleurl/src/gurl.h"
 #include "ipc/ipc_platform_file.h"
+#include "net/base/io_buffer.h"
 #include "net/base/mime_util.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
@@ -64,29 +67,35 @@ FileAPIMessageFilter::FileAPIMessageFilter(
     int process_id,
     net::URLRequestContextGetter* request_context_getter,
     fileapi::FileSystemContext* file_system_context,
-    ChromeBlobStorageContext* blob_storage_context)
+    ChromeBlobStorageContext* blob_storage_context,
+    StreamContext* stream_context)
     : process_id_(process_id),
       context_(file_system_context),
       request_context_getter_(request_context_getter),
       request_context_(NULL),
-      blob_storage_context_(blob_storage_context) {
+      blob_storage_context_(blob_storage_context),
+      stream_context_(stream_context) {
   DCHECK(context_);
   DCHECK(request_context_getter_.get());
   DCHECK(blob_storage_context);
+  DCHECK(stream_context);
 }
 
 FileAPIMessageFilter::FileAPIMessageFilter(
     int process_id,
     net::URLRequestContext* request_context,
     fileapi::FileSystemContext* file_system_context,
-    ChromeBlobStorageContext* blob_storage_context)
+    ChromeBlobStorageContext* blob_storage_context,
+    StreamContext* stream_context)
     : process_id_(process_id),
       context_(file_system_context),
       request_context_(request_context),
-      blob_storage_context_(blob_storage_context) {
+      blob_storage_context_(blob_storage_context),
+      stream_context_(stream_context) {
   DCHECK(context_);
   DCHECK(request_context_);
   DCHECK(blob_storage_context);
+  DCHECK(stream_context);
 }
 
 void FileAPIMessageFilter::OnChannelConnected(int32 peer_pid) {
@@ -100,6 +109,10 @@ void FileAPIMessageFilter::OnChannelConnected(int32 peer_pid) {
   }
 }
 
+scoped_refptr<Stream> FileAPIMessageFilter::GetStreamForURL(const GURL& url) {
+  return stream_context_->registry()->GetStream(url);
+}
+
 void FileAPIMessageFilter::OnChannelClosing() {
   BrowserMessageFilter::OnChannelClosing();
 
@@ -107,7 +120,12 @@ void FileAPIMessageFilter::OnChannelClosing() {
   // process.
   for (base::hash_set<std::string>::const_iterator iter = blob_urls_.begin();
        iter != blob_urls_.end(); ++iter) {
-    blob_storage_context_->controller()->RemoveBlob(GURL(*iter));
+    const GURL url(*iter);
+    // Intercept stream URLs.
+    if (GetStreamForURL(url))
+      stream_context_->registry()->UnregisterStream(url);
+    else
+      blob_storage_context_->controller()->RemoveBlob(url);
   }
 
   in_transit_snapshot_files_.clear();
@@ -165,6 +183,7 @@ bool FileAPIMessageFilter::OnMessageReceived(
     IPC_MESSAGE_HANDLER(FileSystemHostMsg_SyncGetPlatformPath,
                         OnSyncGetPlatformPath)
     IPC_MESSAGE_HANDLER(BlobHostMsg_StartBuildingBlob, OnStartBuildingBlob)
+    IPC_MESSAGE_HANDLER(BlobHostMsg_StartBuildingStream, OnStartBuildingStream)

[kinuko, 2013/07/18 06:06:55]

Sometimes we interchangeably use the term 'blob' and 'stream' (especially when
talking about BlobURL) but sometimes not, which confuses me a bit.

We seem to start building a stream by OnStartBuildingStream, but finish it with
OnFinishBuildingBlob, which feels weird to me.

Similarly OnAppendBlobDataItem can also add data to a stream feels confusing.

If 'Stream' is a blob I feel OnStartBuildingStream should be named in the same
manner, like OnStartBuildingBlobForStream, and the receiving side should have a
single thin 'BlobURLRegistry' or 'BlobURLHandler' class which performs the
actual demux for blob_storage_context and stream_context internally (so that
FileAPIMessageHandler can just focus on IPC message handling).

Otherwise I'd probably add separate IPC messages/stub methods for
OnAppendStreamDataItem, OnFinishBuildingStream and separate the process more
clearly.
 

[tyoshino (SeeGerritForStatus), 2013/07/23 07:27:34]

Darin's idea was making Blob a subclass of the Stream. So, following this idea,
ideally and ultimately .*Blob should be renamed to .*Stream.

But the discussion is suspended. It's too early to rename them to .*Stream. I
think
it's reasonable to use .*Blob as namespace for both Blob and Stream for a while.
OK. Let me add BlobHostMsg_FinishBuildingStream as
BlobHostMsg_FinishBuildingBlob
and BlobHostMsg_FinishBuildingStream have different parameters.

Re: Clone and Remove, may I rename them by removing Blob from them? They're
common
between Blob and Stream and have the same parameters as well as
AppendBlobDataItem and
SyncAppendSharedMemory.
I don't think it's so much confusing. BlobDataItem means what data source is.
Not the type of objective of the operation. Of course, it's better if we could
give more generic name to BlobDataItem.
Sounds good refactoring.

[kinuko, 2013/07/23 08:07:42]

This sounds good to me.

(I initially thought we could probably just rename StartBuildingStream to
StartBuildingBlobForStream to clarify that we use the term 'blob' for stream and
keep everything else as is, but if it doesn't work for you)
Does it make sense to simply name them to like: OnCloneBlobOrStream,
OnRemoveBlobOrStream?

(I'm afraid Remove conflicts with Remove for FileSystem API. Msg names are ok as
they're prefixed, but method names are problematic)
Ok, works for me.

[tyoshino (SeeGerritForStatus), 2013/07/23 11:54:38]

Done
Yes..

     IPC_MESSAGE_HANDLER(BlobHostMsg_AppendBlobDataItem, OnAppendBlobDataItem)
     IPC_MESSAGE_HANDLER(BlobHostMsg_SyncAppendSharedMemory,
                         OnAppendSharedMemory)
@@ -516,9 +535,42 @@ void FileAPIMessageFilter::OnStartBuildingBlob(const GURL& url) {
   blob_urls_.insert(url.spec());
 }
 
+// Currently |content_type| is ignored inside Chromium.
+//
+// TODO(tyoshino): Set |content_type| to the stream.
+void FileAPIMessageFilter::OnStartBuildingStream(
+    const GURL& url, const std::string& content_type) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  // Only an internal Blob URL is expected here. See the BlobURL of the Blink.
+  DCHECK(StartsWithASCII(
+      url.path(), "blobinternal%3A///", true /* case_sensitive*/));
+  // Use an empty security origin for now. Stream accepts a security origin
+  // but how it's handled is not fixed yet.
+  new Stream(stream_context_->registry(),
+             NULL /* write_observer */,
+             GURL() /* security_origin */,
+             url);
+  blob_urls_.insert(url.spec());
+}
+
 void FileAPIMessageFilter::OnAppendBlobDataItem(
     const GURL& url, const BlobData::Item& item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  scoped_refptr<Stream> stream(GetStreamForURL(url));
+  if (stream.get()) {
+    // Data for stream is delivered as TYPE_BYTES item.
+    if (item.type() != BlobData::Item::TYPE_BYTES) {
+      BadMessageReceived();
+      return;
+    }
+    scoped_refptr<net::IOBuffer> buffer(new net::IOBuffer(item.length()));
+    memcpy(buffer->data(), item.bytes(), item.length());
+    stream->AddData(buffer, item.length());
+
+    return;
+  }
+
   if (item.type() == BlobData::Item::TYPE_FILE_FILESYSTEM) {
     base::PlatformFileError error;
     FileSystemURL filesystem_url(context_->CrackURL(item.url()));
@@ -558,6 +610,15 @@ void FileAPIMessageFilter::OnAppendSharedMemory(
     return;
   }
 
+  scoped_refptr<Stream> stream(GetStreamForURL(url));
+  if (stream.get()) {
+    scoped_refptr<net::IOBuffer> buffer(new net::IOBuffer(buffer_size));
+    memcpy(buffer->data(), static_cast<char*>(shared_memory.memory()),
+           buffer_size);
+    stream->AddData(buffer, buffer_size);

[kinuko, 2013/07/18 06:06:55]

nit: it might be easier to read if Stream class just has a method to add bytes
data (which internally creates IOBuffer and takes its ownership)

[tyoshino (SeeGerritForStatus), 2013/07/23 11:54:38]

Done.

+    return;
+  }
+
   BlobData::Item item;
   item.SetToSharedBytes(static_cast<char*>(shared_memory.memory()),
                         buffer_size);
@@ -567,19 +628,29 @@ void FileAPIMessageFilter::OnAppendSharedMemory(
 void FileAPIMessageFilter::OnFinishBuildingBlob(
     const GURL& url, const std::string& content_type) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  blob_storage_context_->controller()->FinishBuildingBlob(url, content_type);
+  scoped_refptr<Stream> stream(GetStreamForURL(url));
+  if (stream.get())
+    stream->Finalize();
+  else
+    blob_storage_context_->controller()->FinishBuildingBlob(url, content_type);
 }
 
 void FileAPIMessageFilter::OnCloneBlob(
     const GURL& url, const GURL& src_url) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  blob_storage_context_->controller()->CloneBlob(url, src_url);
+  if (GetStreamForURL(src_url))
+    stream_context_->registry()->CloneStream(url, src_url);
+  else
+    blob_storage_context_->controller()->CloneBlob(url, src_url);
   blob_urls_.insert(url.spec());
 }
 
 void FileAPIMessageFilter::OnRemoveBlob(const GURL& url) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  blob_storage_context_->controller()->RemoveBlob(url);
+  if (GetStreamForURL(url).get())
+    stream_context_->registry()->UnregisterStream(url);
+  else
+    blob_storage_context_->controller()->RemoveBlob(url);
   blob_urls_.erase(url.spec());
 }