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 312e04dd4fd479c19c911cdf6cfa195e574d5308..be64cdf0632ca1d76c4d5e69d57c43af6ffaaf9c 100644
--- a/content/browser/fileapi/fileapi_message_filter.cc
+++ b/content/browser/fileapi/fileapi_message_filter.cc
@@ -12,15 +12,18 @@
 #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 "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);
+}

[kinuko, 2013/07/23 13:57:23]

nit: please match the method order in .cc and .h (i.e. this needs to be placed
after HasPermissionsForFile)

[tyoshino (SeeGerritForStatus), 2013/07/24 12:14:20]

Done.

+
 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();
@@ -168,12 +186,16 @@ bool FileAPIMessageFilter::OnMessageReceived(
     IPC_MESSAGE_HANDLER(FileSystemHostMsg_SyncGetPlatformPath,
                         OnSyncGetPlatformPath)
     IPC_MESSAGE_HANDLER(BlobHostMsg_StartBuildingBlob, OnStartBuildingBlob)
-    IPC_MESSAGE_HANDLER(BlobHostMsg_AppendBlobDataItem, OnAppendBlobDataItem)
-    IPC_MESSAGE_HANDLER(BlobHostMsg_SyncAppendSharedMemory,
-                        OnAppendSharedMemory)
+    IPC_MESSAGE_HANDLER(BlobHostMsg_StartBuildingStream, OnStartBuildingStream)
+    IPC_MESSAGE_HANDLER(BlobHostMsg_AppendBlobDataItemToBlobOrStream,
+                        OnAppendBlobDataItemToBlobOrStream)
+    IPC_MESSAGE_HANDLER(BlobHostMsg_SyncAppendSharedMemoryToBlobOrStream,
+                        OnAppendSharedMemoryToBlobOrStream)
     IPC_MESSAGE_HANDLER(BlobHostMsg_FinishBuildingBlob, OnFinishBuildingBlob)
-    IPC_MESSAGE_HANDLER(BlobHostMsg_CloneBlob, OnCloneBlob)
-    IPC_MESSAGE_HANDLER(BlobHostMsg_RemoveBlob, OnRemoveBlob)
+    IPC_MESSAGE_HANDLER(BlobHostMsg_FinishBuildingStream,
+                        OnFinishBuildingStream)
+    IPC_MESSAGE_HANDLER(BlobHostMsg_CloneBlobOrStream, OnCloneBlobOrStream)
+    IPC_MESSAGE_HANDLER(BlobHostMsg_RemoveBlobOrStream, OnRemoveBlobOrStream)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP_EX()
   return handled;
@@ -519,22 +541,53 @@ void FileAPIMessageFilter::OnStartBuildingBlob(const GURL& url) {
   blob_urls_.insert(url.spec());
 }
 
-void FileAPIMessageFilter::OnAppendBlobDataItem(
+// 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(

[Tom Sepez, 2013/07/23 18:08:25]

What happens if a compromised renderer violates this assumption? DCHECK isn't
strong enough to protect us here in the absence of other checks later on.
Perhaps call BadMessageReceived here.

[tyoshino (SeeGerritForStatus), 2013/07/24 12:14:20]

Done.

+      url.path(), "blobinternal%3A///", true /* case_sensitive*/));

[kinuko, 2013/07/23 13:57:23]

nit: space between case_sensitive and '*/'

[tyoshino (SeeGerritForStatus), 2013/07/24 12:14:20]

Done.

+  // 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 */,

[Tom Sepez, 2013/07/23 18:08:25]

Does this mean all streams share the same origin?  

[tyoshino (SeeGerritForStatus), 2013/07/24 12:14:20]

Yes. But we have another origin checking system.

That said, I started thinking this harmful. I wrote a CL to remove
security_origin member variable.
https://codereview.chromium.org/19798012/

[tyoshino (SeeGerritForStatus), 2013/07/29 07:44:00]

Merged the fix.

+             url);
+  blob_urls_.insert(url.spec());
+}
+
+void FileAPIMessageFilter::OnAppendBlobDataItemToBlobOrStream(
     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;
+    }
+    stream->AddData(item.bytes(), item.length());
+
+    return;
+  }
+
   if (item.type() == BlobData::Item::TYPE_FILE_FILESYSTEM) {
     base::PlatformFileError error;
     FileSystemURL filesystem_url(context_->CrackURL(item.url()));
     if (!HasPermissionsForFile(filesystem_url,
                                fileapi::kReadFilePermissions, &error)) {
-      OnRemoveBlob(url);
+      OnRemoveBlobOrStream(url);
       return;
     }
   }
   if (item.type() == BlobData::Item::TYPE_FILE &&
       !ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile(
           process_id_, item.path())) {
-    OnRemoveBlob(url);
+    OnRemoveBlobOrStream(url);
     return;
   }
   if (item.length() == 0) {
@@ -544,7 +597,7 @@ void FileAPIMessageFilter::OnAppendBlobDataItem(
   blob_storage_context_->controller()->AppendBlobDataItem(url, item);
 }
 
-void FileAPIMessageFilter::OnAppendSharedMemory(
+void FileAPIMessageFilter::OnAppendSharedMemoryToBlobOrStream(
     const GURL& url, base::SharedMemoryHandle handle, size_t buffer_size) {
   DCHECK(base::SharedMemory::IsHandleValid(handle));

[Tom Sepez, 2013/07/23 18:08:25]

What happens if a compromised renderer either lies about buffer_size, or passes
back one of its handles not associated with shared memory?

[tyoshino (SeeGerritForStatus), 2013/07/24 12:14:20]

Filed a bug and CC-ed you.

   if (!buffer_size) {
@@ -557,7 +610,13 @@ void FileAPIMessageFilter::OnAppendSharedMemory(
   base::SharedMemory shared_memory(handle, true);
 #endif
   if (!shared_memory.Map(buffer_size)) {
-    OnRemoveBlob(url);
+    OnRemoveBlobOrStream(url);
+    return;
+  }
+
+  scoped_refptr<Stream> stream(GetStreamForURL(url));
+  if (stream.get()) {
+    stream->AddData(static_cast<char*>(shared_memory.memory()), buffer_size);
     return;
   }
 
@@ -573,16 +632,29 @@ void FileAPIMessageFilter::OnFinishBuildingBlob(
   blob_storage_context_->controller()->FinishBuildingBlob(url, content_type);
 }
 
-void FileAPIMessageFilter::OnCloneBlob(
+void FileAPIMessageFilter::OnFinishBuildingStream(const GURL& url) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  scoped_refptr<Stream> stream(GetStreamForURL(url));
+  if (stream.get())
+    stream->Finalize();

[kinuko, 2013/07/23 13:57:23]

Should we DCHECK(stream.get()) here?

[tyoshino (SeeGerritForStatus), 2013/07/24 12:14:20]

Added NOTREACHED

+}
+
+void FileAPIMessageFilter::OnCloneBlobOrStream(
     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) {
+void FileAPIMessageFilter::OnRemoveBlobOrStream(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());
 }