Go back to JSON serialization of extension messages. base::Value pickling was

causing OOMs on the browser, and V8ValueConverter has bugs. This is effectively
a revert of both r204067 and r204496, except the json library replaced by the new
safe $JSON, and it's eagerly included via our small-footprint v8::Extension.

BUG=247530,248019,249419
R=mpcomplete@chromium.org
TBR=cdn@chromium.org

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

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

11 files changed, 53 insertions, 112 deletions

diff --git a/chrome/browser/extensions/api/messaging/extension_message_port.cc b/chrome/browser/extensions/api/messaging/extension_message_port.cc
index a3bba97..e4782c7 100644
--- a/chrome/browser/extensions/api/messaging/extension_message_port.cc
+++ b/chrome/browser/extensions/api/messaging/extension_message_port.cc
@@ -4,7 +4,6 @@
 
 #include "chrome/browser/extensions/api/messaging/extension_message_port.h"
 
-#include "base/values.h"
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_system.h"
@@ -46,11 +45,10 @@ void ExtensionMessagePort::DispatchOnDisconnect(
       routing_id_, source_port_id, error_message));
 }
 
-void ExtensionMessagePort::DispatchOnMessage(
-    scoped_ptr<base::ListValue> message,
-    int target_port_id) {
-  process_->Send(new ExtensionMsg_DeliverMessage(
-      routing_id_, target_port_id, *message));
+void ExtensionMessagePort::DispatchOnMessage(const std::string& message,
+                                             int target_port_id) {
+    process_->Send(new ExtensionMsg_DeliverMessage(
+        routing_id_, target_port_id, message));
 }
 
 void ExtensionMessagePort::IncrementLazyKeepaliveCount() {
diff --git a/chrome/browser/extensions/api/messaging/extension_message_port.h b/chrome/browser/extensions/api/messaging/extension_message_port.h
index 7cc386a..ccf9b69 100644
--- a/chrome/browser/extensions/api/messaging/extension_message_port.h
+++ b/chrome/browser/extensions/api/messaging/extension_message_port.h
@@ -9,10 +9,6 @@
 
 class GURL;
 
-namespace base {
-class ListValue;
-}
-
 namespace content {
 class RenderProcessHost;
 }  // namespace content
@@ -34,7 +30,7 @@ class ExtensionMessagePort : public MessageService::MessagePort {
       const GURL& source_url) OVERRIDE;
   virtual void DispatchOnDisconnect(int source_port_id,
                                     const std::string& error_message) OVERRIDE;
-  virtual void DispatchOnMessage(scoped_ptr<base::ListValue> message,
+  virtual void DispatchOnMessage(const std::string& message,
                                  int target_port_id) OVERRIDE;
   virtual void IncrementLazyKeepaliveCount() OVERRIDE;
   virtual void DecrementLazyKeepaliveCount() OVERRIDE;
diff --git a/chrome/browser/extensions/api/messaging/message_service.cc b/chrome/browser/extensions/api/messaging/message_service.cc
index 3188a7f..3d05766 100644
--- a/chrome/browser/extensions/api/messaging/message_service.cc
+++ b/chrome/browser/extensions/api/messaging/message_service.cc
@@ -6,7 +6,6 @@
 
 #include "base/atomic_sequence_num.h"
 #include "base/bind.h"
-#include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
@@ -458,7 +457,7 @@ void MessageService::CloseChannelImpl(
 }
 
 void MessageService::PostMessage(
-    int source_port_id, scoped_ptr<base::ListValue> message) {
+    int source_port_id, const std::string& message) {
   int channel_id = GET_CHANNEL_ID(source_port_id);
   MessageChannelMap::iterator iter = channels_.find(channel_id);
   if (iter == channels_.end()) {
@@ -469,9 +468,7 @@ void MessageService::PostMessage(
       lazy_background_task_queue_->AddPendingTask(
           pending->second.first, pending->second.second,
           base::Bind(&MessageService::PendingPostMessage,
-                     weak_factory_.GetWeakPtr(),
-                     source_port_id,
-                     base::Passed(&message)));
+                     weak_factory_.GetWeakPtr(), source_port_id, message));
     }
     return;
   }
@@ -481,13 +478,12 @@ void MessageService::PostMessage(
   MessagePort* port = IS_OPENER_PORT_ID(dest_port_id) ?
       iter->second->opener.get() : iter->second->receiver.get();
 
-  port->DispatchOnMessage(message.Pass(), dest_port_id);
+  port->DispatchOnMessage(message, dest_port_id);
 }
 
-void MessageService::PostMessageFromNativeProcess(
-    int port_id,
-    scoped_ptr<base::ListValue> message) {
-  PostMessage(port_id, message.Pass());
+void MessageService::PostMessageFromNativeProcess(int port_id,
+                                                  const std::string& message) {
+  PostMessage(port_id, message);
 }
 
 void MessageService::Observe(int type,
diff --git a/chrome/browser/extensions/api/messaging/message_service.h b/chrome/browser/extensions/api/messaging/message_service.h
index 59ba6ff..6de5a74 100644
--- a/chrome/browser/extensions/api/messaging/message_service.h
+++ b/chrome/browser/extensions/api/messaging/message_service.h
@@ -22,7 +22,6 @@ class Profile;
 
 namespace base {
 class DictionaryValue;
-class ListValue;
 }
 
 namespace content {
@@ -83,7 +82,7 @@ class MessageService : public ProfileKeyedAPI,
                                       const std::string& error_message) {}
 
     // Dispatch a message to this end of the communication.
-    virtual void DispatchOnMessage(scoped_ptr<base::ListValue> message,
+    virtual void DispatchOnMessage(const std::string& message,
                                    int target_port_id) = 0;
 
     // MessagPorts that target extensions will need to adjust their keepalive
@@ -145,12 +144,12 @@ class MessageService : public ProfileKeyedAPI,
                             const std::string& error_message) OVERRIDE;
 
   // Sends a message to the given port.
-  void PostMessage(int port_id, scoped_ptr<base::ListValue> message);
+  void PostMessage(int port_id, const std::string& message);
 
   // NativeMessageProcessHost::Client
   virtual void PostMessageFromNativeProcess(
       int port_id,
-      scoped_ptr<base::ListValue> message) OVERRIDE;
+      const std::string& message) OVERRIDE;
 
  private:
   friend class MockMessageService;
@@ -206,10 +205,10 @@ class MessageService : public ProfileKeyedAPI,
       CloseChannel(port_id, error_message);
   }
   void PendingPostMessage(int port_id,
-                          scoped_ptr<base::ListValue> message,
+                          const std::string& message,
                           extensions::ExtensionHost* host) {
     if (host)
-      PostMessage(port_id, message.Pass());
+      PostMessage(port_id, message);
   }
 
   // Immediate dispatches a disconnect to |source| for |port_id|. Sets source's
diff --git a/chrome/browser/extensions/api/messaging/native_message_port.cc b/chrome/browser/extensions/api/messaging/native_message_port.cc
index 7ca7bea..e746e29 100644
--- a/chrome/browser/extensions/api/messaging/native_message_port.cc
+++ b/chrome/browser/extensions/api/messaging/native_message_port.cc
@@ -5,8 +5,6 @@
 #include "chrome/browser/extensions/api/messaging/native_message_port.h"
 
 #include "base/bind.h"
-#include "base/json/json_writer.h"
-#include "base/values.h"
 #include "chrome/browser/extensions/api/messaging/native_message_process_host.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -21,20 +19,12 @@ NativeMessagePort::~NativeMessagePort() {
       content::BrowserThread::IO, FROM_HERE, native_process_);
 }
 
-void NativeMessagePort::DispatchOnMessage(scoped_ptr<base::ListValue> message,
+void NativeMessagePort::DispatchOnMessage(const std::string& message,
                                           int target_port_id) {
-  std::string message_as_json;
-  if (!message->empty()) {
-    DCHECK_EQ(1u, message->GetSize());
-    base::Value* value = NULL;
-    message->Get(0, &value);
-    base::JSONWriter::Write(value, &message_as_json);
-  }
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&NativeMessageProcessHost::Send,
-                 base::Unretained(native_process_),
-                 message_as_json));
+                 base::Unretained(native_process_), message));
 }
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/api/messaging/native_message_port.h b/chrome/browser/extensions/api/messaging/native_message_port.h
index 4c88445..6afce8a 100644
--- a/chrome/browser/extensions/api/messaging/native_message_port.h
+++ b/chrome/browser/extensions/api/messaging/native_message_port.h
@@ -16,7 +16,7 @@ class NativeMessagePort : public MessageService::MessagePort {
   // Takes ownership of |native_process|.
   explicit NativeMessagePort(NativeMessageProcessHost* native_process);
   virtual ~NativeMessagePort();
-  virtual void DispatchOnMessage(scoped_ptr<base::ListValue> message,
+  virtual void DispatchOnMessage(const std::string& message,
                                  int target_port_id) OVERRIDE;
 
  private:
diff --git a/chrome/browser/extensions/api/messaging/native_message_process_host.cc b/chrome/browser/extensions/api/messaging/native_message_process_host.cc
index 9f8c50b..9187a08 100644
--- a/chrome/browser/extensions/api/messaging/native_message_process_host.cc
+++ b/chrome/browser/extensions/api/messaging/native_message_process_host.cc
@@ -5,9 +5,7 @@
 #include "chrome/browser/extensions/api/messaging/native_message_process_host.h"
 
 #include "base/bind.h"
-#include "base/bind_helpers.h"
 #include "base/files/file_path.h"
-#include "base/json/json_reader.h"
 #include "base/logging.h"
 #include "base/platform_file.h"
 #include "base/process_util.h"
@@ -44,8 +42,6 @@ const char kForbiddenError[] =
     "Access to the specified native messaging host is forbidden.";
 const char kHostInputOuputError[] =
     "Error when communicating with the native messaging host.";
-const char kInvalidJsonError[] =
-    "Message must be valid JSON";
 
 }  // namespace
 
@@ -272,35 +268,10 @@ void NativeMessageProcessHost::ProcessIncomingData(
     if (incoming_data_.size() < message_size + kMessageHeaderSize)
       return;
 
-    scoped_ptr<base::ListValue> message(new base::ListValue());
-    {
-      std::string message_as_json =
-          incoming_data_.substr(kMessageHeaderSize, message_size);
-      int error_code;
-      std::string error_message;
-      scoped_ptr<base::Value> message_as_value(
-          base::JSONReader::ReadAndReturnError(message_as_json,
-                                               0,  // no flags
-                                               &error_code,
-                                               &error_message));
-      if (!message_as_value) {
-        base::JSONReader::JsonParseError parse_error =
-            static_cast<base::JSONReader::JsonParseError>(error_code);
-        LOG(ERROR) << "Native Messaging host sent message with invalid JSON \""
-                   << message_as_json << "\": " << error_message << " ("
-                   << base::JSONReader::ErrorCodeToString(parse_error) << "), "
-                   << "message size " << message_size << ", "
-                   << "incoming data size " << incoming_data_.size() << ".";
-        Close(kInvalidJsonError);
-        return;
-      }
-      message->Append(message_as_value.release());
-    }
-
     content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
         base::Bind(&Client::PostMessageFromNativeProcess, weak_client_ui_,
             destination_port_,
-            base::Passed(&message)));
+            incoming_data_.substr(kMessageHeaderSize, message_size)));
 
     incoming_data_.erase(0, kMessageHeaderSize + message_size);
   }
diff --git a/chrome/browser/extensions/api/messaging/native_message_process_host.h b/chrome/browser/extensions/api/messaging/native_message_process_host.h
index ef339b2..987055f 100644
--- a/chrome/browser/extensions/api/messaging/native_message_process_host.h
+++ b/chrome/browser/extensions/api/messaging/native_message_process_host.h
@@ -15,15 +15,13 @@
 #include "chrome/browser/extensions/api/messaging/native_process_launcher.h"
 #include "content/public/browser/browser_thread.h"
 
-namespace base {
-class ListValue;
-}
-
 namespace net {
+
 class DrainableIOBuffer;
 class FileStream;
 class IOBuffer;
 class IOBufferWithSize;
+
 }  // namespace net
 
 namespace extensions {
@@ -45,9 +43,8 @@ class NativeMessageProcessHost
    public:
     virtual ~Client() {}
     // Called on the UI thread.
-    virtual void PostMessageFromNativeProcess(
-        int port_id,
-        scoped_ptr<base::ListValue> message) = 0;
+    virtual void PostMessageFromNativeProcess(int port_id,
+                                              const std::string& message) = 0;
     virtual void CloseChannel(int port_id,
                               const std::string& error_message) = 0;
   };
diff --git a/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc b/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc
index d8fe2c4..6fc7d3a 100644
--- a/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc
+++ b/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc
@@ -104,15 +104,19 @@ class NativeMessagingTest : public ::testing::Test,
 
   virtual void PostMessageFromNativeProcess(
       int port_id,
-      scoped_ptr<base::ListValue> message_as_list) OVERRIDE  {
-    // |message_as_list| should contain a single DictionaryValue. Extract it
-    // into |last_message_|.
-    ASSERT_EQ(1u, message_as_list->GetSize());
-    base::Value* last_message_value = NULL;
-    message_as_list->Remove(0, &last_message_value);
-    ASSERT_EQ(base::Value::TYPE_DICTIONARY, last_message_value->GetType());
-    last_message_.reset(
-        static_cast<base::DictionaryValue*>(last_message_value));
+      const std::string& message) OVERRIDE  {
+    last_message_ = message;
+
+    // Parse the message.
+    base::Value* parsed = base::JSONReader::Read(message);
+    base::DictionaryValue* dict_value;
+    if (parsed && parsed->GetAsDictionary(&dict_value)) {
+      last_message_parsed_.reset(dict_value);
+    } else {
+      LOG(ERROR) << "Failed to parse " << message;
+      last_message_parsed_.reset();
+      delete parsed;
+    }
 
     if (read_message_run_loop_)
       read_message_run_loop_->Quit();
@@ -146,7 +150,8 @@ class NativeMessagingTest : public ::testing::Test,
   base::FilePath user_data_dir_;
   scoped_ptr<base::RunLoop> read_message_run_loop_;
   content::TestBrowserThreadBundle thread_bundle_;
-  scoped_ptr<DictionaryValue> last_message_;
+  std::string last_message_;
+  scoped_ptr<base::DictionaryValue> last_message_parsed_;
 };
 
 // Read a single message from a local file.
@@ -163,26 +168,19 @@ TEST_F(NativeMessagingTest, SingleSendMessageRead) {
   read_message_run_loop_.reset(new base::RunLoop());
   read_message_run_loop_->RunUntilIdle();
 
-  if (!last_message_) {
+  if (last_message_.empty()) {
     read_message_run_loop_.reset(new base::RunLoop());
     native_message_process_host_->ReadNowForTesting();
     read_message_run_loop_->Run();
   }
-  ASSERT_TRUE(last_message_);
-
-  scoped_ptr<base::Value> kTestMessageAsValue(
-      base::JSONReader::Read(kTestMessage));
-  ASSERT_TRUE(kTestMessageAsValue);
-  EXPECT_TRUE(base::Value::Equals(kTestMessageAsValue.get(),
-                                  last_message_.get()))
-      << "Expected " << *kTestMessageAsValue << " got " << *last_message_;
+  EXPECT_EQ(kTestMessage, last_message_);
 }
 
 // Tests sending a single message. The message should get written to
 // |temp_file| and should match the contents of single_message_request.msg.
 TEST_F(NativeMessagingTest, SingleSendMessageWrite) {
   base::FilePath temp_output_file = temp_dir_.path().AppendASCII("output");
-  base::FilePath temp_input_file = CreateTempFileWithMessage("{}");
+  base::FilePath temp_input_file = CreateTempFileWithMessage(std::string());
 
   scoped_ptr<NativeProcessLauncher> launcher(
       new FakeLauncher(temp_input_file, temp_output_file));
@@ -228,29 +226,30 @@ TEST_F(NativeMessagingTest, EchoConnect) {
   native_message_process_host_->Send("{\"text\": \"Hello.\"}");
   read_message_run_loop_.reset(new base::RunLoop());
   read_message_run_loop_->Run();
-  ASSERT_TRUE(last_message_);
+  ASSERT_FALSE(last_message_.empty());
+  ASSERT_TRUE(last_message_parsed_);
 
   std::string expected_url = std::string("chrome-extension://") +
       kTestNativeMessagingExtensionId + "/";
   int id;
-  EXPECT_TRUE(last_message_->GetInteger("id", &id));
+  EXPECT_TRUE(last_message_parsed_->GetInteger("id", &id));
   EXPECT_EQ(1, id);
   std::string text;
-  EXPECT_TRUE(last_message_->GetString("echo.text", &text));
+  EXPECT_TRUE(last_message_parsed_->GetString("echo.text", &text));
   EXPECT_EQ("Hello.", text);
   std::string url;
-  EXPECT_TRUE(last_message_->GetString("caller_url", &url));
+  EXPECT_TRUE(last_message_parsed_->GetString("caller_url", &url));
   EXPECT_EQ(expected_url, url);
 
 
   native_message_process_host_->Send("{\"foo\": \"bar\"}");
   read_message_run_loop_.reset(new base::RunLoop());
   read_message_run_loop_->Run();
-  EXPECT_TRUE(last_message_->GetInteger("id", &id));
+  EXPECT_TRUE(last_message_parsed_->GetInteger("id", &id));
   EXPECT_EQ(2, id);
-  EXPECT_TRUE(last_message_->GetString("echo.foo", &text));
+  EXPECT_TRUE(last_message_parsed_->GetString("echo.foo", &text));
   EXPECT_EQ("bar", text);
-  EXPECT_TRUE(last_message_->GetString("caller_url", &url));
+  EXPECT_TRUE(last_message_parsed_->GetString("caller_url", &url));
   EXPECT_EQ(expected_url, url);
 }
 
diff --git a/chrome/browser/extensions/message_handler.cc b/chrome/browser/extensions/message_handler.cc
index f0576e5..8389b9a 100644
--- a/chrome/browser/extensions/message_handler.cc
+++ b/chrome/browser/extensions/message_handler.cc
@@ -4,7 +4,6 @@
 
 #include "chrome/browser/extensions/message_handler.h"
 
-#include "base/values.h"
 #include "chrome/browser/extensions/api/messaging/message_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/profiles/profile.h"
@@ -44,12 +43,12 @@ void MessageHandler::RenderViewHostInitialized() {
 }
 
 void MessageHandler::OnPostMessage(int port_id,
-                                   const base::ListValue& message) {
+                                   const std::string& message) {
   Profile* profile = Profile::FromBrowserContext(
       render_view_host()->GetProcess()->GetBrowserContext());
   MessageService* message_service = MessageService::Get(profile);
   if (message_service) {
-    message_service->PostMessage(port_id, make_scoped_ptr(message.DeepCopy()));
+    message_service->PostMessage(port_id, message);
   }
 }
 
diff --git a/chrome/browser/extensions/message_handler.h b/chrome/browser/extensions/message_handler.h
index 7142682..140105d 100644
--- a/chrome/browser/extensions/message_handler.h
+++ b/chrome/browser/extensions/message_handler.h
@@ -9,10 +9,6 @@
 
 #include "content/public/browser/render_view_host_observer.h"
 
-namespace base {
-class ListValue;
-}
-
 namespace extensions {
 
 // Filters and dispatches extension-related IPC messages that arrive from
@@ -37,7 +33,7 @@ class MessageHandler : public content::RenderViewHostObserver {
 
  private:
   // Message handlers.
-  void OnPostMessage(int port_id, const base::ListValue& message);
+  void OnPostMessage(int port_id, const std::string& message);
 
   DISALLOW_COPY_AND_ASSIGN(MessageHandler);
 };