Use a reply struct instead of individual values when responding to intents. Add a "data_path" field to the struct so that files info can be returned to intent clients from browser space.

BUG=129769
TBR=jhawkins@chromium.org for gypi files.

Review URL: https://chromiumcodereview.appspot.com/11090051

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@162901 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 102 insertions, 54 deletions

diff --git a/content/browser/intents/intent_injector.cc b/content/browser/intents/intent_injector.cc
index 28cd1e2..57acb3b 100644
--- a/content/browser/intents/intent_injector.cc
+++ b/content/browser/intents/intent_injector.cc
@@ -120,13 +120,12 @@ bool IntentInjector::OnMessageReceived(const IPC::Message& message) {
   return handled;
 }
 
-void IntentInjector::OnReply(webkit_glue::WebIntentReplyType reply_type,
-                             const string16& data) {
+void IntentInjector::OnReply(const webkit_glue::WebIntentReply& reply) {
   if (!intents_dispatcher_)
     return;
 
   // Ensure SendReplyMessage is only called once.
   WebIntentsDispatcher* intents_dispatcher = intents_dispatcher_;
   intents_dispatcher_ = NULL;
-  intents_dispatcher->SendReplyMessage(reply_type, data);
+  intents_dispatcher->SendReply(reply);
 }
diff --git a/content/browser/intents/intent_injector.h b/content/browser/intents/intent_injector.h
index cd0c5ce..874552a 100644
--- a/content/browser/intents/intent_injector.h
+++ b/content/browser/intents/intent_injector.h
@@ -67,8 +67,7 @@ class CONTENT_EXPORT IntentInjector : public content::WebContentsObserver {
   virtual ~IntentInjector();
 
   // Handles receiving a reply from the intent delivery page.
-  void OnReply(webkit_glue::WebIntentReplyType reply_type,
-               const string16& data);
+  void OnReply(const webkit_glue::WebIntentReply& reply);
 
   // Gets notification that someone else has replied to the intent call.
   void OnSendReturnMessage(webkit_glue::WebIntentReplyType reply_type);
diff --git a/content/browser/intents/intent_injector_unittest.cc b/content/browser/intents/intent_injector_unittest.cc
index ddcecc8..a7784a2 100644
--- a/content/browser/intents/intent_injector_unittest.cc
+++ b/content/browser/intents/intent_injector_unittest.cc
@@ -99,7 +99,8 @@ TEST_F(IntentInjectorTest, AbandonDeletes) {
 
   // Sending a message will both auto-delete |dispatcher_| to clean up and
   // verify that messages don't get sent to the (now deleted) |injector|.
-  dispatcher_->SendReplyMessage(
-      webkit_glue::WEB_INTENT_SERVICE_CONTENTS_CLOSED, string16());
-
+  dispatcher_->SendReply(
+      webkit_glue::WebIntentReply(
+          webkit_glue::WEB_INTENT_SERVICE_CONTENTS_CLOSED,
+          string16()));
 }
diff --git a/content/browser/intents/internal_web_intents_dispatcher.cc b/content/browser/intents/internal_web_intents_dispatcher.cc
index dc731ac..31ad386 100644
--- a/content/browser/intents/internal_web_intents_dispatcher.cc
+++ b/content/browser/intents/internal_web_intents_dispatcher.cc
@@ -46,16 +46,21 @@ void InternalWebIntentsDispatcher::ResetDispatch() {
 void InternalWebIntentsDispatcher::SendReplyMessage(
     webkit_glue::WebIntentReplyType reply_type,
     const string16& data) {
+  SendReply(webkit_glue::WebIntentReply(reply_type, data));
+}
+
+void InternalWebIntentsDispatcher::SendReply(
+    const webkit_glue::WebIntentReply& reply) {
   intent_injector_ = NULL;
 
   for (size_t i = 0; i < reply_notifiers_.size(); ++i) {
     if (!reply_notifiers_[i].is_null())
-      reply_notifiers_[i].Run(reply_type);
+      reply_notifiers_[i].Run(reply.type);
   }
 
   // Notify the callback of the reply.
   if (!reply_callback_.is_null())
-    reply_callback_.Run(reply_type, data);
+    reply_callback_.Run(reply);
 
   delete this;
 }
diff --git a/content/browser/intents/internal_web_intents_dispatcher.h b/content/browser/intents/internal_web_intents_dispatcher.h
index c58ae2e..9e4d62f 100644
--- a/content/browser/intents/internal_web_intents_dispatcher.h
+++ b/content/browser/intents/internal_web_intents_dispatcher.h
@@ -25,8 +25,8 @@ class CONTENT_EXPORT InternalWebIntentsDispatcher
  public:
   // This callback will be called during, and receives the same args as,
   // |SendReplyMessage|.
-  typedef base::Callback<void(webkit_glue::WebIntentReplyType,
-                              const string16&)> ReplyCallback;
+  typedef base::Callback<void(const webkit_glue::WebIntentReply&)>
+      ReplyCallback;
 
   // |intent| is the intent payload to be dispatched.
   explicit InternalWebIntentsDispatcher(
@@ -47,6 +47,7 @@ class CONTENT_EXPORT InternalWebIntentsDispatcher
   virtual void ResetDispatch() OVERRIDE;
   virtual void SendReplyMessage(webkit_glue::WebIntentReplyType reply_type,
                                 const string16& data) OVERRIDE;
+  virtual void SendReply(const webkit_glue::WebIntentReply& reply) OVERRIDE;
   virtual void RegisterReplyNotification(
       const content::WebIntentsDispatcher::ReplyNotification& closure) OVERRIDE;
 
diff --git a/content/browser/intents/internal_web_intents_dispatcher_unittest.cc b/content/browser/intents/internal_web_intents_dispatcher_unittest.cc
index 32f0747..79f9f42 100644
--- a/content/browser/intents/internal_web_intents_dispatcher_unittest.cc
+++ b/content/browser/intents/internal_web_intents_dispatcher_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/bind.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/utf_string_conversions.h"
 #include "content/browser/intents/intent_injector.h"
 #include "content/browser/intents/internal_web_intents_dispatcher.h"
@@ -14,26 +15,21 @@
 class InternalWebIntentsDispatcherTest
     : public content::RenderViewHostTestHarness {
  public:
-  InternalWebIntentsDispatcherTest()
-      : replied_(0),
-        notified_reply_type_(webkit_glue::WEB_INTENT_REPLY_INVALID) {
+  InternalWebIntentsDispatcherTest() : reply_count_(0) {
   }
 
   ~InternalWebIntentsDispatcherTest() {}
 
-  void NotifyReply(webkit_glue::WebIntentReplyType reply_type,
-                   const string16& data) {
-    notified_reply_type_ = reply_type;
-    notified_data_ = data;
+  void NotifyReply(const webkit_glue::WebIntentReply& reply) {
+    reply_.reset(new webkit_glue::WebIntentReply(reply));
   }
 
   void ReplySent(webkit_glue::WebIntentReplyType reply_type) {
-    replied_++;
+    reply_count_++;
   }
 
-  int replied_;
-  string16 notified_data_;
-  webkit_glue::WebIntentReplyType notified_reply_type_;
+  int reply_count_;
+  scoped_ptr<const webkit_glue::WebIntentReply> reply_;
 };
 
 // Tests that the internal dispatcher correctly notifies
@@ -51,12 +47,13 @@ TEST_F(InternalWebIntentsDispatcherTest, NotifiesOnReply) {
 
   dispatcher->DispatchIntent(web_contents());
 
-  dispatcher->SendReplyMessage(webkit_glue::WEB_INTENT_REPLY_SUCCESS,
-                              ASCIIToUTF16("success"));
+  webkit_glue::WebIntentReply reply(
+        webkit_glue::WEB_INTENT_REPLY_SUCCESS, ASCIIToUTF16("success"));
+  dispatcher->SendReply(reply);
 
-  EXPECT_EQ(ASCIIToUTF16("success"), notified_data_);
-  EXPECT_EQ(1, replied_);
-  EXPECT_EQ(webkit_glue::WEB_INTENT_REPLY_SUCCESS, notified_reply_type_);
+  ASSERT_TRUE(1 == reply_count_);
+  ASSERT_TRUE(reply_);
+  EXPECT_EQ(*reply_.get(), reply);
 }
 
 TEST_F(InternalWebIntentsDispatcherTest, CancelAbandonsInjector) {
diff --git a/content/browser/intents/web_intents_dispatcher_impl.cc b/content/browser/intents/web_intents_dispatcher_impl.cc
index df5ec99..abddf4f8 100644
--- a/content/browser/intents/web_intents_dispatcher_impl.cc
+++ b/content/browser/intents/web_intents_dispatcher_impl.cc
@@ -55,16 +55,24 @@ void WebIntentsDispatcherImpl::ResetDispatch() {
 void WebIntentsDispatcherImpl::SendReplyMessage(
     webkit_glue::WebIntentReplyType reply_type,
     const string16& data) {
+  SendReply(webkit_glue::WebIntentReply(reply_type, data));
+}
+
+void WebIntentsDispatcherImpl::SendReply(
+    const webkit_glue::WebIntentReply& reply) {
   intent_injector_ = NULL;
 
+  // If this intent is attached to a WebContents, we dispatch the request back
+  // to the renderer. Browser process initiated intents are not
+  // associated with a WebContents.
   if (web_contents()) {
     Send(new IntentsMsg_WebIntentReply(
-        routing_id(), reply_type, data, intent_id_));
+        routing_id(), reply, intent_id_));
   }
 
   for (size_t i = 0; i < reply_notifiers_.size(); ++i) {
     if (!reply_notifiers_[i].is_null())
-      reply_notifiers_[i].Run(reply_type);
+      reply_notifiers_[i].Run(reply.type);
   }
 
   delete this;
diff --git a/content/browser/intents/web_intents_dispatcher_impl.h b/content/browser/intents/web_intents_dispatcher_impl.h
index 011f4ae..40b497a 100644
--- a/content/browser/intents/web_intents_dispatcher_impl.h
+++ b/content/browser/intents/web_intents_dispatcher_impl.h
@@ -11,6 +11,7 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_intents_dispatcher.h"
 #include "webkit/glue/web_intent_data.h"
+#include "webkit/glue/web_intent_reply_data.h"
 
 class IntentInjector;
 
@@ -36,8 +37,11 @@ class WebIntentsDispatcherImpl : public content::WebIntentsDispatcher,
   virtual void DispatchIntent(
       content::WebContents* destination_contents) OVERRIDE;
   virtual void ResetDispatch() OVERRIDE;
+  // Deprecated. Use SendReply.
+  // TODO(smckay): Eliminate use of SendReplyMessage in followup CL.
   virtual void SendReplyMessage(webkit_glue::WebIntentReplyType reply_type,
                                 const string16& data) OVERRIDE;
+  virtual void SendReply(const webkit_glue::WebIntentReply& reply) OVERRIDE;
   virtual void RegisterReplyNotification(
       const content::WebIntentsDispatcher::ReplyNotification& closure) OVERRIDE;
 
diff --git a/content/common/intents_messages.h b/content/common/intents_messages.h
index df7b451..425d3a8 100644
--- a/content/common/intents_messages.h
+++ b/content/common/intents_messages.h
@@ -36,6 +36,13 @@ IPC_STRUCT_TRAITS_BEGIN(webkit_glue::WebIntentData)
   IPC_STRUCT_TRAITS_MEMBER(data_type)
 IPC_STRUCT_TRAITS_END()
 
+IPC_STRUCT_TRAITS_BEGIN(webkit_glue::WebIntentReply)
+  IPC_STRUCT_TRAITS_MEMBER(type)
+  IPC_STRUCT_TRAITS_MEMBER(data)
+  IPC_STRUCT_TRAITS_MEMBER(data_file)
+  IPC_STRUCT_TRAITS_MEMBER(data_file_size)
+  IPC_STRUCT_TRAITS_END()
+
 IPC_STRUCT_TRAITS_BEGIN(webkit_glue::WebIntentServiceData)
   IPC_STRUCT_TRAITS_MEMBER(action)
   IPC_STRUCT_TRAITS_MEMBER(type)
@@ -50,15 +57,13 @@ IPC_MESSAGE_ROUTED1(IntentsMsg_SetWebIntentData,
                     webkit_glue::WebIntentData)
 
 // Send the service's reply to the client page.
-IPC_MESSAGE_ROUTED3(IntentsMsg_WebIntentReply,
-                    webkit_glue::WebIntentReplyType /* reply type */,
-                    string16 /* payload data */,
+IPC_MESSAGE_ROUTED2(IntentsMsg_WebIntentReply,
+                    webkit_glue::WebIntentReply, /* reply */
                     int /* intent ID */)
 
 // Notify the container that the service has replied to the client page.
-IPC_MESSAGE_ROUTED2(IntentsHostMsg_WebIntentReply,
-                    webkit_glue::WebIntentReplyType /* reply type */,
-                    string16 /* payload data */)
+IPC_MESSAGE_ROUTED1(IntentsHostMsg_WebIntentReply,
+                    webkit_glue::WebIntentReply /* reply */)
 
 // Route the startActivity Intents call from a page to the service picker.
 IPC_MESSAGE_ROUTED2(IntentsHostMsg_WebIntentDispatch,
diff --git a/content/public/browser/web_intents_dispatcher.h b/content/public/browser/web_intents_dispatcher.h
index b24ff4b..1288753 100644
--- a/content/public/browser/web_intents_dispatcher.h
+++ b/content/public/browser/web_intents_dispatcher.h
@@ -12,6 +12,7 @@
 namespace webkit_glue {
 enum WebIntentReplyType;
 struct WebIntentData;
+struct WebIntentReply;
 }
 
 namespace content {
@@ -57,12 +58,15 @@ class CONTENT_EXPORT WebIntentsDispatcher {
   // Abandon current attempt to dispatch, allow new call to DispatchIntent.
   virtual void ResetDispatch() = 0;
 
-  // Return a success or failure message to the source context which invoked
-  // the intent. Deletes the object; it should not be used after this call
-  // returns. Calls the reply notifications, if any are registered.
+  // Deprecated. Use SendReply.
   virtual void SendReplyMessage(webkit_glue::WebIntentReplyType reply_type,
                                 const string16& data) = 0;
 
+  // Return a reply to the source context which invoked the intent.
+  // Calls the reply notifications, if any are registered.
+  // Deletes |this| object after handling is complete.
+  virtual void SendReply(const webkit_glue::WebIntentReply& reply) = 0;
+
   // Register a callback to be notified when SendReplyMessage is called.
   // Multiple callbacks may be registered.
   virtual void RegisterReplyNotification(const ReplyNotification& closure) = 0;
diff --git a/content/renderer/web_intents_host.cc b/content/renderer/web_intents_host.cc
index 933f41a..e8876ce 100644
--- a/content/renderer/web_intents_host.cc
+++ b/content/renderer/web_intents_host.cc
@@ -38,6 +38,32 @@ using WebKit::WebString;
 using WebKit::WebSerializedScriptValue;
 using WebKit::WebVector;
 
+namespace {
+
+// Reads reply value, either from the data field, or the data_file field.
+WebSerializedScriptValue GetReplyValue(
+    const webkit_glue::WebIntentReply& reply) {
+  if (reply.data_file_size > -1) {
+    // TODO(smckay): seralize the blob value in the web intent script
+    // context. We simply don't know how to do this at this time.
+    // The following code kills the renderer when we call toV8Value.
+    //    v8::HandleScope scope;
+    //    v8::Local<v8::Context> ctx = ....which context?
+    //    v8::Context::Scope cscope(ctx);
+    //    WebKit::WebBlob blob = WebBlob::createFromFile(
+    //        WebString::fromUTF8(reply.data_file.AsUTF8Unsafe()),
+    //        reply.data_file_size);
+    //    v8::Handle<v8::Value> value = blob.toV8Value();
+    //    return WebSerializedScriptValue::serialize(value);
+    return WebSerializedScriptValue::fromString(
+        ASCIIToUTF16(reply.data_file.AsUTF8Unsafe()));
+  } else {
+    return WebSerializedScriptValue::fromString(reply.data);
+  }
+}
+
+} // namespace
+
 class DeliveredIntentClientImpl : public WebDeliveredIntentClient {
  public:
   explicit DeliveredIntentClientImpl(WebIntentsHost* host) : host_(host) {}
@@ -88,8 +114,7 @@ void WebIntentsHost::OnSetIntent(const webkit_glue::WebIntentData& intent) {
 }
 
 void WebIntentsHost::OnWebIntentReply(
-    webkit_glue::WebIntentReplyType reply_type,
-    const WebKit::WebString& data,
+    const webkit_glue::WebIntentReply& reply,
     int intent_id) {
   std::map<int, WebIntentRequest>::iterator request =
       intent_requests_.find(intent_id);
@@ -97,24 +122,25 @@ void WebIntentsHost::OnWebIntentReply(
     return;
   WebIntentRequest intent_request = request->second;
   intent_requests_.erase(request);
-  WebSerializedScriptValue value =
-      WebSerializedScriptValue::fromString(data);
 
-  if (reply_type == webkit_glue::WEB_INTENT_REPLY_SUCCESS) {
-    intent_request.postResult(value);
+  if (reply.type == webkit_glue::WEB_INTENT_REPLY_SUCCESS) {
+    intent_request.postResult(GetReplyValue(reply));
   } else {
-    intent_request.postFailure(value);
+    intent_request.postFailure(
+        WebSerializedScriptValue::fromString(reply.data));
   }
 }
 
 void WebIntentsHost::OnResult(const WebKit::WebString& data) {
-  Send(new IntentsHostMsg_WebIntentReply(
-      routing_id(), webkit_glue::WEB_INTENT_REPLY_SUCCESS, data));
+  const webkit_glue::WebIntentReply reply(
+      webkit_glue::WEB_INTENT_REPLY_SUCCESS, data);
+  Send(new IntentsHostMsg_WebIntentReply(routing_id(), reply));
 }
 
 void WebIntentsHost::OnFailure(const WebKit::WebString& data) {
-  Send(new IntentsHostMsg_WebIntentReply(
-      routing_id(), webkit_glue::WEB_INTENT_REPLY_FAILURE, data));
+  const webkit_glue::WebIntentReply reply(
+      webkit_glue::WEB_INTENT_REPLY_FAILURE, data);
+  Send(new IntentsHostMsg_WebIntentReply(routing_id(), reply));
 }
 
 // We set the intent payload into all top-level frame window objects. This
diff --git a/content/renderer/web_intents_host.h b/content/renderer/web_intents_host.h
index 3556d76..c3d4022 100644
--- a/content/renderer/web_intents_host.h
+++ b/content/renderer/web_intents_host.h
@@ -68,10 +68,9 @@ class WebIntentsHost : public content::RenderViewObserver {
   CONTENT_EXPORT void OnSetIntent(const webkit_glue::WebIntentData& intent);
 
   // On the client page, handler method for the IntentsMsg_WebIntentReply
-  // message. Forwards the reply |data| to the registered WebIntentRequest
+  // message. Forwards |reply| to the registered WebIntentRequest
   // (found by |intent_id|), where it is dispatched to the client JS callback.
-  void OnWebIntentReply(webkit_glue::WebIntentReplyType reply_type,
-                        const WebKit::WebString& data,
+  void OnWebIntentReply(const webkit_glue::WebIntentReply& reply,
                         int intent_id);
 
   friend class WebIntentsHostTest;
diff --git a/content/renderer/web_intents_host_browsertest.cc b/content/renderer/web_intents_host_browsertest.cc
index 9f2abc7..36a85f6 100644
--- a/content/renderer/web_intents_host_browsertest.cc
+++ b/content/renderer/web_intents_host_browsertest.cc
@@ -27,7 +27,7 @@ class WebIntentsHostTest : public content::RenderViewTest {
     IntentsHostMsg_WebIntentReply::Param reply_params;
     IntentsHostMsg_WebIntentReply::Read(reply_msg, &reply_params);
     WebKit::WebSerializedScriptValue serialized_data =
-        WebKit::WebSerializedScriptValue::fromString(reply_params.b);
+        WebKit::WebSerializedScriptValue::fromString(reply_params.a.data);
 
     v8::HandleScope scope;
     v8::Local<v8::Context> ctx = GetMainFrame()->mainWorldScriptContext();