Add API to construct new vector interchange MIME data type.

content/renderer/web_intents_host.cc

Index: content/renderer/web_intents_host.cc
diff --git a/content/renderer/web_intents_host.cc b/content/renderer/web_intents_host.cc
index 9c60da3529fcf5f99ced41035fcc56b57fa0142a..b130dcc1a74af869e32aabdf58898b67c5ca4b76 100644
--- a/content/renderer/web_intents_host.cc
+++ b/content/renderer/web_intents_host.cc
@@ -8,6 +8,7 @@
 #include "base/bind_helpers.h"
 #include "base/utf_string_conversions.h"
 #include "content/common/intents_messages.h"
+#include "content/public/renderer/v8_value_converter.h"
 #include "content/renderer/render_view_impl.h"
 #include "ipc/ipc_message.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebBindings.h"
@@ -23,7 +24,6 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebSerializedScriptValue.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebVector.h"
-#include "v8/include/v8.h"
 #include "webkit/fileapi/file_system_util.h"
 #include "webkit/glue/cpp_bound_class.h"
 
@@ -120,9 +120,10 @@ void WebIntentsHost::OnFailure(const WebKit::WebString& data) {
 // We set the intent payload into all top-level frame window objects. This
 // should persist the data through redirects, and not deliver it to any
 // sub-frames.
-// TODO(gbillock): match to spec to double-check registration match before
-// delivery.
-void WebIntentsHost::DidClearWindowObject(WebFrame* frame) {
+void WebIntentsHost::DidCreateScriptContext(WebKit::WebFrame* frame,
+                                            v8::Handle<v8::Context> ctx,
+                                            int extension_group,
+                                            int world_id) {
   if (intent_.get() == NULL || frame->top() != frame)
     return;
 
@@ -130,71 +131,103 @@ void WebIntentsHost::DidClearWindowObject(WebFrame* frame) {
     delivered_intent_client_.reset(new DeliveredIntentClientImpl(this));
   }
 
-  WebVector<WebString> extras_keys(intent_->extra_data.size());
-  WebVector<WebString> extras_values(intent_->extra_data.size());
-  std::map<string16, string16>::iterator iter;
-  int i;
-  for (i = 0, iter = intent_->extra_data.begin();
-       iter != intent_->extra_data.end();
+  v8::HandleScope scope;
+  v8::Context::Scope cscope(ctx);

[abarth-chromium, 2012/10/12 19:59:09]

Do we need either of these things any more?  We should already have a
HandleScope on the stack and we should also already be in the proper context.

[Greg Billock, 2012/10/12 21:32:39]

Yes. Just checked and this gets called within these scopes. Good deal!

+  WebIntent web_intent = CreateWebIntent(frame, *(intent_.get()));
+
+  if (!web_intent.action().isEmpty())
+    frame->deliverIntent(web_intent, NULL, delivered_intent_client_.get());
+}
+
+WebIntent WebIntentsHost::CreateWebIntent(
+    WebFrame* frame, const webkit_glue::WebIntentData& intent_data) {
+  // Must be called with v8 scope held.
+  DCHECK(!v8::Context::GetCurrent().IsEmpty());

[abarth-chromium, 2012/10/12 19:59:09]

Why not call v8::Context::InContext() ?  That's the canonical way to check
whether we're in a context.

[Greg Billock, 2012/10/12 21:32:39]

Done.

+
+  // TODO(gbillock): Remove this block when we get rid of |extras|.
+  WebVector<WebString> extras_keys(intent_data.extra_data.size());
+  WebVector<WebString> extras_values(intent_data.extra_data.size());
+  std::map<string16, string16>::const_iterator iter;
+  size_t i;
+  for (i = 0, iter = intent_data.extra_data.begin();
+       iter != intent_data.extra_data.end();
        ++i, ++iter) {
     extras_keys[i] = iter->first;
     extras_values[i] = iter->second;
   }
 
-  v8::HandleScope scope;
-  v8::Local<v8::Context> ctx = frame->mainWorldScriptContext();
-  v8::Context::Scope cscope(ctx);
-  WebIntent web_intent;
-
-  if (intent_->data_type == webkit_glue::WebIntentData::SERIALIZED) {
-    web_intent = WebIntent::create(intent_->action, intent_->type,
-                                   intent_->data,
-                                   extras_keys, extras_values);
-  } else if (intent_->data_type == webkit_glue::WebIntentData::UNSERIALIZED) {
+  if (intent_data.data_type == webkit_glue::WebIntentData::SERIALIZED) {
+    return WebIntent::create(intent_data.action, intent_data.type,
+                             intent_data.data,
+                             extras_keys, extras_values);
+  } else if (intent_data.data_type ==
+             webkit_glue::WebIntentData::UNSERIALIZED) {
     v8::Local<v8::String> dataV8 = v8::String::New(
-        reinterpret_cast<const uint16_t*>(intent_->unserialized_data.data()),
-        static_cast<int>(intent_->unserialized_data.length()));
+        reinterpret_cast<const uint16_t*>(intent_data.unserialized_data.data()),

[abarth-chromium, 2012/10/12 19:59:09]

Is this safe?  Do we need to worry about unpaired surrogates or other Unicode
gotchas?

Generally, we prefer to use array buffers rather than strings for binary data.

[Greg Billock, 2012/10/12 21:32:39]

This is for text data prepared safely in the browser process. For example, with
url.spec(). Binary data is prepared in other ways (there's support for blob, but
not array buffers currently).

+        static_cast<int>(intent_data.unserialized_data.length()));
     WebSerializedScriptValue serialized_data =
         WebSerializedScriptValue::serialize(dataV8);
 
-    web_intent = WebIntent::create(intent_->action, intent_->type,
-                                   serialized_data.toString(),
-                                   extras_keys, extras_values);
-  } else if (intent_->data_type == webkit_glue::WebIntentData::BLOB) {
-    DCHECK(intent_->data_type == webkit_glue::WebIntentData::BLOB);
+    return WebIntent::create(intent_data.action, intent_data.type,
+                             serialized_data.toString(),
+                             extras_keys, extras_values);
+  } else if (intent_data.data_type == webkit_glue::WebIntentData::BLOB) {
     web_blob_ = WebBlob::createFromFile(
-        WebString::fromUTF8(intent_->blob_file.AsUTF8Unsafe()),
-        intent_->blob_length);
+        WebString::fromUTF8(intent_data.blob_file.AsUTF8Unsafe()),

[abarth-chromium, 2012/10/12 19:59:09]

What if the blob_file cannot be represented as UTF8?

[Greg Billock, 2012/10/12 21:32:39]

This refers to the name not having encoding information included in some POSIX
systems. If I read the code correctly, this'll end up in an empty blob.

+        intent_data.blob_length);
     WebSerializedScriptValue serialized_data =
         WebSerializedScriptValue::serialize(web_blob_.toV8Value());
-    web_intent = WebIntent::create(intent_->action, intent_->type,
-                                   serialized_data.toString(),
-                                   extras_keys, extras_values);
-  } else if (intent_->data_type == webkit_glue::WebIntentData::FILESYSTEM) {
+    return WebIntent::create(intent_data.action, intent_data.type,
+                             serialized_data.toString(),
+                             extras_keys, extras_values);
+  } else if (intent_data.data_type == webkit_glue::WebIntentData::FILESYSTEM) {
     const GURL origin = GURL(frame->document().securityOrigin().toString());
     const GURL root_url =
         fileapi::GetFileSystemRootURI(origin, fileapi::kFileSystemTypeIsolated);
     const std::string url = base::StringPrintf(
         "%s%s/%s/",
         root_url.spec().c_str(),
-        intent_->filesystem_id.c_str(),
-        intent_->root_name.c_str());
+        intent_data.filesystem_id.c_str(),
+        intent_data.root_name.c_str());
     // TODO(kmadhusu): This is a temporary hack to create a serializable file
     // system. Once we have a better way to create a serializable file system,
     // remove this hack.
     v8::Handle<v8::Value> filesystem_V8 = frame->createSerializableFileSystem(
         WebKit::WebFileSystem::TypeIsolated,
-        WebKit::WebString::fromUTF8(intent_->root_name),
+        WebKit::WebString::fromUTF8(intent_data.root_name),
         WebKit::WebString::fromUTF8(url));
     WebSerializedScriptValue serialized_data =
         WebSerializedScriptValue::serialize(filesystem_V8);
-    web_intent = WebIntent::create(intent_->action, intent_->type,
-                                   serialized_data.toString(),
-                                   extras_keys, extras_values);
-  } else {
-    NOTREACHED();
+    return WebIntent::create(intent_data.action, intent_data.type,
+                             serialized_data.toString(),
+                             extras_keys, extras_values);
+  } else if (intent_data.data_type == webkit_glue::WebIntentData::MIME_TYPE) {
+    scoped_ptr<content::V8ValueConverter> converter(
+        content::V8ValueConverter::create());
+    v8::Handle<v8::Value> valV8 = converter->ToV8Value(
+        &intent_data.mime_data, v8::Context::GetCurrent());
+
+    // If we are carrying blob data, convert it to v8 format and set it to
+    // the "blob" property of the first element in the array.
+    if (!intent_data.blob_file.empty() && valV8->IsArray()) {
+      v8::Local<v8::Array> valArray = v8::Array::Cast(*valV8);
+      v8::Local<v8::Value> firstVal = valArray->Get(0);
+      if (firstVal->IsObject()) {
+        web_blob_ = WebBlob::createFromFile(
+            WebString::fromUTF8(intent_data.blob_file.AsUTF8Unsafe()),

[abarth-chromium, 2012/10/12 19:59:09]

Should we be concerned that we're calling a function with "unsafe" in the name?

[Greg Billock, 2012/10/12 21:32:39]

See above. I think this is the best way to translate a filename from FilePath
down to webkit.

+            intent_data.blob_length);
+        firstVal->ToObject()->Set(v8::String::New("blob", 4),
+                                 web_blob_.toV8Value());

[abarth-chromium, 2012/10/12 19:59:09]

It's not clear to me why it is safe to call Set here.  Also, why are we creating
a string each time instead of using a single v8::String that we hold in a local
handle.

[Greg Billock, 2012/10/12 21:32:39]

Because of the __setter possibility? I looked through the v8 code, and I do
think it has at least the possibility of going through that path.

But see V8ValueConverterImpl, for instance. It uses v8::Object::New, and then
result->Set on the resulting handle. Do those get run asynchronously, so it's
not a problem, or are (some? all?) uses of this serializer suspect?
You think that optimization is needed? I thought this was easier to read.

+      }
+    }
+
+    WebSerializedScriptValue serialized_data =
+        WebSerializedScriptValue::serialize(valV8);
+    return WebIntent::create(intent_data.action, intent_data.type,
+                             serialized_data.toString(),
+                             WebVector<WebString>(), WebVector<WebString>());
   }
 
-  if (!web_intent.action().isEmpty())
-    frame->deliverIntent(web_intent, NULL, delivered_intent_client_.get());
+  NOTREACHED();
+  return WebIntent();
 }