Media Galleries API: Audio/Video attached pictures support.

chrome/browser/extensions/api/media_galleries/media_galleries_api.cc

Index: chrome/browser/extensions/api/media_galleries/media_galleries_api.cc
diff --git a/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc b/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc
index 171111be5ceb700e63e902559bdf9fec49f38cab..bea2740bab6a9dbef289e25a084f8298485e6587 100644
--- a/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc
+++ b/chrome/browser/extensions/api/media_galleries/media_galleries_api.cc
@@ -13,7 +13,9 @@
 #include "apps/app_window.h"
 #include "apps/app_window_registry.h"
 #include "base/callback.h"
+#include "base/guid.h"
 #include "base/lazy_instance.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/platform_file.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
@@ -37,8 +39,10 @@
 #include "chrome/common/pref_names.h"
 #include "components/storage_monitor/storage_info.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
+#include "content/browser/fileapi/blob_storage_host.h"

[tommycli, 2014/04/29 23:29:26]

This violates the include rules right now. I imagine I would have to move
blob_storage_host.h to content/public, and then make a
blob_storage_host_impl.cc/h?

Does that sound legit to you?

[michaeln, 2014/05/01 21:27:35]

We need to expose an interface in the content api to do this, but it should be
an api to the per-profile BlobStorageContext class instead of the per-process
BlobStorageHost class to better match how we've exposed other components.

 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_security_policy.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
@@ -53,6 +57,7 @@
 #include "grit/generated_resources.h"
 #include "net/base/mime_sniffer.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "webkit/common/blob/blob_data.h"
 
 using content::WebContents;
 using storage_monitor::MediaStorageUtil;
@@ -84,6 +89,12 @@ const char kIsMediaDeviceKey[] = "isMediaDevice";
 const char kIsRemovableKey[] = "isRemovable";
 const char kNameKey[] = "name";
 
+const char kMetadataKey[] = "metadata";
+const char kAttachedImagesKey[] = "attachedImages";
+const char kBlobUUIDKey[] = "blobUUID";
+const char kTypeKey[] = "type";
+const char kSizeKey[] = "size";
+
 MediaFileSystemRegistry* media_file_system_registry() {
   return g_browser_process->media_file_system_registry();
 }
@@ -826,28 +837,35 @@ bool MediaGalleriesGetMetadataFunction::RunImpl() {
   bool mime_type_only = options->metadata_type ==
       MediaGalleries::GET_METADATA_TYPE_MIMETYPEONLY;
 
+  // Get attached images by default.
+  bool get_attached_images =
+      options->metadata_type == MediaGalleries::GET_METADATA_TYPE_ALL ||
+      options->metadata_type == MediaGalleries::GET_METADATA_TYPE_NONE;
+
   return Setup(GetProfile(), &error_, base::Bind(
       &MediaGalleriesGetMetadataFunction::OnPreferencesInit, this,
-      mime_type_only, blob_uuid));
+      mime_type_only, get_attached_images, blob_uuid));
 }
 
 void MediaGalleriesGetMetadataFunction::OnPreferencesInit(
-    bool mime_type_only, const std::string& blob_uuid) {
+    bool mime_type_only, bool get_attached_images,
+    const std::string& blob_uuid) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // BlobReader is self-deleting.
   BlobReader* reader = new BlobReader(
       GetProfile(),
       blob_uuid,
-      base::Bind(&MediaGalleriesGetMetadataFunction::SniffMimeType, this,
-                 mime_type_only, blob_uuid));
+      base::Bind(&MediaGalleriesGetMetadataFunction::GetMetadata, this,
+                 mime_type_only, get_attached_images, blob_uuid));
   reader->SetByteRange(0, net::kMaxBytesToSniff);
   reader->Start();
 }
 
-void MediaGalleriesGetMetadataFunction::SniffMimeType(
-    bool mime_type_only, const std::string& blob_uuid,
-    scoped_ptr<std::string> blob_header, int64 total_blob_length) {
+void MediaGalleriesGetMetadataFunction::GetMetadata(
+    bool mime_type_only, bool get_attached_images,
+    const std::string& blob_uuid, scoped_ptr<std::string> blob_header,
+    int64 total_blob_length) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   std::string mime_type;
@@ -862,26 +880,101 @@ void MediaGalleriesGetMetadataFunction::SniffMimeType(
   if (mime_type_only) {
     MediaGalleries::MediaMetadata metadata;
     metadata.mime_type = mime_type;
-    SetResult(metadata.ToValue().release());
+
+    base::DictionaryValue* result_dictionary = new base::DictionaryValue;
+    result_dictionary->Set(kMetadataKey, metadata.ToValue().release());
+    SetResult(result_dictionary);
     SendResponse(true);
     return;
   }
 
   scoped_refptr<metadata::SafeMediaMetadataParser> parser(
       new metadata::SafeMediaMetadataParser(GetProfile(), blob_uuid,
-                                            total_blob_length, mime_type));
+                                            total_blob_length, mime_type,
+                                            get_attached_images));
   parser->Start(base::Bind(
       &MediaGalleriesGetMetadataFunction::OnSafeMediaMetadataParserDone, this));
 }
 
 void MediaGalleriesGetMetadataFunction::OnSafeMediaMetadataParserDone(
-    bool parse_success, base::DictionaryValue* metadata_dictionary) {
+    bool parse_success, base::DictionaryValue* metadata_dictionary,
+    const std::vector<metadata::AttachedImage>& attached_images) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
   if (!parse_success) {
     SendResponse(false);
     return;
   }
 
-  SetResult(metadata_dictionary->DeepCopy());
+  if (attached_images.empty()) {
+    base::DictionaryValue* result_dictionary = new base::DictionaryValue;
+    result_dictionary->Set(kMetadataKey, metadata_dictionary->DeepCopy());
+    SetResult(result_dictionary);
+    SendResponse(true);
+    return;
+  }
+
+  content::BrowserThread::PostTaskAndReplyWithResult(
+      content::BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&MediaGalleriesGetMetadataFunction::CreateBlobsOnIOThread,
+                 this, attached_images),

[michaeln, 2014/05/01 21:27:35]

Kinda unrelated to the blob handling specifically, but of |attached_image| is as
large as you say, it might be better to make fewer copies of that data in the
browser process. As coded here i think i see three copies: the caller has a
copy, each of these two Bind closures creates their own copy.

[tommycli, 2014/05/07 21:39:04]

Done.

+      base::Bind(&MediaGalleriesGetMetadataFunction::FinishWithBlobsOnUIThread,
+                 this, base::Owned(metadata_dictionary->DeepCopy()),
+                 attached_images));
+}
+
+std::vector<std::string>
+MediaGalleriesGetMetadataFunction::CreateBlobsOnIOThread(
+    const std::vector<metadata::AttachedImage>& attached_images) {
+  content::BlobStorageHost* blob_storage =
+      render_view_host()->GetProcess()->GetBlobStorageHost();
+  std::vector<std::string> blob_uuids;
+
+  for (std::vector<metadata::AttachedImage>::const_iterator it =
+           attached_images.begin();
+       it != attached_images.end(); ++it) {
+    // Here we notably use a UUID generated in the browser process instead of

[tommycli, 2014/04/29 23:29:26]

I generate a UUID on the browser process. Passes my browser test, but not sure
if it's kosher.

[michaeln, 2014/05/01 21:27:35]

This is perfectly halal.

+    // in WebKit. It is upper case and doesn't conform to UUID 4 specifications,
+    // but this doesn't seem to cause any harm.
+    std::string uuid = base::GenerateGUID();
+    BlobData::Item item;
+    item.SetToBytes(it->data.c_str(), it->data.size());
+
+    // These should never return false.
+    CHECK(blob_storage->StartBuildingBlob(uuid));

[michaeln, 2014/05/01 21:27:35]

The BlobStorageContext::AddFinishedBlob(...) method is more intended to support
blob creation and lifetime management in the browser process.

  // Useful for coining blobs from within the browser process. If the
  // blob cannot be added due to memory consumption, returns NULL.
  scoped_ptr<BlobDataHandle> AddFinishedBlob(const BlobData* blob_data);

I think the content api we expose should basically be a wrapper around that
method that's callable from the UI thread. A wrapper that i think would work for
your use case...

  typedef base::Callback<void(scoped_ptr<BlobDataHandle>)>
      BlobCallback;

  void CreateMemoryBackedBlob(
      const char* data, size_t length,
      BlobCallback);

We can add others as needed like CreateFileBackedBlob(path).

[tommycli, 2014/05/07 21:39:04]

Done.

+    CHECK(blob_storage->AppendBlobDataItem(uuid, item));
+    CHECK(blob_storage->FinishBuildingBlob(uuid, it->type));
+
+    blob_uuids.push_back(uuid);
+  }
+
+  return blob_uuids;
+}
+
+void MediaGalleriesGetMetadataFunction::FinishWithBlobsOnUIThread(
+    base::DictionaryValue* metadata_dictionary,
+    const std::vector<metadata::AttachedImage>& attached_images,
+    const std::vector<std::string>& blob_uuids) {
+  DCHECK(attached_images.size() == blob_uuids.size());
+
+  base::DictionaryValue* result_dictionary = new base::DictionaryValue;
+  result_dictionary->Set(kMetadataKey, metadata_dictionary->DeepCopy());
+
+  // The custom JS binding will reconstitute the blobs in the renderer.
+  base::ListValue* attached_images_list = new base::ListValue;
+  for (size_t i = 0; i < attached_images.size(); ++i) {
+    base::DictionaryValue* attached_image = new base::DictionaryValue;
+    attached_image->Set(kBlobUUIDKey, new base::StringValue(blob_uuids[i]));
+    attached_image->Set(kTypeKey, new base::StringValue(
+        attached_images[i].type));
+    attached_image->Set(kSizeKey, new base::FundamentalValue(
+        base::checked_cast<int>(attached_images[i].data.size())));
+    attached_images_list->Append(attached_image);
+  }
+  result_dictionary->Set(kAttachedImagesKey, attached_images_list);
+
+  SetResult(result_dictionary);
   SendResponse(true);
 }