From ef4b36d62185758c6b8cd34675beb12050a66901 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 20 Sep 2013 22:55:42 +0800 Subject: [PATCH] Use string16 instead of std::string when sending IPC messages. The underlying V8::String is represented in UTF18, by using string16 in IPC messages we can avoid the overhead of encode conversion. --- atom.gyp | 1 + browser/api/atom_api_browser_ipc.cc | 3 ++- browser/api/atom_api_event.cc | 6 ++++-- browser/api/atom_api_event.h | 5 ++--- browser/api/atom_api_menu.cc | 12 +----------- browser/api/atom_api_window.cc | 12 ++---------- browser/api/atom_browser_bindings.cc | 11 ++++++----- browser/api/atom_browser_bindings.h | 9 ++++----- browser/native_window.cc | 6 +++--- browser/native_window.h | 4 ++-- common/api/api_messages.h | 11 +++++------ common/string16_conversions.h | 23 +++++++++++++++++++++++ renderer/api/atom_api_renderer_ipc.cc | 10 +++++----- renderer/api/atom_renderer_bindings.cc | 5 +++-- renderer/api/atom_renderer_bindings.h | 6 +++--- renderer/atom_render_view_observer.cc | 2 +- renderer/atom_render_view_observer.h | 2 +- 17 files changed, 68 insertions(+), 60 deletions(-) create mode 100644 common/string16_conversions.h diff --git a/atom.gyp b/atom.gyp index c9aff86aa4a2..a5bfac70d986 100644 --- a/atom.gyp +++ b/atom.gyp @@ -157,6 +157,7 @@ 'common/platform_util.h', 'common/platform_util_mac.mm', 'common/platform_util_win.cc', + 'common/string16_conversions.h', 'common/v8_value_converter_impl.cc', 'common/v8_value_converter_impl.h', 'renderer/api/atom_api_renderer_ipc.cc', diff --git a/browser/api/atom_api_browser_ipc.cc b/browser/api/atom_api_browser_ipc.cc index ac9d6c277c4b..d100cb4e0c99 100644 --- a/browser/api/atom_api_browser_ipc.cc +++ b/browser/api/atom_api_browser_ipc.cc @@ -6,6 +6,7 @@ #include "base/values.h" #include "common/api/api_messages.h" +#include "common/string16_conversions.h" #include "common/v8_value_converter_impl.h" #include "content/public/browser/render_view_host.h" #include "vendor/node/src/node.h" @@ -25,7 +26,7 @@ v8::Handle BrowserIPC::Send(const v8::Arguments &args) { if (!args[0]->IsString() || !args[1]->IsNumber() || !args[2]->IsNumber()) return node::ThrowTypeError("Bad argument"); - std::string channel(*v8::String::Utf8Value(args[0])); + string16 channel(V8ValueToUTF16(args[0])); int process_id = args[1]->IntegerValue(); int routing_id = args[2]->IntegerValue(); diff --git a/browser/api/atom_api_event.cc b/browser/api/atom_api_event.cc index ad666deb5768..accfb2995517 100644 --- a/browser/api/atom_api_event.cc +++ b/browser/api/atom_api_event.cc @@ -4,6 +4,8 @@ #include "browser/api/atom_api_event.h" +#include "common/string16_conversions.h" + using node::node_isolate; namespace atom { @@ -40,10 +42,10 @@ v8::Handle Event::CreateV8Object() { } // static -std::string Event::GetReturnValue(v8::Handle event) { +string16 Event::GetReturnValue(v8::Handle event) { v8::HandleScope scope; v8::Local json = event->Get(v8::String::New("returnValue")); - return *v8::String::Utf8Value(json); + return V8ValueToUTF16(json); } v8::Handle Event::New(const v8::Arguments &args) { diff --git a/browser/api/atom_api_event.h b/browser/api/atom_api_event.h index d8585315b8ec..6517e14b85d4 100644 --- a/browser/api/atom_api_event.h +++ b/browser/api/atom_api_event.h @@ -5,9 +5,8 @@ #ifndef ATOM_BROWSER_ATOM_API_EVENT_H_ #define ATOM_BROWSER_ATOM_API_EVENT_H_ -#include - #include "base/basictypes.h" +#include "base/string16.h" #include "vendor/node/src/node_object_wrap.h" namespace atom { @@ -22,7 +21,7 @@ class Event : public node::ObjectWrap { static v8::Handle CreateV8Object(); // Get JSON string of the event.returnValue from a Event object. - static std::string GetReturnValue(v8::Handle event); + static string16 GetReturnValue(v8::Handle event); // Accessor to return handle_, this follows Google C++ Style. v8::Persistent& handle() { return handle_; } diff --git a/browser/api/atom_api_menu.cc b/browser/api/atom_api_menu.cc index 27879e7993d4..df9f97a98afc 100644 --- a/browser/api/atom_api_menu.cc +++ b/browser/api/atom_api_menu.cc @@ -6,6 +6,7 @@ #include "browser/api/atom_api_window.h" #include "browser/ui/accelerator_util.h" +#include "common/string16_conversions.h" #define UNWRAP_MEMNU_AND_CHECK \ Menu* self = ObjectWrap::Unwrap(args.This()); \ @@ -18,17 +19,6 @@ namespace api { namespace { -// Converts a V8 value to a string16. -string16 V8ValueToUTF16(v8::Handle value) { - v8::String::Value s(value); - return string16(reinterpret_cast(*s), s.length()); -} - -// Converts string16 to V8 String. -v8::Handle UTF16ToV8Value(const string16& s) { - return v8::String::New(reinterpret_cast(s.data()), s.size()); -} - // Call method of delegate object. v8::Handle CallDelegate(v8::Handle default_value, v8::Handle menu, diff --git a/browser/api/atom_api_window.cc b/browser/api/atom_api_window.cc index 4749f7ec04b0..192c318e6795 100644 --- a/browser/api/atom_api_window.cc +++ b/browser/api/atom_api_window.cc @@ -6,6 +6,7 @@ #include "base/values.h" #include "browser/native_window.h" +#include "common/string16_conversions.h" #include "common/v8_value_converter_impl.h" #include "content/public/browser/navigation_entry.h" #include "content/public/browser/web_contents.h" @@ -27,15 +28,6 @@ namespace atom { namespace api { -namespace { - -// Converts string16 to V8 String. -v8::Handle UTF16ToV8String(const string16& s) { - return v8::String::New(reinterpret_cast(s.data()), s.size()); -} - -} // namespace - Window::Window(v8::Handle wrapper, base::DictionaryValue* options) : EventEmitter(wrapper), window_(NativeWindow::Create(options)) { @@ -473,7 +465,7 @@ v8::Handle Window::GetPageTitle(const v8::Arguments &args) { string16 title = self->window_->GetWebContents()->GetTitle(); - return UTF16ToV8String(title); + return UTF16ToV8Value(title); } // static diff --git a/browser/api/atom_browser_bindings.cc b/browser/api/atom_browser_bindings.cc index 544f49fc0c5e..7c69de97f97b 100644 --- a/browser/api/atom_browser_bindings.cc +++ b/browser/api/atom_browser_bindings.cc @@ -9,6 +9,7 @@ #include "base/logging.h" #include "base/values.h" #include "browser/api/atom_api_event.h" +#include "common/string16_conversions.h" #include "common/v8_value_converter_impl.h" #include "content/public/browser/browser_thread.h" #include "vendor/node/src/node.h" @@ -43,7 +44,7 @@ void AtomBrowserBindings::AfterLoad() { void AtomBrowserBindings::OnRendererMessage(int process_id, int routing_id, - const std::string& channel, + const string16& channel, const base::ListValue& args) { v8::HandleScope scope; @@ -54,7 +55,7 @@ void AtomBrowserBindings::OnRendererMessage(int process_id, // process.emit(channel, 'message', process_id, routing_id); std::vector> arguments; arguments.reserve(3 + args.GetSize()); - arguments.push_back(v8::String::New(channel.c_str(), channel.size())); + arguments.push_back(UTF16ToV8Value(channel)); const base::Value* value; if (args.Get(0, &value)) arguments.push_back(converter->ToV8Value(value, context)); @@ -73,9 +74,9 @@ void AtomBrowserBindings::OnRendererMessage(int process_id, void AtomBrowserBindings::OnRendererMessageSync( int process_id, int routing_id, - const std::string& channel, + const string16& channel, const base::ListValue& args, - std::string* result) { + string16* result) { v8::HandleScope scope; v8::Handle context = v8::Context::GetCurrent(); @@ -87,7 +88,7 @@ void AtomBrowserBindings::OnRendererMessageSync( // process.emit(channel, 'sync-message', event, process_id, routing_id); std::vector> arguments; arguments.reserve(3 + args.GetSize()); - arguments.push_back(v8::String::New(channel.c_str(), channel.size())); + arguments.push_back(UTF16ToV8Value(channel)); const base::Value* value; if (args.Get(0, &value)) arguments.push_back(converter->ToV8Value(value, context)); diff --git a/browser/api/atom_browser_bindings.h b/browser/api/atom_browser_bindings.h index d936c66af94e..92ddd4990a6a 100644 --- a/browser/api/atom_browser_bindings.h +++ b/browser/api/atom_browser_bindings.h @@ -5,8 +5,7 @@ #ifndef ATOM_BROWSER_API_ATOM_BROWSER_BINDINGS_ #define ATOM_BROWSER_API_ATOM_BROWSER_BINDINGS_ -#include - +#include "base/string16.h" #include "common/api/atom_bindings.h" namespace base { @@ -26,15 +25,15 @@ class AtomBrowserBindings : public AtomBindings { // Called when received a message from renderer. void OnRendererMessage(int process_id, int routing_id, - const std::string& channel, + const string16& channel, const base::ListValue& args); // Called when received a synchronous message from renderer. void OnRendererMessageSync(int process_id, int routing_id, - const std::string& channel, + const string16& channel, const base::ListValue& args, - std::string* result); + string16* result); // The require('atom').browserMainParts object. v8::Handle browser_main_parts() { diff --git a/browser/native_window.cc b/browser/native_window.cc index 6c4f3dfa5310..9a7ba7d68fca 100644 --- a/browser/native_window.cc +++ b/browser/native_window.cc @@ -341,7 +341,7 @@ void NativeWindow::Observe(int type, } } -void NativeWindow::OnRendererMessage(const std::string& channel, +void NativeWindow::OnRendererMessage(const string16& channel, const base::ListValue& args) { AtomBrowserMainParts::Get()->atom_bindings()->OnRendererMessage( GetWebContents()->GetRenderProcessHost()->GetID(), @@ -350,10 +350,10 @@ void NativeWindow::OnRendererMessage(const std::string& channel, args); } -void NativeWindow::OnRendererMessageSync(const std::string& channel, +void NativeWindow::OnRendererMessageSync(const string16& channel, const base::ListValue& args, IPC::Message* reply_msg) { - std::string json; + string16 json; AtomBrowserMainParts::Get()->atom_bindings()->OnRendererMessageSync( GetWebContents()->GetRenderProcessHost()->GetID(), GetWebContents()->GetRoutingID(), diff --git a/browser/native_window.h b/browser/native_window.h index 03a28832588b..b170c57af86d 100644 --- a/browser/native_window.h +++ b/browser/native_window.h @@ -177,10 +177,10 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate, private: void RendererUnresponsiveDelayed(); - void OnRendererMessage(const std::string& channel, + void OnRendererMessage(const string16& channel, const base::ListValue& args); - void OnRendererMessageSync(const std::string& channel, + void OnRendererMessageSync(const string16& channel, const base::ListValue& args, IPC::Message* reply_msg); diff --git a/common/api/api_messages.h b/common/api/api_messages.h index c93abebd20df..8d21a7d93c03 100644 --- a/common/api/api_messages.h +++ b/common/api/api_messages.h @@ -4,8 +4,7 @@ // Multiply-included file, no traditional include guard. -#include - +#include "base/string16.h" #include "base/values.h" #include "common/draggable_region.h" #include "content/public/common/common_param_traits.h" @@ -22,16 +21,16 @@ IPC_STRUCT_TRAITS_BEGIN(atom::DraggableRegion) IPC_STRUCT_TRAITS_END() IPC_MESSAGE_ROUTED2(AtomViewHostMsg_Message, - std::string /* channel */, + string16 /* channel */, ListValue /* arguments */) IPC_SYNC_MESSAGE_ROUTED2_1(AtomViewHostMsg_Message_Sync, - std::string /* channel */, + string16 /* channel */, ListValue /* arguments */, - std::string /* result (in JSON) */) + string16 /* result (in JSON) */) IPC_MESSAGE_ROUTED2(AtomViewMsg_Message, - std::string /* channel */, + string16 /* channel */, ListValue /* arguments */) // Sent by the renderer when the draggable regions are updated. diff --git a/common/string16_conversions.h b/common/string16_conversions.h new file mode 100644 index 000000000000..314eb1b50a0c --- /dev/null +++ b/common/string16_conversions.h @@ -0,0 +1,23 @@ +// Copyright (c) 2013 GitHub, Inc. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMMON_STRING16_CONVERSIONS_H_ +#define COMMON_STRING16_CONVERSIONS_H_ + +#include "v8/include/v8.h" + +class string16; + +// Converts a V8 value to a string16. +inline string16 V8ValueToUTF16(v8::Handle value) { + v8::String::Value s(value); + return string16(reinterpret_cast(*s), s.length()); +} + +// Converts string16 to V8 String. +inline v8::Handle UTF16ToV8Value(const string16& s) { + return v8::String::New(reinterpret_cast(s.data()), s.size()); +} + +#endif // COMMON_STRING16_CONVERSIONS_H_ diff --git a/renderer/api/atom_api_renderer_ipc.cc b/renderer/api/atom_api_renderer_ipc.cc index 1f7a6f5c661f..4bdbd4b8094f 100644 --- a/renderer/api/atom_api_renderer_ipc.cc +++ b/renderer/api/atom_api_renderer_ipc.cc @@ -6,6 +6,7 @@ #include "base/values.h" #include "common/api/api_messages.h" +#include "common/string16_conversions.h" #include "content/public/renderer/render_view.h" #include "content/public/renderer/v8_value_converter.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h" @@ -47,8 +48,7 @@ v8::Handle RendererIPC::Send(const v8::Arguments &args) { if (!args[0]->IsString()) return node::ThrowTypeError("Bad argument"); - std::string channel(*v8::String::Utf8Value(args[0])); - + string16 channel(V8ValueToUTF16(args[0])); RenderView* render_view = GetCurrentRenderView(); // Convert Arguments to Array, so we can use V8ValueConverter to convert it @@ -82,7 +82,7 @@ v8::Handle RendererIPC::SendSync(const v8::Arguments &args) { return node::ThrowTypeError("Bad argument"); v8::Handle context = v8::Context::GetCurrent(); - std::string channel(*v8::String::Utf8Value(args[0])); + string16 channel(V8ValueToUTF16(args[0])); // Convert Arguments to Array, so we can use V8ValueConverter to convert it // to ListValue. @@ -97,7 +97,7 @@ v8::Handle RendererIPC::SendSync(const v8::Arguments &args) { RenderView* render_view = GetCurrentRenderView(); - std::string json; + string16 json; IPC::SyncMessage* message = new AtomViewHostMsg_Message_Sync( render_view->GetRoutingID(), channel, @@ -110,7 +110,7 @@ v8::Handle RendererIPC::SendSync(const v8::Arguments &args) { if (!success) return node::ThrowError("Unable to send AtomViewHostMsg_Message_Sync"); - return scope.Close(v8::String::New(json.data(), json.size())); + return scope.Close(UTF16ToV8Value(json)); } // static diff --git a/renderer/api/atom_renderer_bindings.cc b/renderer/api/atom_renderer_bindings.cc index 9471277d0002..e0260b9a279a 100644 --- a/renderer/api/atom_renderer_bindings.cc +++ b/renderer/api/atom_renderer_bindings.cc @@ -8,6 +8,7 @@ #include "base/logging.h" #include "base/values.h" +#include "common/string16_conversions.h" #include "content/public/renderer/render_view.h" #include "content/public/renderer/v8_value_converter.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h" @@ -51,7 +52,7 @@ void AtomRendererBindings::BindToFrame(WebFrame* frame) { AtomBindings::BindTo(GetProcessObject(context)); } -void AtomRendererBindings::OnBrowserMessage(const std::string& channel, +void AtomRendererBindings::OnBrowserMessage(const string16& channel, const base::ListValue& args) { if (!render_view_->GetWebView()) return; @@ -70,7 +71,7 @@ void AtomRendererBindings::OnBrowserMessage(const std::string& channel, std::vector> arguments; arguments.reserve(1 + args.GetSize()); - arguments.push_back(v8::String::New(channel.c_str(), channel.size())); + arguments.push_back(UTF16ToV8Value(channel)); for (size_t i = 0; i < args.GetSize(); i++) { const base::Value* value; diff --git a/renderer/api/atom_renderer_bindings.h b/renderer/api/atom_renderer_bindings.h index 6132ed457a82..394cdea41ec2 100644 --- a/renderer/api/atom_renderer_bindings.h +++ b/renderer/api/atom_renderer_bindings.h @@ -5,10 +5,10 @@ #ifndef ATOM_RENDERER_API_ATOM_RENDERER_BINDINGS_H_ #define ATOM_RENDERER_API_ATOM_RENDERER_BINDINGS_H_ -#include - #include "common/api/atom_bindings.h" +#include "base/string16.h" + namespace base { class ListValue; } @@ -32,7 +32,7 @@ class AtomRendererBindings : public AtomBindings { void BindToFrame(WebKit::WebFrame* frame); // Dispatch messages from browser. - void OnBrowserMessage(const std::string& channel, + void OnBrowserMessage(const string16& channel, const base::ListValue& args); private: diff --git a/renderer/atom_render_view_observer.cc b/renderer/atom_render_view_observer.cc index 6a394e447379..4405d531afb0 100644 --- a/renderer/atom_render_view_observer.cc +++ b/renderer/atom_render_view_observer.cc @@ -101,7 +101,7 @@ bool AtomRenderViewObserver::OnMessageReceived(const IPC::Message& message) { return handled; } -void AtomRenderViewObserver::OnBrowserMessage(const std::string& channel, +void AtomRenderViewObserver::OnBrowserMessage(const string16& channel, const base::ListValue& args) { atom_bindings()->OnBrowserMessage(channel, args); } diff --git a/renderer/atom_render_view_observer.h b/renderer/atom_render_view_observer.h index fd984d012468..d8d90e0cb4b1 100644 --- a/renderer/atom_render_view_observer.h +++ b/renderer/atom_render_view_observer.h @@ -35,7 +35,7 @@ class AtomRenderViewObserver : content::RenderViewObserver { virtual void DraggableRegionsChanged(WebKit::WebFrame* frame) OVERRIDE; virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE; - void OnBrowserMessage(const std::string& channel, + void OnBrowserMessage(const string16& channel, const base::ListValue& args); scoped_ptr atom_bindings_;