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..e4359574df11f15ad90ee69703880c05e3a433ad 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,81 +120,101 @@ 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;
 
+  if (ctx != frame->mainWorldScriptContext())
+    return;
+
   if (!delivered_intent_client_.get()) {
     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();
+  WebIntent web_intent = CreateWebIntent(frame, *(intent_.get()));

[jamesr, 2012/10/15 01:58:29]

scoped_ptr<> has an operator* so you can say just "*intent_" instead of
"*(intent_.get())"

[Greg Billock, 2012/10/15 19:23:33]

Done.

+
+  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::InContext());
+
+  // 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();

[jamesr, 2012/10/15 01:58:29]

nit: i would scope the loop counter and iterator inside the for loop since they
don't appear to be used for the rest of the (long) function

[Greg Billock, 2012/10/15 19:23:33]

The compiler gets confused that the two initializations aren't the same type.
Moved the size_t one inline.

+       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) {

[jamesr, 2012/10/15 01:58:29]

This big else-if ladder chain should be a switch statement. Then the compiler
could tell you if you were missing a case instead of only having the runtime
NOT_REACHED() at the bottom of the function

[Greg Billock, 2012/10/15 19:23:33]

Done.

+    return WebIntent::create(intent_data.action, intent_data.type,
+                             intent_data.data,

[jamesr, 2012/10/15 01:58:29]

this doesn't appear to be aligned correctly

[Greg Billock, 2012/10/15 19:23:33]

Weird. It shows up right in my editor. Deletion doesn't show any niggly tabs.
Deleted and replaced all spaces anyway, we'll see if that helps...

+                             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()),

[jamesr, 2012/10/15 01:58:29]

I'm a bit surprised by this reinterpret_cast<> - unserialized_data is a string16
aka basic_string<char16...> and char16 is uint16_t everywhere, so why does this
need the reinterpret?

[Greg Billock, 2012/10/15 19:23:33]

Agreed this is unexpected. The windows compiler gets confused by this, and can't
decide which v8::String::New to use.

+        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()),
+        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());
+
+    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();
 }