Media Galleries API Metadata: SafeMediaMetadataParser and IPCTunnelDataReader.

chrome/browser/media_galleries/fileapi/safe_media_metadata_parser.cc

Index: chrome/browser/media_galleries/fileapi/safe_media_metadata_parser.cc
diff --git a/chrome/browser/media_galleries/fileapi/safe_media_metadata_parser.cc b/chrome/browser/media_galleries/fileapi/safe_media_metadata_parser.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e42bb7022e0763cd34db98e96df5743b0b1b3e3d
--- /dev/null
+++ b/chrome/browser/media_galleries/fileapi/safe_media_metadata_parser.cc
@@ -0,0 +1,129 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/media_galleries/fileapi/safe_media_metadata_parser.h"
+
+#include "chrome/browser/extensions/blob_reader.h"
+#include "chrome/common/chrome_utility_messages.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/child_process_data.h"
+#include "content/public/browser/utility_process_host.h"
+
+using content::BrowserThread;
+
+namespace metadata {
+
+SafeMediaMetadataParser::SafeMediaMetadataParser(Profile* profile,
+                                                 const std::string& blob_uuid,
+                                                 int64 blob_size,
+                                                 const std::string& mime_type)
+    : profile_(profile),
+      blob_uuid_(blob_uuid),
+      blob_size_(blob_size),
+      mime_type_(mime_type),
+      parser_state_(INITIAL_STATE) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+}
+
+void SafeMediaMetadataParser::Start(const DoneCallback& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!callback.is_null());
+
+  callback_ = callback;
+
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&SafeMediaMetadataParser::StartWorkOnIOThread, this));
+}
+
+SafeMediaMetadataParser::~SafeMediaMetadataParser() {
+}
+
+void SafeMediaMetadataParser::StartWorkOnIOThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK_EQ(INITIAL_STATE, parser_state_);
+
+  utility_process_host_ = content::UtilityProcessHost::Create(
+      this, base::MessageLoopProxy::current())->AsWeakPtr();
+
+  utility_process_host_->Send(
+      new ChromeUtilityMsg_ParseMediaMetadata(mime_type_, blob_size_));
+
+  parser_state_ = STARTED_PARSING_STATE;
+}
+
+void SafeMediaMetadataParser::OnParseMediaMetadataFinished(
+    bool parse_success,
+    const base::DictionaryValue& metadata_dictionary) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(!callback_.is_null());
+
+  if (parser_state_ != STARTED_PARSING_STATE)
+    return;
+
+  scoped_ptr<MediaMetadata> metadata(
+      MediaMetadata::FromValue(metadata_dictionary));

[vandebo (ex-Chrome), 2014/01/07 18:34:59]

Aren't we going to need to turn this back into Values later?  If so, why not
just keep it in that format?

[tommycli, 2014/01/07 20:23:31]

Done.

+
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(callback_, parse_success, base::Passed(metadata.Pass())));

[vandebo (ex-Chrome), 2014/01/07 18:34:59]

You said callback_ is only access on the UI thread.  Probably it should only be
accessed on the IO thread.

[tommycli, 2014/01/07 20:23:31]

I updated the .h comment to read:
// Assigned on the UI thread, then accessed on the IO thread only.

+  parser_state_ = FINISHED_PARSING_STATE;
+}
+
+void SafeMediaMetadataParser::OnUtilityProcessRequestBlobBytes(
+    int64 request_id,
+    int64 byte_start,
+    int64 length) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  // BlobReader is self-deleting.
+  BlobReader* reader = new BlobReader(
+      profile_,
+      blob_uuid_,
+      base::Bind(&SafeMediaMetadataParser::OnBlobReaderDone,
+                 base::Unretained(this),

[vandebo (ex-Chrome), 2014/01/07 18:34:59]

This concerns me.  What if the SafeMediaMetadataParser owner doesn't want and
cleans up?  What if the utility process crashes?  A weak pointer might be an
option.  You could also avoid trampolining through SafeMediaMetadataParser,
using a local function with bound parameters to call through directly to the
utility process.

[tommycli, 2014/01/07 20:23:31]

Done.

[vandebo (ex-Chrome), 2014/01/08 00:26:18]

I think bound parameters would be preferred...

base::Bind(DoBlobReaderDone, utility_process_host_, request_id)

void DoBlobReaderDone(WeakPtr<> utility_process_host_, request_id,
scoped_ptr<std::string> data) {

if (utility_process_host_.IsValid()) {
  ...
}

}

[tommycli, 2014/01/08 01:17:10]

Done.

+                 request_id));
+  reader->SetByteRange(byte_start, length);
+  reader->Start();
+}
+
+void SafeMediaMetadataParser::OnBlobReaderDone(
+    int64 request_id,
+    scoped_ptr<std::string> data) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  utility_process_host_->Send(
+      new ChromeUtilityMsg_ParseMediaMetadata_RequestBlobBytes_Finished(
+          request_id, *data));
+}
+
+void SafeMediaMetadataParser::OnProcessCrashed(int exit_code) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(!callback_.is_null());
+

[vandebo (ex-Chrome), 2014/01/07 18:34:59]

This should set the state.

[tommycli, 2014/01/07 20:23:31]

Done.

+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(callback_, false,
+                 base::Passed(scoped_ptr<MediaMetadata>().Pass())));
+}
+
+bool SafeMediaMetadataParser::OnMessageReceived(
+    const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(SafeMediaMetadataParser, message)
+    IPC_MESSAGE_HANDLER(
+        ChromeUtilityHostMsg_ParseMediaMetadata_Finished,
+        OnParseMediaMetadataFinished)
+    IPC_MESSAGE_HANDLER(
+        ChromeUtilityHostMsg_ParseMediaMetadata_RequestBlobBytes,
+        OnUtilityProcessRequestBlobBytes)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+}  // namespace metadata