From b225a59a15e68fd05913be51e3997f12b48d5bef Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 20 Sep 2013 21:24:04 +0800 Subject: [PATCH 01/34] Prefer event.returnValue to event.result for sync messages. --- docs/api/browser/ipc-browser.md | 2 +- docs/api/renderer/ipc-renderer.md | 2 +- renderer/api/lib/ipc.coffee | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/api/browser/ipc-browser.md b/docs/api/browser/ipc-browser.md index 34ebda8a4dc..2edaa29463a 100644 --- a/docs/api/browser/ipc-browser.md +++ b/docs/api/browser/ipc-browser.md @@ -18,7 +18,7 @@ Emitted when renderer sent a message to the browser. * `routingId` Integer Emitted when renderer sent a synchronous message to the browser. The receiver -should store the result in `event.result`. +should store the result in `event.returnValue`. **Note:** Due to the limitation of `EventEmitter`, returning value in the event handler has no effect, so we have to store the result by using the diff --git a/docs/api/renderer/ipc-renderer.md b/docs/api/renderer/ipc-renderer.md index cdeadf39f68..4076b93b395 100644 --- a/docs/api/renderer/ipc-renderer.md +++ b/docs/api/renderer/ipc-renderer.md @@ -30,7 +30,7 @@ An example of sending synchronous message from renderer to browser: // In browser: var ipc = require('ipc'); ipc.on('browser-data-request', function(event, processId, routingId, message) { - event.result = 'THIS SOME DATA FROM THE BROWSER'; + event.returnValue = 'THIS SOME DATA FROM THE BROWSER'; }); ``` diff --git a/renderer/api/lib/ipc.coffee b/renderer/api/lib/ipc.coffee index 28b9a9692d0..88b8c1c4158 100644 --- a/renderer/api/lib/ipc.coffee +++ b/renderer/api/lib/ipc.coffee @@ -16,9 +16,11 @@ class Ipc extends EventEmitter ipc.send('ATOM_INTERNAL_MESSAGE', args...) sendSync: (args...) -> - ipc.sendSync('ATOM_INTERNAL_MESSAGE_SYNC', 'sync-message', args...).result + msg = ipc.sendSync('ATOM_INTERNAL_MESSAGE_SYNC', 'sync-message', args...) + msg.returnValue ? msg.result sendChannelSync: (args...) -> - ipc.sendSync('ATOM_INTERNAL_MESSAGE_SYNC', args...).result + msg = ipc.sendSync('ATOM_INTERNAL_MESSAGE_SYNC', args...) + msg.returnValue ? msg.result module.exports = new Ipc From a9c824eba1ea2cc028b2b813f8f8ad9d8edf685b Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 20 Sep 2013 21:37:47 +0800 Subject: [PATCH 02/34] Use event.returnValue instead of event.result in atom-shell's code. event.result is still kept for backward compatible. --- browser/atom/rpc-server.coffee | 34 +++++++++++++++++----------------- spec/main.js | 2 +- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/browser/atom/rpc-server.coffee b/browser/atom/rpc-server.coffee index f62cd098837..30e9dbd5f0c 100644 --- a/browser/atom/rpc-server.coffee +++ b/browser/atom/rpc-server.coffee @@ -63,15 +63,15 @@ unwrapArgs = (processId, routingId, args) -> ipc.on 'ATOM_BROWSER_REQUIRE', (event, processId, routingId, module) -> try - event.result = valueToMeta processId, routingId, require(module) + event.returnValue = valueToMeta processId, routingId, require(module) catch e - event.result = errorToMeta e + event.returnValue = errorToMeta e ipc.on 'ATOM_BROWSER_GLOBAL', (event, processId, routingId, name) -> try - event.result = valueToMeta processId, routingId, global[name] + event.returnValue = valueToMeta processId, routingId, global[name] catch e - event.result = errorToMeta e + event.returnValue = errorToMeta e ipc.on 'ATOM_BROWSER_RELEASE_RENDER_VIEW', (event, processId, routingId) -> objectsRegistry.clear processId, routingId @@ -80,9 +80,9 @@ ipc.on 'ATOM_BROWSER_CURRENT_WINDOW', (event, processId, routingId) -> try BrowserWindow = require 'browser-window' window = BrowserWindow.fromProcessIdAndRoutingId processId, routingId - event.result = valueToMeta processId, routingId, window + event.returnValue = valueToMeta processId, routingId, window catch e - event.result = errorToMeta e + event.returnValue = errorToMeta e ipc.on 'ATOM_BROWSER_CONSTRUCTOR', (event, processId, routingId, id, args) -> try @@ -91,18 +91,18 @@ ipc.on 'ATOM_BROWSER_CONSTRUCTOR', (event, processId, routingId, id, args) -> # Call new with array of arguments. # http://stackoverflow.com/questions/1606797/use-of-apply-with-new-operator-is-this-possible obj = new (Function::bind.apply(constructor, [null].concat(args))) - event.result = valueToMeta processId, routingId, obj + event.returnValue = valueToMeta processId, routingId, obj catch e - event.result = errorToMeta e + event.returnValue = errorToMeta e ipc.on 'ATOM_BROWSER_FUNCTION_CALL', (event, processId, routingId, id, args) -> try args = unwrapArgs processId, routingId, args func = objectsRegistry.get id ret = func.apply global, args - event.result = valueToMeta processId, routingId, ret + event.returnValue = valueToMeta processId, routingId, ret catch e - event.result = errorToMeta e + event.returnValue = errorToMeta e ipc.on 'ATOM_BROWSER_MEMBER_CONSTRUCTOR', (event, processId, routingId, id, method, args) -> try @@ -110,32 +110,32 @@ ipc.on 'ATOM_BROWSER_MEMBER_CONSTRUCTOR', (event, processId, routingId, id, meth constructor = objectsRegistry.get(id)[method] # Call new with array of arguments. obj = new (Function::bind.apply(constructor, [null].concat(args))) - event.result = valueToMeta processId, routingId, obj + event.returnValue = valueToMeta processId, routingId, obj catch e - event.result = errorToMeta e + event.returnValue = errorToMeta e ipc.on 'ATOM_BROWSER_MEMBER_CALL', (event, processId, routingId, id, method, args) -> try args = unwrapArgs processId, routingId, args obj = objectsRegistry.get id ret = obj[method].apply(obj, args) - event.result = valueToMeta processId, routingId, ret + event.returnValue = valueToMeta processId, routingId, ret catch e - event.result = errorToMeta e + event.returnValue = errorToMeta e ipc.on 'ATOM_BROWSER_MEMBER_SET', (event, processId, routingId, id, name, value) -> try obj = objectsRegistry.get id obj[name] = value catch e - event.result = errorToMeta e + event.returnValue = errorToMeta e ipc.on 'ATOM_BROWSER_MEMBER_GET', (event, processId, routingId, id, name) -> try obj = objectsRegistry.get id - event.result = valueToMeta processId, routingId, obj[name] + event.returnValue = valueToMeta processId, routingId, obj[name] catch e - event.result = errorToMeta e + event.returnValue = errorToMeta e ipc.on 'ATOM_BROWSER_DEREFERENCE', (processId, routingId, storeId) -> objectsRegistry.remove processId, routingId, storeId diff --git a/spec/main.js b/spec/main.js index f2e52627946..ac211ff5a23 100644 --- a/spec/main.js +++ b/spec/main.js @@ -23,7 +23,7 @@ ipc.on('process.exit', function(pid, rid, code) { }); ipc.on('eval', function(ev, pid, rid, script) { - ev.result = eval(script); + ev.returnValue = eval(script); }); process.on('uncaughtException', function() { From 07b5039c64007d470e68981472b926211db3c0b4 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 20 Sep 2013 21:39:07 +0800 Subject: [PATCH 03/34] Make sure all sync messages get a return value. --- browser/atom/rpc-server.coffee | 2 ++ renderer/api/lib/remote.coffee | 1 + 2 files changed, 3 insertions(+) diff --git a/browser/atom/rpc-server.coffee b/browser/atom/rpc-server.coffee index 30e9dbd5f0c..0e5c707c403 100644 --- a/browser/atom/rpc-server.coffee +++ b/browser/atom/rpc-server.coffee @@ -75,6 +75,7 @@ ipc.on 'ATOM_BROWSER_GLOBAL', (event, processId, routingId, name) -> ipc.on 'ATOM_BROWSER_RELEASE_RENDER_VIEW', (event, processId, routingId) -> objectsRegistry.clear processId, routingId + event.returnValue = null ipc.on 'ATOM_BROWSER_CURRENT_WINDOW', (event, processId, routingId) -> try @@ -127,6 +128,7 @@ ipc.on 'ATOM_BROWSER_MEMBER_SET', (event, processId, routingId, id, name, value) try obj = objectsRegistry.get id obj[name] = value + event.returnValue = null catch e event.returnValue = errorToMeta e diff --git a/renderer/api/lib/remote.coffee b/renderer/api/lib/remote.coffee index d839a9899a9..7eaafc20aa5 100644 --- a/renderer/api/lib/remote.coffee +++ b/renderer/api/lib/remote.coffee @@ -72,6 +72,7 @@ metaToValue = (meta) -> ret.__defineSetter__ member.name, (value) -> # Set member data. ipc.sendChannelSync 'ATOM_BROWSER_MEMBER_SET', meta.id, member.name, value + value ret.__defineGetter__ member.name, -> # Get member data. From ef5a4b5fe07bca5ef38ad121bb75ccc403817267 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 20 Sep 2013 22:32:59 +0800 Subject: [PATCH 04/34] Pass synchronous messages by JSON string. We are going to use IPC_MESSAGE_HANDLER_DELAY_REPLY to handle synchronous messages but DictionaryValue is not copyable, so we pass the JSON string instead. --- browser/api/atom_api_event.cc | 7 +++++++ browser/api/atom_api_event.h | 5 +++++ browser/api/atom_browser_bindings.cc | 9 +++------ browser/api/atom_browser_bindings.h | 3 +-- browser/api/lib/ipc.coffee | 11 +++++++++-- browser/native_window.cc | 11 ++++++++--- browser/native_window.h | 6 +++++- common/api/api_messages.h | 2 +- renderer/api/atom_api_renderer_ipc.cc | 6 +++--- renderer/api/lib/ipc.coffee | 4 ++-- 10 files changed, 44 insertions(+), 20 deletions(-) diff --git a/browser/api/atom_api_event.cc b/browser/api/atom_api_event.cc index 9854401760b..ad666deb576 100644 --- a/browser/api/atom_api_event.cc +++ b/browser/api/atom_api_event.cc @@ -39,6 +39,13 @@ v8::Handle Event::CreateV8Object() { return scope.Close(v8_event); } +// static +std::string Event::GetReturnValue(v8::Handle event) { + v8::HandleScope scope; + v8::Local json = event->Get(v8::String::New("returnValue")); + return *v8::String::Utf8Value(json); +} + v8::Handle Event::New(const v8::Arguments &args) { Event* event = new Event; event->Wrap(args.This()); diff --git a/browser/api/atom_api_event.h b/browser/api/atom_api_event.h index c0ab00078bc..d8585315b8e 100644 --- a/browser/api/atom_api_event.h +++ b/browser/api/atom_api_event.h @@ -5,6 +5,8 @@ #ifndef ATOM_BROWSER_ATOM_API_EVENT_H_ #define ATOM_BROWSER_ATOM_API_EVENT_H_ +#include + #include "base/basictypes.h" #include "vendor/node/src/node_object_wrap.h" @@ -19,6 +21,9 @@ class Event : public node::ObjectWrap { // Create a V8 Event object. static v8::Handle CreateV8Object(); + // Get JSON string of the event.returnValue from a Event object. + static std::string GetReturnValue(v8::Handle event); + // Accessor to return handle_, this follows Google C++ Style. v8::Persistent& handle() { return handle_; } diff --git a/browser/api/atom_browser_bindings.cc b/browser/api/atom_browser_bindings.cc index fb102ff1c75..544f49fc0c5 100644 --- a/browser/api/atom_browser_bindings.cc +++ b/browser/api/atom_browser_bindings.cc @@ -8,6 +8,7 @@ #include "base/logging.h" #include "base/values.h" +#include "browser/api/atom_api_event.h" #include "common/v8_value_converter_impl.h" #include "content/public/browser/browser_thread.h" #include "vendor/node/src/node.h" @@ -74,7 +75,7 @@ void AtomBrowserBindings::OnRendererMessageSync( int routing_id, const std::string& channel, const base::ListValue& args, - base::DictionaryValue* result) { + std::string* result) { v8::HandleScope scope; v8::Handle context = v8::Context::GetCurrent(); @@ -101,11 +102,7 @@ void AtomBrowserBindings::OnRendererMessageSync( } node::MakeCallback(node::process, "emit", arguments.size(), &arguments[0]); - - scoped_ptr base_event(converter->FromV8Value(event, context)); - DCHECK(base_event && base_event->IsType(base::Value::TYPE_DICTIONARY)); - - result->Swap(static_cast(base_event.get())); + *result = api::Event::GetReturnValue(event); } } // namespace atom diff --git a/browser/api/atom_browser_bindings.h b/browser/api/atom_browser_bindings.h index 32a6fae6289..d936c66af94 100644 --- a/browser/api/atom_browser_bindings.h +++ b/browser/api/atom_browser_bindings.h @@ -10,7 +10,6 @@ #include "common/api/atom_bindings.h" namespace base { -class DictionaryValue; class ListValue; } @@ -35,7 +34,7 @@ class AtomBrowserBindings : public AtomBindings { int routing_id, const std::string& channel, const base::ListValue& args, - base::DictionaryValue* result); + std::string* result); // The require('atom').browserMainParts object. v8::Handle browser_main_parts() { diff --git a/browser/api/lib/ipc.coffee b/browser/api/lib/ipc.coffee index 2aa4d06be59..8e81e0d5fce 100644 --- a/browser/api/lib/ipc.coffee +++ b/browser/api/lib/ipc.coffee @@ -14,8 +14,15 @@ class Ipc extends EventEmitter constructor: -> process.on 'ATOM_INTERNAL_MESSAGE', (args...) => @emit(args...) - process.on 'ATOM_INTERNAL_MESSAGE_SYNC', (args...) => - @emit(args...) + process.on 'ATOM_INTERNAL_MESSAGE_SYNC', (channel, event, args...) => + returnValue = 'null' + get = -> returnValue + set = (value) -> returnValue = JSON.stringify(value) + + Object.defineProperty event, 'returnValue', {get, set} + Object.defineProperty event, 'result', {get, set} + + @emit(channel, event, args...) send: (processId, routingId, args...) -> @sendChannel(processId, routingId, 'message', args...) diff --git a/browser/native_window.cc b/browser/native_window.cc index 154a1a60738..6c4f3dfa531 100644 --- a/browser/native_window.cc +++ b/browser/native_window.cc @@ -300,7 +300,8 @@ bool NativeWindow::OnMessageReceived(const IPC::Message& message) { bool handled = true; IPC_BEGIN_MESSAGE_MAP(NativeWindow, message) IPC_MESSAGE_HANDLER(AtomViewHostMsg_Message, OnRendererMessage) - IPC_MESSAGE_HANDLER(AtomViewHostMsg_Message_Sync, OnRendererMessageSync) + IPC_MESSAGE_HANDLER_DELAY_REPLY(AtomViewHostMsg_Message_Sync, + OnRendererMessageSync) IPC_MESSAGE_HANDLER(AtomViewHostMsg_UpdateDraggableRegions, UpdateDraggableRegions) IPC_MESSAGE_UNHANDLED(handled = false) @@ -351,13 +352,17 @@ void NativeWindow::OnRendererMessage(const std::string& channel, void NativeWindow::OnRendererMessageSync(const std::string& channel, const base::ListValue& args, - base::DictionaryValue* result) { + IPC::Message* reply_msg) { + std::string json; AtomBrowserMainParts::Get()->atom_bindings()->OnRendererMessageSync( GetWebContents()->GetRenderProcessHost()->GetID(), GetWebContents()->GetRoutingID(), channel, args, - result); + &json); + + AtomViewHostMsg_Message_Sync::WriteReplyParams(reply_msg, json); + Send(reply_msg); } } // namespace atom diff --git a/browser/native_window.h b/browser/native_window.h index 3316b07053c..03a28832588 100644 --- a/browser/native_window.h +++ b/browser/native_window.h @@ -35,6 +35,10 @@ class Rect; class Size; } +namespace IPC { +class Message; +} + namespace atom { class AtomJavaScriptDialogManager; @@ -178,7 +182,7 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate, void OnRendererMessageSync(const std::string& channel, const base::ListValue& args, - base::DictionaryValue* result); + IPC::Message* reply_msg); // Notification manager. content::NotificationRegistrar registrar_; diff --git a/common/api/api_messages.h b/common/api/api_messages.h index ee031e6487e..c93abebd20d 100644 --- a/common/api/api_messages.h +++ b/common/api/api_messages.h @@ -28,7 +28,7 @@ IPC_MESSAGE_ROUTED2(AtomViewHostMsg_Message, IPC_SYNC_MESSAGE_ROUTED2_1(AtomViewHostMsg_Message_Sync, std::string /* channel */, ListValue /* arguments */, - DictionaryValue /* result */) + std::string /* result (in JSON) */) IPC_MESSAGE_ROUTED2(AtomViewMsg_Message, std::string /* channel */, diff --git a/renderer/api/atom_api_renderer_ipc.cc b/renderer/api/atom_api_renderer_ipc.cc index 022ad28741d..1f7a6f5c661 100644 --- a/renderer/api/atom_api_renderer_ipc.cc +++ b/renderer/api/atom_api_renderer_ipc.cc @@ -97,12 +97,12 @@ v8::Handle RendererIPC::SendSync(const v8::Arguments &args) { RenderView* render_view = GetCurrentRenderView(); - base::DictionaryValue result; + std::string json; IPC::SyncMessage* message = new AtomViewHostMsg_Message_Sync( render_view->GetRoutingID(), channel, *static_cast(arguments.get()), - &result); + &json); // Enable the UI thread in browser to receive messages. message->EnableMessagePumping(); bool success = render_view->Send(message); @@ -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(converter->ToV8Value(&result, context)); + return scope.Close(v8::String::New(json.data(), json.size())); } // static diff --git a/renderer/api/lib/ipc.coffee b/renderer/api/lib/ipc.coffee index 88b8c1c4158..c54545d8a2a 100644 --- a/renderer/api/lib/ipc.coffee +++ b/renderer/api/lib/ipc.coffee @@ -17,10 +17,10 @@ class Ipc extends EventEmitter sendSync: (args...) -> msg = ipc.sendSync('ATOM_INTERNAL_MESSAGE_SYNC', 'sync-message', args...) - msg.returnValue ? msg.result + JSON.parse(msg) sendChannelSync: (args...) -> msg = ipc.sendSync('ATOM_INTERNAL_MESSAGE_SYNC', args...) - msg.returnValue ? msg.result + JSON.parse(msg) module.exports = new Ipc From ef4b36d62185758c6b8cd34675beb12050a66901 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 20 Sep 2013 22:55:42 +0800 Subject: [PATCH 05/34] 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 c9aff86aa4a..a5bfac70d98 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 ac9d6c277c4..d100cb4e0c9 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 ad666deb576..accfb299551 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 d8585315b8e..6517e14b85d 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 27879e7993d..df9f97a98af 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 4749f7ec04b..192c318e679 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 544f49fc0c5..7c69de97f97 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 d936c66af94..92ddd4990a6 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 6c4f3dfa531..9a7ba7d68fc 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 03a28832588..b170c57af86 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 c93abebd20d..8d21a7d93c0 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 00000000000..314eb1b50a0 --- /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 1f7a6f5c661..4bdbd4b8094 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 9471277d000..e0260b9a279 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 6132ed457a8..394cdea41ec 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 6a394e44737..4405d531afb 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 fd984d01246..d8d90e0cb4b 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_; From d443b3644645c8ea1c94ca3dc7df212a110a4aa7 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 22 Sep 2013 09:52:58 +0800 Subject: [PATCH 06/34] Send reply for sync messages when event.returnValue is set. --- browser/api/atom_api_event.cc | 38 +++++++++++++++++++++++++--- browser/api/atom_api_event.h | 18 +++++++++++-- browser/api/atom_browser_bindings.cc | 8 +++--- browser/api/atom_browser_bindings.h | 8 +++++- browser/api/lib/ipc.coffee | 7 +++-- browser/native_window.cc | 7 ++--- 6 files changed, 70 insertions(+), 16 deletions(-) diff --git a/browser/api/atom_api_event.cc b/browser/api/atom_api_event.cc index accfb299551..d9b523bd903 100644 --- a/browser/api/atom_api_event.cc +++ b/browser/api/atom_api_event.cc @@ -4,7 +4,9 @@ #include "browser/api/atom_api_event.h" +#include "common/api/api_messages.h" #include "common/string16_conversions.h" +#include "ipc/ipc_sender.h" using node::node_isolate; @@ -15,7 +17,9 @@ namespace api { v8::Persistent Event::constructor_template_; Event::Event() - : prevent_default_(false) { + : sender_(NULL), + message_(NULL), + prevent_default_(false) { } Event::~Event() { @@ -33,6 +37,7 @@ v8::Handle Event::CreateV8Object() { constructor_template_->SetClassName(v8::String::NewSymbol("Event")); NODE_SET_PROTOTYPE_METHOD(t, "preventDefault", PreventDefault); + NODE_SET_PROTOTYPE_METHOD(t, "sendReply", SendReply); } v8::Handle v8_event = @@ -48,14 +53,24 @@ string16 Event::GetReturnValue(v8::Handle event) { return V8ValueToUTF16(json); } -v8::Handle Event::New(const v8::Arguments &args) { + +void Event::SetSenderAndMessage(IPC::Sender* sender, IPC::Message* message) { + DCHECK(!sender_); + DCHECK(!message_); + sender_ = sender; + message_ = message; +} + +// static +v8::Handle Event::New(const v8::Arguments& args) { Event* event = new Event; event->Wrap(args.This()); return args.This(); } -v8::Handle Event::PreventDefault(const v8::Arguments &args) { +// static +v8::Handle Event::PreventDefault(const v8::Arguments& args) { Event* event = Unwrap(args.This()); if (event == NULL) return node::ThrowError("Event is already destroyed"); @@ -65,6 +80,23 @@ v8::Handle Event::PreventDefault(const v8::Arguments &args) { return v8::Undefined(); } +// static +v8::Handle Event::SendReply(const v8::Arguments& args) { + Event* event = Unwrap(args.This()); + if (event == NULL) + return node::ThrowError("Event is already destroyed"); + + if (event->sender_ == NULL) + return node::ThrowError("Can only send reply to synchronous events"); + + string16 json = GetReturnValue(args.This()); + + AtomViewHostMsg_Message_Sync::WriteReplyParams(event->message_, json); + event->sender_->Send(event->message_); + + return v8::Undefined(); +} + } // namespace api } // namespace atom diff --git a/browser/api/atom_api_event.h b/browser/api/atom_api_event.h index 6517e14b85d..ebbec7b97ca 100644 --- a/browser/api/atom_api_event.h +++ b/browser/api/atom_api_event.h @@ -9,6 +9,11 @@ #include "base/string16.h" #include "vendor/node/src/node_object_wrap.h" +namespace IPC { +class Message; +class Sender; +} + namespace atom { namespace api { @@ -23,6 +28,9 @@ class Event : public node::ObjectWrap { // Get JSON string of the event.returnValue from a Event object. static string16 GetReturnValue(v8::Handle event); + // Pass the sender and message to be replied. + void SetSenderAndMessage(IPC::Sender* sender, IPC::Message* message); + // Accessor to return handle_, this follows Google C++ Style. v8::Persistent& handle() { return handle_; } @@ -33,11 +41,17 @@ class Event : public node::ObjectWrap { Event(); private: - static v8::Handle New(const v8::Arguments &args); - static v8::Handle PreventDefault(const v8::Arguments &args); + static v8::Handle New(const v8::Arguments& args); + + static v8::Handle PreventDefault(const v8::Arguments& args); + static v8::Handle SendReply(const v8::Arguments& args); static v8::Persistent constructor_template_; + // Replyer for the synchronous messages. + IPC::Sender* sender_; + IPC::Message* message_; + bool prevent_default_; DISALLOW_COPY_AND_ASSIGN(Event); diff --git a/browser/api/atom_browser_bindings.cc b/browser/api/atom_browser_bindings.cc index 7c69de97f97..85ba15cced6 100644 --- a/browser/api/atom_browser_bindings.cc +++ b/browser/api/atom_browser_bindings.cc @@ -76,14 +76,17 @@ void AtomBrowserBindings::OnRendererMessageSync( int routing_id, const string16& channel, const base::ListValue& args, - string16* result) { + IPC::Sender* sender, + IPC::Message* message) { v8::HandleScope scope; v8::Handle context = v8::Context::GetCurrent(); scoped_ptr converter(new V8ValueConverterImpl()); - v8::Handle event = v8::Object::New(); + // Create the event object. + v8::Handle event = api::Event::CreateV8Object(); + api::Event::Unwrap(event)->SetSenderAndMessage(sender, message); // process.emit(channel, 'sync-message', event, process_id, routing_id); std::vector> arguments; @@ -103,7 +106,6 @@ void AtomBrowserBindings::OnRendererMessageSync( } node::MakeCallback(node::process, "emit", arguments.size(), &arguments[0]); - *result = api::Event::GetReturnValue(event); } } // namespace atom diff --git a/browser/api/atom_browser_bindings.h b/browser/api/atom_browser_bindings.h index 92ddd4990a6..27b9195cdee 100644 --- a/browser/api/atom_browser_bindings.h +++ b/browser/api/atom_browser_bindings.h @@ -12,6 +12,11 @@ namespace base { class ListValue; } +namespace IPC { +class Message; +class Sender; +} + namespace atom { class AtomBrowserBindings : public AtomBindings { @@ -33,7 +38,8 @@ class AtomBrowserBindings : public AtomBindings { int routing_id, const string16& channel, const base::ListValue& args, - string16* result); + IPC::Sender* sender, + IPC::Message* message); // The require('atom').browserMainParts object. v8::Handle browser_main_parts() { diff --git a/browser/api/lib/ipc.coffee b/browser/api/lib/ipc.coffee index 8e81e0d5fce..27deb700fb5 100644 --- a/browser/api/lib/ipc.coffee +++ b/browser/api/lib/ipc.coffee @@ -15,9 +15,12 @@ class Ipc extends EventEmitter process.on 'ATOM_INTERNAL_MESSAGE', (args...) => @emit(args...) process.on 'ATOM_INTERNAL_MESSAGE_SYNC', (channel, event, args...) => - returnValue = 'null' + returnValue = null get = -> returnValue - set = (value) -> returnValue = JSON.stringify(value) + set = (value) -> + throw new Error('returnValue can be only set once') if returnValue? + returnValue = JSON.stringify(value) + event.sendReply() Object.defineProperty event, 'returnValue', {get, set} Object.defineProperty event, 'result', {get, set} diff --git a/browser/native_window.cc b/browser/native_window.cc index 9a7ba7d68fc..f6028195d30 100644 --- a/browser/native_window.cc +++ b/browser/native_window.cc @@ -353,16 +353,13 @@ void NativeWindow::OnRendererMessage(const string16& channel, void NativeWindow::OnRendererMessageSync(const string16& channel, const base::ListValue& args, IPC::Message* reply_msg) { - string16 json; AtomBrowserMainParts::Get()->atom_bindings()->OnRendererMessageSync( GetWebContents()->GetRenderProcessHost()->GetID(), GetWebContents()->GetRoutingID(), channel, args, - &json); - - AtomViewHostMsg_Message_Sync::WriteReplyParams(reply_msg, json); - Send(reply_msg); + this, + reply_msg); } } // namespace atom From 1e4762ce925f687b52b09101b016aa77f739737f Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 22 Sep 2013 10:03:47 +0800 Subject: [PATCH 07/34] Do not store the event.returnValue. --- browser/api/atom_api_event.cc | 14 ++++---------- browser/api/atom_api_event.h | 3 --- browser/api/lib/ipc.coffee | 11 +++-------- 3 files changed, 7 insertions(+), 21 deletions(-) diff --git a/browser/api/atom_api_event.cc b/browser/api/atom_api_event.cc index d9b523bd903..5e66fa705b8 100644 --- a/browser/api/atom_api_event.cc +++ b/browser/api/atom_api_event.cc @@ -46,14 +46,6 @@ v8::Handle Event::CreateV8Object() { return scope.Close(v8_event); } -// static -string16 Event::GetReturnValue(v8::Handle event) { - v8::HandleScope scope; - v8::Local json = event->Get(v8::String::New("returnValue")); - return V8ValueToUTF16(json); -} - - void Event::SetSenderAndMessage(IPC::Sender* sender, IPC::Message* message) { DCHECK(!sender_); DCHECK(!message_); @@ -87,13 +79,15 @@ v8::Handle Event::SendReply(const v8::Arguments& args) { return node::ThrowError("Event is already destroyed"); if (event->sender_ == NULL) - return node::ThrowError("Can only send reply to synchronous events"); + return node::ThrowError("Can only send reply to synchronous events once"); - string16 json = GetReturnValue(args.This()); + string16 json = V8ValueToUTF16(args[0]); AtomViewHostMsg_Message_Sync::WriteReplyParams(event->message_, json); event->sender_->Send(event->message_); + event->sender_ = NULL; + event->message_ = NULL; return v8::Undefined(); } diff --git a/browser/api/atom_api_event.h b/browser/api/atom_api_event.h index ebbec7b97ca..4fd5266b7fe 100644 --- a/browser/api/atom_api_event.h +++ b/browser/api/atom_api_event.h @@ -25,9 +25,6 @@ class Event : public node::ObjectWrap { // Create a V8 Event object. static v8::Handle CreateV8Object(); - // Get JSON string of the event.returnValue from a Event object. - static string16 GetReturnValue(v8::Handle event); - // Pass the sender and message to be replied. void SetSenderAndMessage(IPC::Sender* sender, IPC::Message* message); diff --git a/browser/api/lib/ipc.coffee b/browser/api/lib/ipc.coffee index 27deb700fb5..970709ebc90 100644 --- a/browser/api/lib/ipc.coffee +++ b/browser/api/lib/ipc.coffee @@ -15,15 +15,10 @@ class Ipc extends EventEmitter process.on 'ATOM_INTERNAL_MESSAGE', (args...) => @emit(args...) process.on 'ATOM_INTERNAL_MESSAGE_SYNC', (channel, event, args...) => - returnValue = null - get = -> returnValue - set = (value) -> - throw new Error('returnValue can be only set once') if returnValue? - returnValue = JSON.stringify(value) - event.sendReply() + set = (value) -> event.sendReply JSON.stringify(value) - Object.defineProperty event, 'returnValue', {get, set} - Object.defineProperty event, 'result', {get, set} + Object.defineProperty event, 'returnValue', {set} + Object.defineProperty event, 'result', {set} @emit(channel, event, args...) From 761b9d22c85744a2e809d9908eb2e28d7fd3d8ac Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 22 Sep 2013 10:43:54 +0800 Subject: [PATCH 08/34] Do not reply sync messages when window is closed. --- browser/api/atom_api_event.cc | 16 ++++++++++++---- browser/api/atom_api_event.h | 15 +++++++++++---- browser/api/atom_browser_bindings.cc | 2 +- browser/api/atom_browser_bindings.h | 5 +++-- 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/browser/api/atom_api_event.cc b/browser/api/atom_api_event.cc index 5e66fa705b8..5a6ff2828bc 100644 --- a/browser/api/atom_api_event.cc +++ b/browser/api/atom_api_event.cc @@ -4,9 +4,9 @@ #include "browser/api/atom_api_event.h" +#include "browser/native_window.h" #include "common/api/api_messages.h" #include "common/string16_conversions.h" -#include "ipc/ipc_sender.h" using node::node_isolate; @@ -23,6 +23,8 @@ Event::Event() } Event::~Event() { + if (sender_) + sender_->RemoveObserver(this); } // static @@ -46,11 +48,18 @@ v8::Handle Event::CreateV8Object() { return scope.Close(v8_event); } -void Event::SetSenderAndMessage(IPC::Sender* sender, IPC::Message* message) { +void Event::SetSenderAndMessage(NativeWindow* sender, IPC::Message* message) { DCHECK(!sender_); DCHECK(!message_); sender_ = sender; message_ = message; + + sender_->AddObserver(this); +} + +void Event::OnWindowClosed() { + sender_ = NULL; + message_ = NULL; } // static @@ -78,7 +87,7 @@ v8::Handle Event::SendReply(const v8::Arguments& args) { if (event == NULL) return node::ThrowError("Event is already destroyed"); - if (event->sender_ == NULL) + if (event->message_ == NULL) return node::ThrowError("Can only send reply to synchronous events once"); string16 json = V8ValueToUTF16(args[0]); @@ -86,7 +95,6 @@ v8::Handle Event::SendReply(const v8::Arguments& args) { AtomViewHostMsg_Message_Sync::WriteReplyParams(event->message_, json); event->sender_->Send(event->message_); - event->sender_ = NULL; event->message_ = NULL; return v8::Undefined(); } diff --git a/browser/api/atom_api_event.h b/browser/api/atom_api_event.h index 4fd5266b7fe..7007892b349 100644 --- a/browser/api/atom_api_event.h +++ b/browser/api/atom_api_event.h @@ -6,19 +6,23 @@ #define ATOM_BROWSER_ATOM_API_EVENT_H_ #include "base/basictypes.h" +#include "base/compiler_specific.h" #include "base/string16.h" +#include "browser/native_window_observer.h" #include "vendor/node/src/node_object_wrap.h" namespace IPC { class Message; -class Sender; } namespace atom { +class NativeWindow; + namespace api { -class Event : public node::ObjectWrap { +class Event : public node::ObjectWrap, + public NativeWindowObserver { public: virtual ~Event(); @@ -26,7 +30,7 @@ class Event : public node::ObjectWrap { static v8::Handle CreateV8Object(); // Pass the sender and message to be replied. - void SetSenderAndMessage(IPC::Sender* sender, IPC::Message* message); + void SetSenderAndMessage(NativeWindow* sender, IPC::Message* message); // Accessor to return handle_, this follows Google C++ Style. v8::Persistent& handle() { return handle_; } @@ -37,6 +41,9 @@ class Event : public node::ObjectWrap { protected: Event(); + // NativeWindowObserver implementations: + virtual void OnWindowClosed() OVERRIDE; + private: static v8::Handle New(const v8::Arguments& args); @@ -46,7 +53,7 @@ class Event : public node::ObjectWrap { static v8::Persistent constructor_template_; // Replyer for the synchronous messages. - IPC::Sender* sender_; + NativeWindow* sender_; IPC::Message* message_; bool prevent_default_; diff --git a/browser/api/atom_browser_bindings.cc b/browser/api/atom_browser_bindings.cc index 85ba15cced6..44df9a6c44d 100644 --- a/browser/api/atom_browser_bindings.cc +++ b/browser/api/atom_browser_bindings.cc @@ -76,7 +76,7 @@ void AtomBrowserBindings::OnRendererMessageSync( int routing_id, const string16& channel, const base::ListValue& args, - IPC::Sender* sender, + NativeWindow* sender, IPC::Message* message) { v8::HandleScope scope; diff --git a/browser/api/atom_browser_bindings.h b/browser/api/atom_browser_bindings.h index 27b9195cdee..dde6997a8c7 100644 --- a/browser/api/atom_browser_bindings.h +++ b/browser/api/atom_browser_bindings.h @@ -14,11 +14,12 @@ class ListValue; namespace IPC { class Message; -class Sender; } namespace atom { +class NativeWindow; + class AtomBrowserBindings : public AtomBindings { public: AtomBrowserBindings(); @@ -38,7 +39,7 @@ class AtomBrowserBindings : public AtomBindings { int routing_id, const string16& channel, const base::ListValue& args, - IPC::Sender* sender, + NativeWindow* sender, IPC::Message* message); // The require('atom').browserMainParts object. From 68bdad9a23ff425714505b24e00281bb0a4329ef Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 22 Sep 2013 12:06:41 +0800 Subject: [PATCH 09/34] Add spec for ipc.sendSync. --- spec/api/ipc.coffee | 7 ++++++- spec/main.js | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/spec/api/ipc.coffee b/spec/api/ipc.coffee index 4a50b07407c..c50635f9888 100644 --- a/spec/api/ipc.coffee +++ b/spec/api/ipc.coffee @@ -49,7 +49,12 @@ describe 'ipc', -> describe 'ipc.send', -> it 'should work when sending an object containing id property', (done) -> obj = id: 1, name: 'ly' - ipc.on 'message', (message) -> + ipc.once 'message', (message) -> assert.deepEqual message, obj done() ipc.send obj + + describe 'ipc.sendSync', -> + it 'can be replied by setting event.returnValue', -> + msg = ipc.sendChannelSync 'echo', 'test' + assert.equal msg, 'test' diff --git a/spec/main.js b/spec/main.js index ac211ff5a23..cf2453c5a11 100644 --- a/spec/main.js +++ b/spec/main.js @@ -26,6 +26,10 @@ ipc.on('eval', function(ev, pid, rid, script) { ev.returnValue = eval(script); }); +ipc.on('echo', function(ev, pid, rid, msg) { + ev.returnValue = msg; +}); + process.on('uncaughtException', function() { window.openDevTools(); }); From bfe59480e358ec456d01b65a19356208a15fdf5f Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 22 Sep 2013 17:11:09 +0800 Subject: [PATCH 10/34] Add header for asynchronous version of ShowMessageBox. --- browser/ui/message_box.h | 12 ++++++++++++ browser/ui/message_box_mac.mm | 12 ++++++++++++ browser/ui/message_box_win.cc | 12 ++++++++++++ 3 files changed, 36 insertions(+) diff --git a/browser/ui/message_box.h b/browser/ui/message_box.h index dfe7198ba67..a538eea9b7f 100644 --- a/browser/ui/message_box.h +++ b/browser/ui/message_box.h @@ -8,6 +8,8 @@ #include #include +#include "base/callback_forward.h" + namespace atom { class NativeWindow; @@ -18,6 +20,8 @@ enum MessageBoxType { MESSAGE_BOX_TYPE_WARNING }; +typedef base::Callback MessageBoxCallback; + int ShowMessageBox(NativeWindow* parent_window, MessageBoxType type, const std::vector& buttons, @@ -25,6 +29,14 @@ int ShowMessageBox(NativeWindow* parent_window, const std::string& message, const std::string& detail); +void ShowMessageBox(NativeWindow* parent_window, + MessageBoxType type, + const std::vector& buttons, + const std::string& title, + const std::string& message, + const std::string& detail, + const MessageBoxCallback& callback); + } // namespace atom #endif // BROWSER_UI_MESSAGE_BOX_H_ diff --git a/browser/ui/message_box_mac.mm b/browser/ui/message_box_mac.mm index c6fb14555b0..8da5a4002fe 100644 --- a/browser/ui/message_box_mac.mm +++ b/browser/ui/message_box_mac.mm @@ -6,6 +6,7 @@ #import +#include "base/callback.h" #include "base/strings/sys_string_conversions.h" #include "browser/native_window.h" #include "browser/ui/nsalert_synchronous_sheet_mac.h" @@ -46,4 +47,15 @@ int ShowMessageBox(NativeWindow* parent_window, return [alert runModal]; } +void ShowMessageBox(NativeWindow* parent_window, + MessageBoxType type, + const std::vector& buttons, + const std::string& title, + const std::string& message, + const std::string& detail, + const MessageBoxCallback& callback) { + callback.Run(ShowMessageBox( + parent_window, type, buttons, title, message, detail)); +} + } // namespace atom diff --git a/browser/ui/message_box_win.cc b/browser/ui/message_box_win.cc index 52181e40780..99da31fabd8 100644 --- a/browser/ui/message_box_win.cc +++ b/browser/ui/message_box_win.cc @@ -4,6 +4,7 @@ #include "browser/ui/message_box.h" +#include "base/callback.h" #include "base/message_loop.h" #include "base/run_loop.h" #include "base/string_util.h" @@ -253,4 +254,15 @@ int ShowMessageBox(NativeWindow* parent_window, } } +void ShowMessageBox(NativeWindow* parent_window, + MessageBoxType type, + const std::vector& buttons, + const std::string& title, + const std::string& message, + const std::string& detail, + const MessageBoxCallback& callback) { + callback.Run(ShowMessageBox( + parent_window, type, buttons, title, message, detail)); +} + } // namespace atom From b70722feb6a16fff32c7e63d0a09d62c61121202 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 22 Sep 2013 18:47:00 +0800 Subject: [PATCH 11/34] mac: Implement async ShowMessageBox. --- browser/ui/message_box_mac.mm | 66 ++++++++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/browser/ui/message_box_mac.mm b/browser/ui/message_box_mac.mm index 8da5a4002fe..fb5e6cb6b48 100644 --- a/browser/ui/message_box_mac.mm +++ b/browser/ui/message_box_mac.mm @@ -11,16 +11,48 @@ #include "browser/native_window.h" #include "browser/ui/nsalert_synchronous_sheet_mac.h" +@interface ModalDelegate : NSObject { + @private + atom::MessageBoxCallback callback_; + NSAlert* alert_; +} +- (id)initWithCallback:(const atom::MessageBoxCallback&)callback + andAlert:(NSAlert*)alert; +@end + +@implementation ModalDelegate + +- (id)initWithCallback:(const atom::MessageBoxCallback&)callback + andAlert:(NSAlert*)alert { + if ((self = [super init])) { + callback_ = callback; + alert_ = alert; + } + return self; +} + +- (void)alertDidEnd:(NSAlert*)alert + returnCode:(NSInteger)returnCode + contextInfo:(void*)contextInfo { + callback_.Run(returnCode); + [alert_ release]; + [self release]; +} + +@end + namespace atom { -int ShowMessageBox(NativeWindow* parent_window, - MessageBoxType type, - const std::vector& buttons, - const std::string& title, - const std::string& message, - const std::string& detail) { +namespace { + +NSAlert* CreateNSAlert(NativeWindow* parent_window, + MessageBoxType type, + const std::vector& buttons, + const std::string& title, + const std::string& message, + const std::string& detail) { // Ignore the title; it's the window title on other platforms and ignorable. - NSAlert* alert = [[[NSAlert alloc] init] autorelease]; + NSAlert* alert = [[NSAlert alloc] init]; [alert setMessageText:base::SysUTF8ToNSString(message)]; [alert setInformativeText:base::SysUTF8ToNSString(detail)]; @@ -41,6 +73,21 @@ int ShowMessageBox(NativeWindow* parent_window, [button setTag:i]; } + return alert; +} + +} // namespace + +int ShowMessageBox(NativeWindow* parent_window, + MessageBoxType type, + const std::vector& buttons, + const std::string& title, + const std::string& message, + const std::string& detail) { + NSAlert* alert = CreateNSAlert( + parent_window, type, buttons, title, message, detail); + [alert autorelease]; + if (parent_window) return [alert runModalSheetForWindow:parent_window->GetNativeWindow()]; else @@ -54,8 +101,9 @@ void ShowMessageBox(NativeWindow* parent_window, const std::string& message, const std::string& detail, const MessageBoxCallback& callback) { - callback.Run(ShowMessageBox( - parent_window, type, buttons, title, message, detail)); + NSAlert* alert = CreateNSAlert( + parent_window, type, buttons, title, message, detail); + [[ModalDelegate alloc] initWithCallback:callback andAlert:alert]; } } // namespace atom From 85d65886615ae4f5624b62c650a610ab78565125 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 14:29:55 +0800 Subject: [PATCH 12/34] Make dialog.showMessageBox asynchronous. --- browser/api/atom_api_dialog.cc | 49 +++++++++++++++++++++++++++++----- browser/api/lib/dialog.coffee | 7 +++-- browser/ui/message_box_mac.mm | 9 ++++++- 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/browser/api/atom_api_dialog.cc b/browser/api/atom_api_dialog.cc index 69b0135b294..298cba49ca0 100644 --- a/browser/api/atom_api_dialog.cc +++ b/browser/api/atom_api_dialog.cc @@ -6,12 +6,16 @@ #include +#include "base/bind.h" #include "base/utf_string_conversions.h" #include "base/values.h" #include "browser/api/atom_api_window.h" #include "browser/native_window.h" #include "browser/ui/file_dialog.h" #include "browser/ui/message_box.h" +#include "vendor/node/src/node_internals.h" + +using node::node_isolate; namespace atom { @@ -24,11 +28,25 @@ base::FilePath V8ValueToFilePath(v8::Handle path) { return base::FilePath::FromUTF8Unsafe(path_string); } -v8::Handle FilePathToV8Value(const base::FilePath path) { +v8::Handle ToV8Value(const base::FilePath& path) { std::string path_string(path.AsUTF8Unsafe()); return v8::String::New(path_string.data(), path_string.size()); } +v8::Handle ToV8Value(int code) { + return v8::Integer::New(code); +} + +template +void CallV8Function(v8::Persistent callback, T arg) { + DCHECK(!callback.IsEmpty()); + + v8::HandleScope scope; + v8::Handle value = ToV8Value(arg); + callback->Call(callback, 1, &value); + callback.Dispose(node_isolate); +} + void Initialize(v8::Handle target) { v8::HandleScope scope; @@ -58,6 +76,13 @@ v8::Handle ShowMessageBox(const v8::Arguments &args) { native_window = window->window(); } + v8::Persistent callback; + if (args[6]->IsFunction()) { + callback = v8::Persistent::New( + node_isolate, + v8::Local::Cast(args[6])); + } + MessageBoxType type = (MessageBoxType)(args[0]->IntegerValue()); std::vector buttons; @@ -69,9 +94,21 @@ v8::Handle ShowMessageBox(const v8::Arguments &args) { std::string message(*v8::String::Utf8Value(args[3])); std::string detail(*v8::String::Utf8Value(args[4])); - int chosen = atom::ShowMessageBox( - native_window, type, buttons, title, message, detail); - return scope.Close(v8::Integer::New(chosen)); + if (callback.IsEmpty()) { + int chosen = atom::ShowMessageBox( + native_window, type, buttons, title, message, detail); + return scope.Close(v8::Integer::New(chosen)); + } else { + atom::ShowMessageBox( + native_window, + type, + buttons, + title, + message, + detail, + base::Bind(&CallV8Function, callback)); + return v8::Undefined(); + } } v8::Handle ShowOpenDialog(const v8::Arguments &args) { @@ -92,7 +129,7 @@ v8::Handle ShowOpenDialog(const v8::Arguments &args) { v8::Handle result = v8::Array::New(paths.size()); for (size_t i = 0; i < paths.size(); ++i) - result->Set(i, FilePathToV8Value(paths[i])); + result->Set(i, ToV8Value(paths[i])); return scope.Close(result); } @@ -119,7 +156,7 @@ v8::Handle ShowSaveDialog(const v8::Arguments &args) { &path)) return v8::Undefined(); - return scope.Close(FilePathToV8Value(path)); + return scope.Close(ToV8Value(path)); } } // namespace api diff --git a/browser/api/lib/dialog.coffee b/browser/api/lib/dialog.coffee index 59568bcf5b4..7011de205b3 100644 --- a/browser/api/lib/dialog.coffee +++ b/browser/api/lib/dialog.coffee @@ -30,8 +30,10 @@ module.exports = binding.showSaveDialog window, options.title, options.defaultPath - showMessageBox: (window, options) -> + showMessageBox: (window, options, callback) -> if window? and window.constructor isnt BrowserWindow + # Shift. + callback = options options = window window = null @@ -51,4 +53,5 @@ module.exports = String(options.title), String(options.message), String(options.detail), - window + window, + callback diff --git a/browser/ui/message_box_mac.mm b/browser/ui/message_box_mac.mm index fb5e6cb6b48..b5d0cc4bb58 100644 --- a/browser/ui/message_box_mac.mm +++ b/browser/ui/message_box_mac.mm @@ -103,7 +103,14 @@ void ShowMessageBox(NativeWindow* parent_window, const MessageBoxCallback& callback) { NSAlert* alert = CreateNSAlert( parent_window, type, buttons, title, message, detail); - [[ModalDelegate alloc] initWithCallback:callback andAlert:alert]; + ModalDelegate* delegate = [[ModalDelegate alloc] initWithCallback:callback + andAlert:alert]; + + NSWindow* window = parent_window ? parent_window->GetNativeWindow() : nil; + [alert beginSheetModalForWindow:window + modalDelegate:delegate + didEndSelector:@selector(alertDidEnd:returnCode:contextInfo:) + contextInfo:nil]; } } // namespace atom From a4262bc39d98870f87107c83d33ca1cddbab0127 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 16:27:22 +0800 Subject: [PATCH 13/34] mac: Make ShowOpenDialog able to be shown as sheet. --- browser/api/atom_api_dialog.cc | 6 +++++- browser/ui/file_dialog.h | 5 +++-- browser/ui/file_dialog_mac.mm | 20 ++++++++++++++++++-- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/browser/api/atom_api_dialog.cc b/browser/api/atom_api_dialog.cc index 298cba49ca0..75d31d088fd 100644 --- a/browser/api/atom_api_dialog.cc +++ b/browser/api/atom_api_dialog.cc @@ -124,7 +124,11 @@ v8::Handle ShowOpenDialog(const v8::Arguments &args) { int properties = args[2]->IntegerValue(); std::vector paths; - if (!file_dialog::ShowOpenDialog(title, default_path, properties, &paths)) + if (!file_dialog::ShowOpenDialog(NULL, + title, + default_path, + properties, + &paths)) return v8::Undefined(); v8::Handle result = v8::Array::New(paths.size()); diff --git a/browser/ui/file_dialog.h b/browser/ui/file_dialog.h index 67b526f785b..77e5ac4607c 100644 --- a/browser/ui/file_dialog.h +++ b/browser/ui/file_dialog.h @@ -23,12 +23,13 @@ enum FileDialogProperty { FILE_DIALOG_CREATE_DIRECTORY = 8, }; -bool ShowOpenDialog(const std::string& title, +bool ShowOpenDialog(atom::NativeWindow* parent_window, + const std::string& title, const base::FilePath& default_path, int properties, std::vector* paths); -bool ShowSaveDialog(atom::NativeWindow* window, +bool ShowSaveDialog(atom::NativeWindow* parent_window, const std::string& title, const base::FilePath& default_path, base::FilePath* path); diff --git a/browser/ui/file_dialog_mac.mm b/browser/ui/file_dialog_mac.mm index d51be89a23a..54a901cf2a1 100644 --- a/browser/ui/file_dialog_mac.mm +++ b/browser/ui/file_dialog_mac.mm @@ -43,7 +43,8 @@ void SetupDialog(NSSavePanel* dialog, } // namespace -bool ShowOpenDialog(const std::string& title, +bool ShowOpenDialog(atom::NativeWindow* parent_window, + const std::string& title, const base::FilePath& default_path, int properties, std::vector* paths) { @@ -60,7 +61,22 @@ bool ShowOpenDialog(const std::string& title, if (properties & FILE_DIALOG_MULTI_SELECTIONS) [dialog setAllowsMultipleSelection:YES]; - if ([dialog runModal] == NSFileHandlingPanelCancelButton) + __block int chosen = -1; + + if (parent_window == NULL) { + chosen = [dialog runModal]; + } else { + NSWindow* window = parent_window->GetNativeWindow(); + + [dialog beginSheetModalForWindow:window + completionHandler:^(NSInteger c) { + chosen = c; + [NSApp stopModal]; + }]; + [NSApp runModalForWindow:window]; + } + + if (chosen == NSFileHandlingPanelCancelButton) return false; NSArray* urls = [dialog URLs]; From 76ac8f2719725643939065177130c5df883953cb Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 16:36:33 +0800 Subject: [PATCH 14/34] Enable taking window as parameter in dialog.showOpenDialog. --- browser/api/atom_api_dialog.cc | 11 ++++++++++- browser/api/lib/dialog.coffee | 13 +++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/browser/api/atom_api_dialog.cc b/browser/api/atom_api_dialog.cc index 75d31d088fd..89b83ce9e2c 100644 --- a/browser/api/atom_api_dialog.cc +++ b/browser/api/atom_api_dialog.cc @@ -119,12 +119,21 @@ v8::Handle ShowOpenDialog(const v8::Arguments &args) { !args[2]->IsNumber()) // properties return node::ThrowTypeError("Bad argument"); + NativeWindow* native_window = NULL; + if (args[3]->IsObject()) { + Window* window = Window::Unwrap(args[3]->ToObject()); + if (!window || !window->window()) + return node::ThrowError("Invalid window"); + + native_window = window->window(); + } + std::string title(*v8::String::Utf8Value(args[0])); base::FilePath default_path(V8ValueToFilePath(args[1])); int properties = args[2]->IntegerValue(); std::vector paths; - if (!file_dialog::ShowOpenDialog(NULL, + if (!file_dialog::ShowOpenDialog(native_window, title, default_path, properties, diff --git a/browser/api/lib/dialog.coffee b/browser/api/lib/dialog.coffee index 7011de205b3..760d633be0e 100644 --- a/browser/api/lib/dialog.coffee +++ b/browser/api/lib/dialog.coffee @@ -7,7 +7,13 @@ fileDialogProperties = messageBoxTypes = ['none', 'info', 'warning'] module.exports = - showOpenDialog: (options) -> + showOpenDialog: (window, options, callback) -> + if window? and window.constructor isnt BrowserWindow + # Shift. + callback = options + options = window + window = null + options = title: 'Open', properties: ['openFile'] unless options? options.properties = options.properties ? ['openFile'] throw new TypeError('Properties need to be array') unless Array.isArray options.properties @@ -19,7 +25,10 @@ module.exports = options.title = options.title ? '' options.defaultPath = options.defaultPath ? '' - binding.showOpenDialog options.title, options.defaultPath, properties + binding.showOpenDialog options.title, + options.defaultPath, + properties, + window showSaveDialog: (window, options) -> throw new TypeError('Invalid window') unless window?.constructor is BrowserWindow From f444e9dc7422e0121463aa46af95aa30643bd8c9 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 16:51:00 +0800 Subject: [PATCH 15/34] :lipstick: CoffeeScript is cute. --- browser/api/lib/dialog.coffee | 36 ++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/browser/api/lib/dialog.coffee b/browser/api/lib/dialog.coffee index 760d633be0e..69713ab4c8c 100644 --- a/browser/api/lib/dialog.coffee +++ b/browser/api/lib/dialog.coffee @@ -8,54 +8,56 @@ messageBoxTypes = ['none', 'info', 'warning'] module.exports = showOpenDialog: (window, options, callback) -> - if window? and window.constructor isnt BrowserWindow + unless window?.constructor is BrowserWindow # Shift. callback = options options = window window = null - options = title: 'Open', properties: ['openFile'] unless options? - options.properties = options.properties ? ['openFile'] + options ?= title: 'Open', properties: ['openFile'] + options.properties ?= ['openFile'] throw new TypeError('Properties need to be array') unless Array.isArray options.properties properties = 0 for prop, value of fileDialogProperties properties |= value if prop in options.properties - options.title = options.title ? '' - options.defaultPath = options.defaultPath ? '' + options.title ?= '' + options.defaultPath ?= '' - binding.showOpenDialog options.title, - options.defaultPath, + binding.showOpenDialog String(options.title), + String(options.defaultPath), properties, window showSaveDialog: (window, options) -> throw new TypeError('Invalid window') unless window?.constructor is BrowserWindow - options = title: 'Save' unless options? - options.title = options.title ? '' - options.defaultPath = options.defaultPath ? '' + options ?= title: 'Save' + options.title ?= '' + options.defaultPath ?= '' - binding.showSaveDialog window, options.title, options.defaultPath + binding.showSaveDialog window, + String(options.title), + String(options.defaultPath) showMessageBox: (window, options, callback) -> - if window? and window.constructor isnt BrowserWindow + unless window?.constructor is BrowserWindow # Shift. callback = options options = window window = null - options = type: 'none' unless options? - options.type = options.type ? 'none' + options ?= type: 'none' + options.type ?= 'none' options.type = messageBoxTypes.indexOf options.type throw new TypeError('Invalid message box type') unless options.type > -1 throw new TypeError('Buttons need to be array') unless Array.isArray options.buttons - options.title = options.title ? '' - options.message = options.message ? '' - options.detail = options.detail ? '' + options.title ?= '' + options.message ?= '' + options.detail ?= '' binding.showMessageBox options.type, options.buttons, From 7e86ee37f39ceb7493f7edfd5b36c8080a0b6295 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 17:27:32 +0800 Subject: [PATCH 16/34] :lipstick: cpplint. --- browser/ui/message_box.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/ui/message_box.h b/browser/ui/message_box.h index a538eea9b7f..529e46595cc 100644 --- a/browser/ui/message_box.h +++ b/browser/ui/message_box.h @@ -20,7 +20,7 @@ enum MessageBoxType { MESSAGE_BOX_TYPE_WARNING }; -typedef base::Callback MessageBoxCallback; +typedef base::Callback MessageBoxCallback; int ShowMessageBox(NativeWindow* parent_window, MessageBoxType type, From d3dd2b43327c7d2b6829a488b4c9a690fa492b9a Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 19:22:36 +0800 Subject: [PATCH 17/34] mac: Add asynchronous ShowOpenDialog. --- browser/ui/file_dialog.h | 10 ++++++ browser/ui/file_dialog_mac.mm | 59 +++++++++++++++++++++++++++-------- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/browser/ui/file_dialog.h b/browser/ui/file_dialog.h index 77e5ac4607c..250ba5ce437 100644 --- a/browser/ui/file_dialog.h +++ b/browser/ui/file_dialog.h @@ -8,6 +8,7 @@ #include #include +#include "base/callback_forward.h" #include "base/files/file_path.h" namespace atom { @@ -23,12 +24,21 @@ enum FileDialogProperty { FILE_DIALOG_CREATE_DIRECTORY = 8, }; +typedef base::Callback paths)> OpenDialogCallback; + bool ShowOpenDialog(atom::NativeWindow* parent_window, const std::string& title, const base::FilePath& default_path, int properties, std::vector* paths); +void ShowOpenDialog(atom::NativeWindow* parent_window, + const std::string& title, + const base::FilePath& default_path, + int properties, + const OpenDialogCallback& callback); + bool ShowSaveDialog(atom::NativeWindow* parent_window, const std::string& title, const base::FilePath& default_path, diff --git a/browser/ui/file_dialog_mac.mm b/browser/ui/file_dialog_mac.mm index 54a901cf2a1..539fe9138c5 100644 --- a/browser/ui/file_dialog_mac.mm +++ b/browser/ui/file_dialog_mac.mm @@ -41,6 +41,23 @@ void SetupDialog(NSSavePanel* dialog, [dialog setAllowsOtherFileTypes:YES]; } +void SetupDialogForProperties(NSOpenPanel* dialog, int properties) { + [dialog setCanChooseFiles:(properties & FILE_DIALOG_OPEN_FILE)]; + if (properties & FILE_DIALOG_OPEN_DIRECTORY) + [dialog setCanChooseDirectories:YES]; + if (properties & FILE_DIALOG_CREATE_DIRECTORY) + [dialog setCanCreateDirectories:YES]; + if (properties & FILE_DIALOG_MULTI_SELECTIONS) + [dialog setAllowsMultipleSelection:YES]; +} + +void ReadDialogPaths(NSOpenPanel* dialog, std::vector* paths) { + NSArray* urls = [dialog URLs]; + for (NSURL* url in urls) + if ([url isFileURL]) + paths->push_back(base::FilePath(base::SysNSStringToUTF8([url path]))); +} + } // namespace bool ShowOpenDialog(atom::NativeWindow* parent_window, @@ -52,14 +69,7 @@ bool ShowOpenDialog(atom::NativeWindow* parent_window, NSOpenPanel* dialog = [NSOpenPanel openPanel]; SetupDialog(dialog, title, default_path); - - [dialog setCanChooseFiles:(properties & FILE_DIALOG_OPEN_FILE)]; - if (properties & FILE_DIALOG_OPEN_DIRECTORY) - [dialog setCanChooseDirectories:YES]; - if (properties & FILE_DIALOG_CREATE_DIRECTORY) - [dialog setCanCreateDirectories:YES]; - if (properties & FILE_DIALOG_MULTI_SELECTIONS) - [dialog setAllowsMultipleSelection:YES]; + SetupDialogForProperties(dialog, properties); __block int chosen = -1; @@ -79,14 +89,37 @@ bool ShowOpenDialog(atom::NativeWindow* parent_window, if (chosen == NSFileHandlingPanelCancelButton) return false; - NSArray* urls = [dialog URLs]; - for (NSURL* url in urls) - if ([url isFileURL]) - paths->push_back(base::FilePath(base::SysNSStringToUTF8([url path]))); - + ReadDialogPaths(dialog, paths); return true; } +void ShowOpenDialog(atom::NativeWindow* parent_window, + const std::string& title, + const base::FilePath& default_path, + int properties, + const OpenDialogCallback& c) { + NSOpenPanel* dialog = [NSOpenPanel openPanel]; + + SetupDialog(dialog, title, default_path); + SetupDialogForProperties(dialog, properties); + + // Duplicate the callback object here since c is a reference and gcd would + // only store the pointer, by duplication we can force gcd to store a copy. + __block OpenDialogCallback callback = c; + + NSWindow* window = parent_window ? parent_window->GetNativeWindow() : NULL; + [dialog beginSheetModalForWindow:window + completionHandler:^(NSInteger chosen) { + if (chosen == NSFileHandlingPanelCancelButton) { + callback.Run(false, std::vector()); + } else { + std::vector paths; + ReadDialogPaths(dialog, &paths); + callback.Run(true, paths); + } + }]; +} + bool ShowSaveDialog(atom::NativeWindow* window, const std::string& title, const base::FilePath& default_path, From c95cfc954005e74eccc18b0a3ffbba5abf943422 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 19:23:49 +0800 Subject: [PATCH 18/34] Make dialog.showOpenDialog accept callback. --- browser/api/atom_api_dialog.cc | 63 +++++++++++++++++++++++++++------- browser/api/lib/dialog.coffee | 3 +- 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/browser/api/atom_api_dialog.cc b/browser/api/atom_api_dialog.cc index 89b83ce9e2c..6f4bfdc668d 100644 --- a/browser/api/atom_api_dialog.cc +++ b/browser/api/atom_api_dialog.cc @@ -33,10 +33,21 @@ v8::Handle ToV8Value(const base::FilePath& path) { return v8::String::New(path_string.data(), path_string.size()); } +v8::Handle ToV8Value(void*) { + return v8::Undefined(); +} + v8::Handle ToV8Value(int code) { return v8::Integer::New(code); } +v8::Handle ToV8Value(const std::vector& paths) { + v8::Handle result = v8::Array::New(paths.size()); + for (size_t i = 0; i < paths.size(); ++i) + result->Set(i, ToV8Value(paths[i])); + return result; +} + template void CallV8Function(v8::Persistent callback, T arg) { DCHECK(!callback.IsEmpty()); @@ -47,6 +58,16 @@ void CallV8Function(v8::Persistent callback, T arg) { callback.Dispose(node_isolate); } +template +void CallV8Function2(v8::Persistent callback, + bool result, + T arg) { + if (result) + return CallV8Function(callback, arg); + else + return CallV8Function(callback, NULL); +} + void Initialize(v8::Handle target) { v8::HandleScope scope; @@ -128,23 +149,41 @@ v8::Handle ShowOpenDialog(const v8::Arguments &args) { native_window = window->window(); } + v8::Persistent callback; + if (args[4]->IsFunction()) { + callback = v8::Persistent::New( + node_isolate, + v8::Local::Cast(args[4])); + } + std::string title(*v8::String::Utf8Value(args[0])); base::FilePath default_path(V8ValueToFilePath(args[1])); int properties = args[2]->IntegerValue(); - std::vector paths; - if (!file_dialog::ShowOpenDialog(native_window, - title, - default_path, - properties, - &paths)) + if (callback.IsEmpty()) { + std::vector paths; + if (!file_dialog::ShowOpenDialog(native_window, + title, + default_path, + properties, + &paths)) + return v8::Undefined(); + + v8::Handle result = v8::Array::New(paths.size()); + for (size_t i = 0; i < paths.size(); ++i) + result->Set(i, ToV8Value(paths[i])); + + return scope.Close(result); + } else { + file_dialog::ShowOpenDialog( + native_window, + title, + default_path, + properties, + base::Bind(&CallV8Function2>, + callback)); return v8::Undefined(); - - v8::Handle result = v8::Array::New(paths.size()); - for (size_t i = 0; i < paths.size(); ++i) - result->Set(i, ToV8Value(paths[i])); - - return scope.Close(result); + } } v8::Handle ShowSaveDialog(const v8::Arguments &args) { diff --git a/browser/api/lib/dialog.coffee b/browser/api/lib/dialog.coffee index 69713ab4c8c..069858a3627 100644 --- a/browser/api/lib/dialog.coffee +++ b/browser/api/lib/dialog.coffee @@ -28,7 +28,8 @@ module.exports = binding.showOpenDialog String(options.title), String(options.defaultPath), properties, - window + window, + callback showSaveDialog: (window, options) -> throw new TypeError('Invalid window') unless window?.constructor is BrowserWindow From 43b492c64130d50351587df05e234dfa201112dd Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 19:36:52 +0800 Subject: [PATCH 19/34] mac: make ShowSaveDialog accept no parent window. --- browser/ui/file_dialog_mac.mm | 36 +++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/browser/ui/file_dialog_mac.mm b/browser/ui/file_dialog_mac.mm index 539fe9138c5..0770d0e4035 100644 --- a/browser/ui/file_dialog_mac.mm +++ b/browser/ui/file_dialog_mac.mm @@ -120,11 +120,10 @@ void ShowOpenDialog(atom::NativeWindow* parent_window, }]; } -bool ShowSaveDialog(atom::NativeWindow* window, +bool ShowSaveDialog(atom::NativeWindow* parent_window, const std::string& title, const base::FilePath& default_path, base::FilePath* path) { - DCHECK(window); DCHECK(path); NSSavePanel* dialog = [NSSavePanel savePanel]; @@ -132,25 +131,26 @@ bool ShowSaveDialog(atom::NativeWindow* window, [dialog setCanSelectHiddenExtension:YES]; - __block bool result = false; - __block base::FilePath ret_path; - [dialog beginSheetModalForWindow:window->GetNativeWindow() - completionHandler:^(NSInteger chosen) { - if (chosen == NSFileHandlingPanelCancelButton || - ![[dialog URL] isFileURL]) { - result = false; - } else { - result = true; - ret_path = base::FilePath(base::SysNSStringToUTF8([[dialog URL] path])); - } + __block int chosen = -1; - [NSApp stopModal]; - }]; + if (parent_window == NULL) { + chosen = [dialog runModal]; + } else { + NSWindow* window = parent_window->GetNativeWindow(); - [NSApp runModalForWindow:window->GetNativeWindow()]; + [dialog beginSheetModalForWindow:window + completionHandler:^(NSInteger c) { + chosen = c; + [NSApp stopModal]; + }]; + [NSApp runModalForWindow:window]; + } - *path = ret_path; - return result; + if (chosen == NSFileHandlingPanelCancelButton || ![[dialog URL] isFileURL]) + return false; + + *path = base::FilePath(base::SysNSStringToUTF8([[dialog URL] path])); + return true; } } // namespace file_dialog From e824b6c910ff6b603c26ef2afe4b1b449ab43937 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 19:42:07 +0800 Subject: [PATCH 20/34] :lipstick: Pick duplicate code together. --- browser/ui/file_dialog_mac.mm | 54 ++++++++++++++--------------------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/browser/ui/file_dialog_mac.mm b/browser/ui/file_dialog_mac.mm index 0770d0e4035..e4902e4b5f5 100644 --- a/browser/ui/file_dialog_mac.mm +++ b/browser/ui/file_dialog_mac.mm @@ -38,6 +38,7 @@ void SetupDialog(NSSavePanel* dialog, if (default_filename) [dialog setNameFieldStringValue:default_filename]; + [dialog setCanSelectHiddenExtension:YES]; [dialog setAllowsOtherFileTypes:YES]; } @@ -51,6 +52,25 @@ void SetupDialogForProperties(NSOpenPanel* dialog, int properties) { [dialog setAllowsMultipleSelection:YES]; } +// Run modal dialog with parent window and return user's choice. +int RunModalDialog(NSSavePanel* dialog, atom::NativeWindow* parent_window) { + __block int chosen = NSFileHandlingPanelCancelButton; + if (parent_window == NULL) { + chosen = [dialog runModal]; + } else { + NSWindow* window = parent_window->GetNativeWindow(); + + [dialog beginSheetModalForWindow:window + completionHandler:^(NSInteger c) { + chosen = c; + [NSApp stopModal]; + }]; + [NSApp runModalForWindow:window]; + } + + return chosen; +} + void ReadDialogPaths(NSOpenPanel* dialog, std::vector* paths) { NSArray* urls = [dialog URLs]; for (NSURL* url in urls) @@ -71,21 +91,7 @@ bool ShowOpenDialog(atom::NativeWindow* parent_window, SetupDialog(dialog, title, default_path); SetupDialogForProperties(dialog, properties); - __block int chosen = -1; - - if (parent_window == NULL) { - chosen = [dialog runModal]; - } else { - NSWindow* window = parent_window->GetNativeWindow(); - - [dialog beginSheetModalForWindow:window - completionHandler:^(NSInteger c) { - chosen = c; - [NSApp stopModal]; - }]; - [NSApp runModalForWindow:window]; - } - + int chosen = RunModalDialog(dialog, parent_window); if (chosen == NSFileHandlingPanelCancelButton) return false; @@ -129,23 +135,7 @@ bool ShowSaveDialog(atom::NativeWindow* parent_window, SetupDialog(dialog, title, default_path); - [dialog setCanSelectHiddenExtension:YES]; - - __block int chosen = -1; - - if (parent_window == NULL) { - chosen = [dialog runModal]; - } else { - NSWindow* window = parent_window->GetNativeWindow(); - - [dialog beginSheetModalForWindow:window - completionHandler:^(NSInteger c) { - chosen = c; - [NSApp stopModal]; - }]; - [NSApp runModalForWindow:window]; - } - + int chosen = RunModalDialog(dialog, parent_window); if (chosen == NSFileHandlingPanelCancelButton || ![[dialog URL] isFileURL]) return false; From 30ca085fd889267e44e8f46de98c7c8527acb6a0 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 19:59:00 +0800 Subject: [PATCH 21/34] Make dialog.showSaveDialog accept no parent window. --- browser/api/atom_api_dialog.cc | 22 +++++++++++++--------- browser/api/lib/dialog.coffee | 15 ++++++++++----- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/browser/api/atom_api_dialog.cc b/browser/api/atom_api_dialog.cc index 6f4bfdc668d..5200a78a097 100644 --- a/browser/api/atom_api_dialog.cc +++ b/browser/api/atom_api_dialog.cc @@ -189,20 +189,24 @@ v8::Handle ShowOpenDialog(const v8::Arguments &args) { v8::Handle ShowSaveDialog(const v8::Arguments &args) { v8::HandleScope scope; - if (!args[0]->IsObject() || // window - !args[1]->IsString() || // title - !args[2]->IsString()) // default_path + if (!args[0]->IsString() || // title + !args[1]->IsString()) // default_path return node::ThrowTypeError("Bad argument"); - Window* window = Window::Unwrap(args[0]->ToObject()); - if (!window || !window->window()) - return node::ThrowError("Invalid window"); + NativeWindow* native_window = NULL; + if (args[2]->IsObject()) { + Window* window = Window::Unwrap(args[2]->ToObject()); + if (!window || !window->window()) + return node::ThrowError("Invalid window"); - std::string title(*v8::String::Utf8Value(args[1])); - base::FilePath default_path(V8ValueToFilePath(args[2])); + native_window = window->window(); + } + + std::string title(*v8::String::Utf8Value(args[0])); + base::FilePath default_path(V8ValueToFilePath(args[1])); base::FilePath path; - if (!file_dialog::ShowSaveDialog(window->window(), + if (!file_dialog::ShowSaveDialog(native_window, title, default_path, &path)) diff --git a/browser/api/lib/dialog.coffee b/browser/api/lib/dialog.coffee index 069858a3627..7cf538b2f41 100644 --- a/browser/api/lib/dialog.coffee +++ b/browser/api/lib/dialog.coffee @@ -31,16 +31,21 @@ module.exports = window, callback - showSaveDialog: (window, options) -> - throw new TypeError('Invalid window') unless window?.constructor is BrowserWindow + showSaveDialog: (window, options, callback) -> + unless window?.constructor is BrowserWindow + # Shift. + callback = options + options = window + window = null options ?= title: 'Save' options.title ?= '' options.defaultPath ?= '' - binding.showSaveDialog window, - String(options.title), - String(options.defaultPath) + binding.showSaveDialog String(options.title), + String(options.defaultPath), + window, + callback showMessageBox: (window, options, callback) -> unless window?.constructor is BrowserWindow From c7637c78d16a16f2a0b8e9573ea09ef501f354c1 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 20:08:32 +0800 Subject: [PATCH 22/34] mac: Add asynchronous ShowSaveDialog. --- browser/ui/file_dialog.h | 10 +++++++++- browser/ui/file_dialog_mac.mm | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/browser/ui/file_dialog.h b/browser/ui/file_dialog.h index 250ba5ce437..96c0061cdb9 100644 --- a/browser/ui/file_dialog.h +++ b/browser/ui/file_dialog.h @@ -25,7 +25,10 @@ enum FileDialogProperty { }; typedef base::Callback paths)> OpenDialogCallback; + bool result, const std::vector& paths)> OpenDialogCallback; + +typedef base::Callback SaveDialogCallback; bool ShowOpenDialog(atom::NativeWindow* parent_window, const std::string& title, @@ -44,6 +47,11 @@ bool ShowSaveDialog(atom::NativeWindow* parent_window, const base::FilePath& default_path, base::FilePath* path); +void ShowSaveDialog(atom::NativeWindow* parent_window, + const std::string& title, + const base::FilePath& default_path, + const SaveDialogCallback& callback); + } // namespace file_dialog #endif // BROWSER_UI_FILE_DIALOG_H_ diff --git a/browser/ui/file_dialog_mac.mm b/browser/ui/file_dialog_mac.mm index e4902e4b5f5..fd28cf55d6f 100644 --- a/browser/ui/file_dialog_mac.mm +++ b/browser/ui/file_dialog_mac.mm @@ -143,4 +143,26 @@ bool ShowSaveDialog(atom::NativeWindow* parent_window, return true; } +void ShowSaveDialog(atom::NativeWindow* parent_window, + const std::string& title, + const base::FilePath& default_path, + const SaveDialogCallback& c) { + NSSavePanel* dialog = [NSSavePanel savePanel]; + + SetupDialog(dialog, title, default_path); + + __block SaveDialogCallback callback = c; + + NSWindow* window = parent_window ? parent_window->GetNativeWindow() : NULL; + [dialog beginSheetModalForWindow:window + completionHandler:^(NSInteger chosen) { + if (chosen == NSFileHandlingPanelCancelButton) { + callback.Run(false, base::FilePath()); + } else { + std::string path = base::SysNSStringToUTF8([[dialog URL] path]); + callback.Run(true, base::FilePath(path)); + } + }]; +} + } // namespace file_dialog From 26f0e49c9aa4f5055bbea6abef82dc44e9df9c7d Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 20:08:49 +0800 Subject: [PATCH 23/34] Make dialog.showSaveDialog accept a callback. --- browser/api/atom_api_dialog.cc | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/browser/api/atom_api_dialog.cc b/browser/api/atom_api_dialog.cc index 5200a78a097..f7da08fae70 100644 --- a/browser/api/atom_api_dialog.cc +++ b/browser/api/atom_api_dialog.cc @@ -180,7 +180,7 @@ v8::Handle ShowOpenDialog(const v8::Arguments &args) { title, default_path, properties, - base::Bind(&CallV8Function2>, + base::Bind(&CallV8Function2&>, callback)); return v8::Undefined(); } @@ -202,17 +202,33 @@ v8::Handle ShowSaveDialog(const v8::Arguments &args) { native_window = window->window(); } + v8::Persistent callback; + if (args[3]->IsFunction()) { + callback = v8::Persistent::New( + node_isolate, + v8::Local::Cast(args[3])); + } + std::string title(*v8::String::Utf8Value(args[0])); base::FilePath default_path(V8ValueToFilePath(args[1])); - base::FilePath path; - if (!file_dialog::ShowSaveDialog(native_window, - title, - default_path, - &path)) - return v8::Undefined(); + if (callback.IsEmpty()) { + base::FilePath path; + if (!file_dialog::ShowSaveDialog(native_window, + title, + default_path, + &path)) + return v8::Undefined(); - return scope.Close(ToV8Value(path)); + return scope.Close(ToV8Value(path)); + } else { + file_dialog::ShowSaveDialog( + native_window, + title, + default_path, + base::Bind(&CallV8Function2, callback)); + return v8::Undefined(); + } } } // namespace api From 770a0068a3defc6ce497fdb648bc282905fe998d Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 21:12:40 +0800 Subject: [PATCH 24/34] Simplify conversions between native types and v8 types. --- browser/api/atom_api_dialog.cc | 110 +++++++++++++++------------------ 1 file changed, 51 insertions(+), 59 deletions(-) diff --git a/browser/api/atom_api_dialog.cc b/browser/api/atom_api_dialog.cc index f7da08fae70..9845658832e 100644 --- a/browser/api/atom_api_dialog.cc +++ b/browser/api/atom_api_dialog.cc @@ -23,17 +23,48 @@ namespace api { namespace { -base::FilePath V8ValueToFilePath(v8::Handle path) { - std::string path_string(*v8::String::Utf8Value(path)); - return base::FilePath::FromUTF8Unsafe(path_string); -} +// Trick to overload functions by return value's type. +struct FromV8Value { + explicit FromV8Value(v8::Handle value) : value_(value) {} + + operator std::string() { + return *v8::String::Utf8Value(value_); + } + + operator int() { + return value_->IntegerValue(); + } + + operator base::FilePath() { + return base::FilePath::FromUTF8Unsafe(FromV8Value(value_)); + } + + operator atom::NativeWindow*() { + if (value_->IsObject()) { + Window* window = Window::Unwrap(value_->ToObject()); + if (window && window->window()) + return window->window(); + } + return NULL; + } + + operator v8::Persistent() { + return value_->IsFunction() ? + v8::Persistent::New( + node_isolate, + v8::Handle::Cast(value_)) : + v8::Persistent(); + } + + v8::Handle value_; +}; v8::Handle ToV8Value(const base::FilePath& path) { std::string path_string(path.AsUTF8Unsafe()); return v8::String::New(path_string.data(), path_string.size()); } -v8::Handle ToV8Value(void*) { +v8::Handle ToV8Value(void* whatever) { return v8::Undefined(); } @@ -88,32 +119,19 @@ v8::Handle ShowMessageBox(const v8::Arguments &args) { !args[4]->IsString()) // detail return node::ThrowTypeError("Bad argument"); - NativeWindow* native_window = NULL; - if (args[5]->IsObject()) { - Window* window = Window::Unwrap(args[5]->ToObject()); - if (!window || !window->window()) - return node::ThrowError("Invalid window"); - - native_window = window->window(); - } - - v8::Persistent callback; - if (args[6]->IsFunction()) { - callback = v8::Persistent::New( - node_isolate, - v8::Local::Cast(args[6])); - } + NativeWindow* native_window = FromV8Value(args[5]); + v8::Persistent callback = FromV8Value(args[6]); MessageBoxType type = (MessageBoxType)(args[0]->IntegerValue()); std::vector buttons; v8::Handle v8_buttons = v8::Handle::Cast(args[1]); for (uint32_t i = 0; i < v8_buttons->Length(); ++i) - buttons.push_back(*v8::String::Utf8Value(v8_buttons->Get(i))); + buttons.push_back(FromV8Value(v8_buttons->Get(i))); - std::string title(*v8::String::Utf8Value(args[2])); - std::string message(*v8::String::Utf8Value(args[3])); - std::string detail(*v8::String::Utf8Value(args[4])); + std::string title = FromV8Value(args[2]); + std::string message = FromV8Value(args[3]); + std::string detail = FromV8Value(args[4]); if (callback.IsEmpty()) { int chosen = atom::ShowMessageBox( @@ -140,25 +158,12 @@ v8::Handle ShowOpenDialog(const v8::Arguments &args) { !args[2]->IsNumber()) // properties return node::ThrowTypeError("Bad argument"); - NativeWindow* native_window = NULL; - if (args[3]->IsObject()) { - Window* window = Window::Unwrap(args[3]->ToObject()); - if (!window || !window->window()) - return node::ThrowError("Invalid window"); + NativeWindow* native_window = FromV8Value(args[3]); + v8::Persistent callback = FromV8Value(args[4]); - native_window = window->window(); - } - - v8::Persistent callback; - if (args[4]->IsFunction()) { - callback = v8::Persistent::New( - node_isolate, - v8::Local::Cast(args[4])); - } - - std::string title(*v8::String::Utf8Value(args[0])); - base::FilePath default_path(V8ValueToFilePath(args[1])); - int properties = args[2]->IntegerValue(); + std::string title = FromV8Value(args[0]); + base::FilePath default_path = FromV8Value(args[1]); + int properties = FromV8Value(args[2]); if (callback.IsEmpty()) { std::vector paths; @@ -193,24 +198,11 @@ v8::Handle ShowSaveDialog(const v8::Arguments &args) { !args[1]->IsString()) // default_path return node::ThrowTypeError("Bad argument"); - NativeWindow* native_window = NULL; - if (args[2]->IsObject()) { - Window* window = Window::Unwrap(args[2]->ToObject()); - if (!window || !window->window()) - return node::ThrowError("Invalid window"); + NativeWindow* native_window = FromV8Value(args[2]); + v8::Persistent callback = FromV8Value(args[3]); - native_window = window->window(); - } - - v8::Persistent callback; - if (args[3]->IsFunction()) { - callback = v8::Persistent::New( - node_isolate, - v8::Local::Cast(args[3])); - } - - std::string title(*v8::String::Utf8Value(args[0])); - base::FilePath default_path(V8ValueToFilePath(args[1])); + std::string title = FromV8Value(args[0]); + base::FilePath default_path = FromV8Value(args[1]); if (callback.IsEmpty()) { base::FilePath path; From a0d1a7620c397b5d56d626d99ead96c3c90cb61f Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 21:30:54 +0800 Subject: [PATCH 25/34] Put FromV8Value and ToV8Value to a new header. --- atom.gyp | 1 + browser/api/atom_api_dialog.cc | 95 ++++++---------------------------- common/v8_conversions.h | 82 +++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 79 deletions(-) create mode 100644 common/v8_conversions.h diff --git a/atom.gyp b/atom.gyp index a5bfac70d98..bd2ce11b234 100644 --- a/atom.gyp +++ b/atom.gyp @@ -158,6 +158,7 @@ 'common/platform_util_mac.mm', 'common/platform_util_win.cc', 'common/string16_conversions.h', + 'common/v8_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_dialog.cc b/browser/api/atom_api_dialog.cc index 9845658832e..c94fd590d99 100644 --- a/browser/api/atom_api_dialog.cc +++ b/browser/api/atom_api_dialog.cc @@ -4,15 +4,11 @@ #include "browser/api/atom_api_dialog.h" -#include - #include "base/bind.h" -#include "base/utf_string_conversions.h" -#include "base/values.h" -#include "browser/api/atom_api_window.h" #include "browser/native_window.h" #include "browser/ui/file_dialog.h" #include "browser/ui/message_box.h" +#include "common/v8_conversions.h" #include "vendor/node/src/node_internals.h" using node::node_isolate; @@ -23,62 +19,6 @@ namespace api { namespace { -// Trick to overload functions by return value's type. -struct FromV8Value { - explicit FromV8Value(v8::Handle value) : value_(value) {} - - operator std::string() { - return *v8::String::Utf8Value(value_); - } - - operator int() { - return value_->IntegerValue(); - } - - operator base::FilePath() { - return base::FilePath::FromUTF8Unsafe(FromV8Value(value_)); - } - - operator atom::NativeWindow*() { - if (value_->IsObject()) { - Window* window = Window::Unwrap(value_->ToObject()); - if (window && window->window()) - return window->window(); - } - return NULL; - } - - operator v8::Persistent() { - return value_->IsFunction() ? - v8::Persistent::New( - node_isolate, - v8::Handle::Cast(value_)) : - v8::Persistent(); - } - - v8::Handle value_; -}; - -v8::Handle ToV8Value(const base::FilePath& path) { - std::string path_string(path.AsUTF8Unsafe()); - return v8::String::New(path_string.data(), path_string.size()); -} - -v8::Handle ToV8Value(void* whatever) { - return v8::Undefined(); -} - -v8::Handle ToV8Value(int code) { - return v8::Integer::New(code); -} - -v8::Handle ToV8Value(const std::vector& paths) { - v8::Handle result = v8::Array::New(paths.size()); - for (size_t i = 0; i < paths.size(); ++i) - result->Set(i, ToV8Value(paths[i])); - return result; -} - template void CallV8Function(v8::Persistent callback, T arg) { DCHECK(!callback.IsEmpty()); @@ -119,28 +59,27 @@ v8::Handle ShowMessageBox(const v8::Arguments &args) { !args[4]->IsString()) // detail return node::ThrowTypeError("Bad argument"); - NativeWindow* native_window = FromV8Value(args[5]); - v8::Persistent callback = FromV8Value(args[6]); - - MessageBoxType type = (MessageBoxType)(args[0]->IntegerValue()); - - std::vector buttons; - v8::Handle v8_buttons = v8::Handle::Cast(args[1]); - for (uint32_t i = 0; i < v8_buttons->Length(); ++i) - buttons.push_back(FromV8Value(v8_buttons->Get(i))); - + int type = FromV8Value(args[0]); + std::vector buttons = FromV8Value(args[1]); std::string title = FromV8Value(args[2]); std::string message = FromV8Value(args[3]); std::string detail = FromV8Value(args[4]); + NativeWindow* native_window = FromV8Value(args[5]); + v8::Persistent callback = FromV8Value(args[6]); if (callback.IsEmpty()) { int chosen = atom::ShowMessageBox( - native_window, type, buttons, title, message, detail); + native_window, + (MessageBoxType)type, + buttons, + title, + message, + detail); return scope.Close(v8::Integer::New(chosen)); } else { atom::ShowMessageBox( native_window, - type, + (MessageBoxType)type, buttons, title, message, @@ -158,12 +97,11 @@ v8::Handle ShowOpenDialog(const v8::Arguments &args) { !args[2]->IsNumber()) // properties return node::ThrowTypeError("Bad argument"); - NativeWindow* native_window = FromV8Value(args[3]); - v8::Persistent callback = FromV8Value(args[4]); - std::string title = FromV8Value(args[0]); base::FilePath default_path = FromV8Value(args[1]); int properties = FromV8Value(args[2]); + NativeWindow* native_window = FromV8Value(args[3]); + v8::Persistent callback = FromV8Value(args[4]); if (callback.IsEmpty()) { std::vector paths; @@ -198,11 +136,10 @@ v8::Handle ShowSaveDialog(const v8::Arguments &args) { !args[1]->IsString()) // default_path return node::ThrowTypeError("Bad argument"); - NativeWindow* native_window = FromV8Value(args[2]); - v8::Persistent callback = FromV8Value(args[3]); - std::string title = FromV8Value(args[0]); base::FilePath default_path = FromV8Value(args[1]); + NativeWindow* native_window = FromV8Value(args[2]); + v8::Persistent callback = FromV8Value(args[3]); if (callback.IsEmpty()) { base::FilePath path; diff --git a/common/v8_conversions.h b/common/v8_conversions.h new file mode 100644 index 00000000000..35a140c21b4 --- /dev/null +++ b/common/v8_conversions.h @@ -0,0 +1,82 @@ +// 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_V8_CONVERSIONS_H_ +#define COMMON_V8_CONVERSIONS_H_ + +#include +#include + +#include "base/files/file_path.h" +#include "browser/api/atom_api_window.h" +#include "v8/include/v8.h" + +// Trick to overload functions by return value's type. +struct FromV8Value { + explicit FromV8Value(v8::Handle value) : value_(value) {} + + operator std::string() { + return *v8::String::Utf8Value(value_); + } + + operator int() { + return value_->IntegerValue(); + } + + operator base::FilePath() { + return base::FilePath::FromUTF8Unsafe(FromV8Value(value_)); + } + + operator std::vector() { + std::vector array; + v8::Handle v8_array = v8::Handle::Cast(value_); + for (uint32_t i = 0; i < v8_array->Length(); ++i) + array.push_back(FromV8Value(v8_array->Get(i))); + + return array; + } + + operator atom::NativeWindow*() { + using atom::api::Window; + if (value_->IsObject()) { + Window* window = Window::Unwrap(value_->ToObject()); + if (window && window->window()) + return window->window(); + } + return NULL; + } + + operator v8::Persistent() { + return value_->IsFunction() ? + v8::Persistent::New( + node::node_isolate, + v8::Handle::Cast(value_)) : + v8::Persistent(); + } + + v8::Handle value_; +}; + +inline v8::Handle ToV8Value(const base::FilePath& path) { + std::string path_string(path.AsUTF8Unsafe()); + return v8::String::New(path_string.data(), path_string.size()); +} + +inline v8::Handle ToV8Value(void* whatever) { + return v8::Undefined(); +} + +inline v8::Handle ToV8Value(int code) { + return v8::Integer::New(code); +} + +inline +v8::Handle ToV8Value(const std::vector& paths) { + v8::Handle result = v8::Array::New(paths.size()); + for (size_t i = 0; i < paths.size(); ++i) + result->Set(i, ToV8Value(paths[i])); + return result; +} + +#endif // COMMON_V8_CONVERSIONS_H_ From 1e5e0194bd40733a964118803587794460fade34 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 22:48:55 +0800 Subject: [PATCH 26/34] Add convient function for converting args from V8 Arguments. --- common/v8_conversions.h | 117 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 115 insertions(+), 2 deletions(-) diff --git a/common/v8_conversions.h b/common/v8_conversions.h index 35a140c21b4..b483d665915 100644 --- a/common/v8_conversions.h +++ b/common/v8_conversions.h @@ -12,7 +12,7 @@ #include "browser/api/atom_api_window.h" #include "v8/include/v8.h" -// Trick to overload functions by return value's type. +// Convert V8 value to arbitrary supported types. struct FromV8Value { explicit FromV8Value(v8::Handle value) : value_(value) {} @@ -58,6 +58,7 @@ struct FromV8Value { v8::Handle value_; }; +// Convert arbitrary supported native type to V8 value. inline v8::Handle ToV8Value(const base::FilePath& path) { std::string path_string(path.AsUTF8Unsafe()); return v8::String::New(path_string.data(), path_string.size()); @@ -71,7 +72,6 @@ inline v8::Handle ToV8Value(int code) { return v8::Integer::New(code); } -inline v8::Handle ToV8Value(const std::vector& paths) { v8::Handle result = v8::Array::New(paths.size()); for (size_t i = 0; i < paths.size(); ++i) @@ -79,4 +79,117 @@ v8::Handle ToV8Value(const std::vector& paths) { return result; } +// Check if a V8 Value is of specified type. +template inline +bool V8ValueCanBeConvertedTo(v8::Handle value) { + return false; +} + +template<> inline +bool V8ValueCanBeConvertedTo(v8::Handle value) { + return value->IsNumber(); +} + +template<> inline +bool V8ValueCanBeConvertedTo(v8::Handle value) { + return value->IsString(); +} + +template<> inline +bool V8ValueCanBeConvertedTo(v8::Handle value) { + return V8ValueCanBeConvertedTo(value); +} + +template<> inline +bool V8ValueCanBeConvertedTo>( + v8::Handle value) { + return value->IsArray(); +} + +template<> +bool V8ValueCanBeConvertedTo(v8::Handle value) { + using atom::api::Window; + if (value->IsObject()) { + Window* window = Window::Unwrap(value->ToObject()); + if (window && window->window()) + return true; + } + return false; +} + +template<> inline +bool V8ValueCanBeConvertedTo>( + v8::Handle value) { + return value->IsFunction(); +} + +// Check and convert V8's Arguments to native types. +template +bool FromV8Arguments(const v8::Arguments& args, T1* value) { + if (!V8ValueCanBeConvertedTo(args[index])) + return false; + *value = static_cast(FromV8Value(args[index])); + return true; +} + +template +bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2) { + return FromV8Arguments(args, a1) && FromV8Arguments(args, a2); +} + +template +bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2, T3* a3) { + return FromV8Arguments(args, a1, a2) && + FromV8Arguments(args, a3); +} + +template +bool FromV8Arguments(const v8::Arguments& args, + T1* a1, + T2* a2, + T3* a3, + T4* a4) { + return FromV8Arguments(args, a1, a2, a3) && + FromV8Arguments(args, a4); +} + +template +bool FromV8Arguments(const v8::Arguments& args, + T1* a1, + T2* a2, + T3* a3, + T4* a4, + T5* a5) { + return FromV8Arguments(args, a1, a2, a3, a4) && + FromV8Arguments(args, a5); +} + +template +bool FromV8Arguments(const v8::Arguments& args, + T1* a1, + T2* a2, + T3* a3, + T4* a4, + T5* a5, + T6* a6) { + return FromV8Arguments(args, a1, a2, a3, a4, a5) && + FromV8Arguments(args, a6); +} + +template +bool FromV8Arguments(const v8::Arguments& args, + T1* a1, + T2* a2, + T3* a3, + T4* a4, + T5* a5, + T6* a6, + T7* a7) { + return + FromV8Arguments(args, a1, a2, a3, a4, a5, a6) && + FromV8Arguments(args, a7); +} + #endif // COMMON_V8_CONVERSIONS_H_ From 8fdd3b304496995d9ef772ebfa6abde110afe3be Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 22:52:48 +0800 Subject: [PATCH 27/34] Use FromV8Arguments in atom_api_dialog.cc. --- browser/api/atom_api_dialog.cc | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/browser/api/atom_api_dialog.cc b/browser/api/atom_api_dialog.cc index c94fd590d99..e5d52c43472 100644 --- a/browser/api/atom_api_dialog.cc +++ b/browser/api/atom_api_dialog.cc @@ -52,18 +52,12 @@ void Initialize(v8::Handle target) { v8::Handle ShowMessageBox(const v8::Arguments &args) { v8::HandleScope scope; - if (!args[0]->IsNumber() || // type - !args[1]->IsArray() || // buttons - !args[2]->IsString() || // title - !args[3]->IsString() || // message - !args[4]->IsString()) // detail + int type; + std::vector buttons; + std::string title, message, detail; + if (!FromV8Arguments(args, &type, &buttons, &title, &message, &detail)) return node::ThrowTypeError("Bad argument"); - int type = FromV8Value(args[0]); - std::vector buttons = FromV8Value(args[1]); - std::string title = FromV8Value(args[2]); - std::string message = FromV8Value(args[3]); - std::string detail = FromV8Value(args[4]); NativeWindow* native_window = FromV8Value(args[5]); v8::Persistent callback = FromV8Value(args[6]); @@ -92,14 +86,12 @@ v8::Handle ShowMessageBox(const v8::Arguments &args) { v8::Handle ShowOpenDialog(const v8::Arguments &args) { v8::HandleScope scope; - if (!args[0]->IsString() || // title - !args[1]->IsString() || // default_path - !args[2]->IsNumber()) // properties + std::string title; + base::FilePath default_path; + int properties; + if (!FromV8Arguments(args, &title, &default_path, &properties)) return node::ThrowTypeError("Bad argument"); - std::string title = FromV8Value(args[0]); - base::FilePath default_path = FromV8Value(args[1]); - int properties = FromV8Value(args[2]); NativeWindow* native_window = FromV8Value(args[3]); v8::Persistent callback = FromV8Value(args[4]); @@ -132,12 +124,11 @@ v8::Handle ShowOpenDialog(const v8::Arguments &args) { v8::Handle ShowSaveDialog(const v8::Arguments &args) { v8::HandleScope scope; - if (!args[0]->IsString() || // title - !args[1]->IsString()) // default_path + std::string title; + base::FilePath default_path; + if (!FromV8Arguments(args, &title, &default_path)) return node::ThrowTypeError("Bad argument"); - std::string title = FromV8Value(args[0]); - base::FilePath default_path = FromV8Value(args[1]); NativeWindow* native_window = FromV8Value(args[2]); v8::Persistent callback = FromV8Value(args[3]); From a824c88352d13bab7cf55939d9404a6b702b2452 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 23 Sep 2013 23:00:58 +0800 Subject: [PATCH 28/34] Use same name convention in string16_conversions. --- browser/api/atom_api_browser_ipc.cc | 2 +- browser/api/atom_api_event.cc | 2 +- browser/api/atom_api_menu.cc | 40 +++++++++++++------------- browser/api/atom_api_window.cc | 2 +- browser/api/atom_browser_bindings.cc | 4 +-- common/string16_conversions.h | 7 ++--- common/v8_conversions.h | 17 +++++------ renderer/api/atom_api_renderer_ipc.cc | 6 ++-- renderer/api/atom_renderer_bindings.cc | 2 +- 9 files changed, 41 insertions(+), 41 deletions(-) diff --git a/browser/api/atom_api_browser_ipc.cc b/browser/api/atom_api_browser_ipc.cc index d100cb4e0c9..763a3dfba70 100644 --- a/browser/api/atom_api_browser_ipc.cc +++ b/browser/api/atom_api_browser_ipc.cc @@ -26,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"); - string16 channel(V8ValueToUTF16(args[0])); + string16 channel(FromV8Value(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 5a6ff2828bc..7ab14467042 100644 --- a/browser/api/atom_api_event.cc +++ b/browser/api/atom_api_event.cc @@ -90,7 +90,7 @@ v8::Handle Event::SendReply(const v8::Arguments& args) { if (event->message_ == NULL) return node::ThrowError("Can only send reply to synchronous events once"); - string16 json = V8ValueToUTF16(args[0]); + string16 json = FromV8Value(args[0]); AtomViewHostMsg_Message_Sync::WriteReplyParams(event->message_, json); event->sender_->Send(event->message_); diff --git a/browser/api/atom_api_menu.cc b/browser/api/atom_api_menu.cc index df9f97a98af..afb7e157757 100644 --- a/browser/api/atom_api_menu.cc +++ b/browser/api/atom_api_menu.cc @@ -4,9 +4,9 @@ #include "browser/api/atom_api_menu.h" -#include "browser/api/atom_api_window.h" #include "browser/ui/accelerator_util.h" #include "common/string16_conversions.h" +#include "common/v8_conversions.h" #define UNWRAP_MEMNU_AND_CHECK \ Menu* self = ObjectWrap::Unwrap(args.This()); \ @@ -100,18 +100,18 @@ bool Menu::IsItemForCommandIdDynamic(int command_id) const { string16 Menu::GetLabelForCommandId(int command_id) const { v8::HandleScope scope; - return V8ValueToUTF16(CallDelegate(v8::False(), - handle(), - "getLabelForCommandId", - command_id)); + return FromV8Value(CallDelegate(v8::False(), + handle(), + "getLabelForCommandId", + command_id)); } string16 Menu::GetSublabelForCommandId(int command_id) const { v8::HandleScope scope; - return V8ValueToUTF16(CallDelegate(v8::False(), - handle(), - "getSubLabelForCommandId", - command_id)); + return FromV8Value(CallDelegate(v8::False(), + handle(), + "getSubLabelForCommandId", + command_id)); } void Menu::ExecuteCommand(int command_id, int event_flags) { @@ -141,10 +141,10 @@ v8::Handle Menu::InsertItem(const v8::Arguments &args) { int index = args[0]->IntegerValue(); if (index < 0) - self->model_->AddItem(args[1]->IntegerValue(), V8ValueToUTF16(args[2])); + self->model_->AddItem(args[1]->IntegerValue(), FromV8Value(args[2])); else self->model_->InsertItemAt( - index, args[1]->IntegerValue(), V8ValueToUTF16(args[2])); + index, args[1]->IntegerValue(), FromV8Value(args[2])); return v8::Undefined(); } @@ -160,9 +160,9 @@ v8::Handle Menu::InsertCheckItem(const v8::Arguments &args) { int command_id = args[1]->IntegerValue(); if (index < 0) - self->model_->AddCheckItem(command_id, V8ValueToUTF16(args[2])); + self->model_->AddCheckItem(command_id, FromV8Value(args[2])); else - self->model_->InsertCheckItemAt(index, command_id, V8ValueToUTF16(args[2])); + self->model_->InsertCheckItemAt(index, command_id, FromV8Value(args[2])); return v8::Undefined(); } @@ -182,10 +182,10 @@ v8::Handle Menu::InsertRadioItem(const v8::Arguments &args) { int group_id = args[3]->IntegerValue(); if (index < 0) - self->model_->AddRadioItem(command_id, V8ValueToUTF16(args[2]), group_id); + self->model_->AddRadioItem(command_id, FromV8Value(args[2]), group_id); else self->model_->InsertRadioItemAt( - index, command_id, V8ValueToUTF16(args[2]), group_id); + index, command_id, FromV8Value(args[2]), group_id); return v8::Undefined(); } @@ -226,10 +226,10 @@ v8::Handle Menu::InsertSubMenu(const v8::Arguments &args) { if (index < 0) self->model_->AddSubMenu( - command_id, V8ValueToUTF16(args[2]), submenu->model_.get()); + command_id, FromV8Value(args[2]), submenu->model_.get()); else self->model_->InsertSubMenuAt( - index, command_id, V8ValueToUTF16(args[2]), submenu->model_.get()); + index, command_id, FromV8Value(args[2]), submenu->model_.get()); return v8::Undefined(); } @@ -253,7 +253,7 @@ v8::Handle Menu::SetSublabel(const v8::Arguments &args) { if (!args[0]->IsNumber() || !args[1]->IsString()) return node::ThrowTypeError("Bad argument"); - self->model_->SetSublabel(args[0]->IntegerValue(), V8ValueToUTF16(args[1])); + self->model_->SetSublabel(args[0]->IntegerValue(), FromV8Value(args[1])); return v8::Undefined(); } @@ -291,14 +291,14 @@ v8::Handle Menu::GetCommandIdAt(const v8::Arguments &args) { v8::Handle Menu::GetLabelAt(const v8::Arguments &args) { UNWRAP_MEMNU_AND_CHECK; int index = args[0]->IntegerValue(); - return UTF16ToV8Value(self->model_->GetLabelAt(index)); + return ToV8Value(self->model_->GetLabelAt(index)); } // static v8::Handle Menu::GetSublabelAt(const v8::Arguments &args) { UNWRAP_MEMNU_AND_CHECK; int index = args[0]->IntegerValue(); - return UTF16ToV8Value(self->model_->GetSublabelAt(index)); + return ToV8Value(self->model_->GetSublabelAt(index)); } // static diff --git a/browser/api/atom_api_window.cc b/browser/api/atom_api_window.cc index 192c318e679..be582d87794 100644 --- a/browser/api/atom_api_window.cc +++ b/browser/api/atom_api_window.cc @@ -465,7 +465,7 @@ v8::Handle Window::GetPageTitle(const v8::Arguments &args) { string16 title = self->window_->GetWebContents()->GetTitle(); - return UTF16ToV8Value(title); + return ToV8Value(title); } // static diff --git a/browser/api/atom_browser_bindings.cc b/browser/api/atom_browser_bindings.cc index 44df9a6c44d..9c2189460f6 100644 --- a/browser/api/atom_browser_bindings.cc +++ b/browser/api/atom_browser_bindings.cc @@ -55,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(UTF16ToV8Value(channel)); + arguments.push_back(ToV8Value(channel)); const base::Value* value; if (args.Get(0, &value)) arguments.push_back(converter->ToV8Value(value, context)); @@ -91,7 +91,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(UTF16ToV8Value(channel)); + arguments.push_back(ToV8Value(channel)); const base::Value* value; if (args.Get(0, &value)) arguments.push_back(converter->ToV8Value(value, context)); diff --git a/common/string16_conversions.h b/common/string16_conversions.h index 314eb1b50a0..db2d5da0bff 100644 --- a/common/string16_conversions.h +++ b/common/string16_conversions.h @@ -5,18 +5,17 @@ #ifndef COMMON_STRING16_CONVERSIONS_H_ #define COMMON_STRING16_CONVERSIONS_H_ +#include "base/string16.h" #include "v8/include/v8.h" -class string16; - // Converts a V8 value to a string16. -inline string16 V8ValueToUTF16(v8::Handle value) { +inline string16 FromV8Value(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) { +inline v8::Handle ToV8Value(const string16& s) { return v8::String::New(reinterpret_cast(s.data()), s.size()); } diff --git a/common/v8_conversions.h b/common/v8_conversions.h index b483d665915..a82e5978c99 100644 --- a/common/v8_conversions.h +++ b/common/v8_conversions.h @@ -72,6 +72,7 @@ inline v8::Handle ToV8Value(int code) { return v8::Integer::New(code); } +inline v8::Handle ToV8Value(const std::vector& paths) { v8::Handle result = v8::Array::New(paths.size()); for (size_t i = 0; i < paths.size(); ++i) @@ -106,7 +107,7 @@ bool V8ValueCanBeConvertedTo>( return value->IsArray(); } -template<> +template<> inline bool V8ValueCanBeConvertedTo(v8::Handle value) { using atom::api::Window; if (value->IsObject()) { @@ -124,7 +125,7 @@ bool V8ValueCanBeConvertedTo>( } // Check and convert V8's Arguments to native types. -template +template inline bool FromV8Arguments(const v8::Arguments& args, T1* value) { if (!V8ValueCanBeConvertedTo(args[index])) return false; @@ -132,18 +133,18 @@ bool FromV8Arguments(const v8::Arguments& args, T1* value) { return true; } -template +template inline bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2) { return FromV8Arguments(args, a1) && FromV8Arguments(args, a2); } -template +template inline bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2, T3* a3) { return FromV8Arguments(args, a1, a2) && FromV8Arguments(args, a3); } -template +template inline bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2, @@ -153,7 +154,7 @@ bool FromV8Arguments(const v8::Arguments& args, FromV8Arguments(args, a4); } -template +template inline bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2, @@ -165,7 +166,7 @@ bool FromV8Arguments(const v8::Arguments& args, } template + typename T6> inline bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2, @@ -178,7 +179,7 @@ bool FromV8Arguments(const v8::Arguments& args, } template + typename T6, typename T7> inline bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2, diff --git a/renderer/api/atom_api_renderer_ipc.cc b/renderer/api/atom_api_renderer_ipc.cc index 4bdbd4b8094..16ded068712 100644 --- a/renderer/api/atom_api_renderer_ipc.cc +++ b/renderer/api/atom_api_renderer_ipc.cc @@ -48,7 +48,7 @@ v8::Handle RendererIPC::Send(const v8::Arguments &args) { if (!args[0]->IsString()) return node::ThrowTypeError("Bad argument"); - string16 channel(V8ValueToUTF16(args[0])); + string16 channel = FromV8Value(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(); - string16 channel(V8ValueToUTF16(args[0])); + string16 channel = FromV8Value(args[0]); // Convert Arguments to Array, so we can use V8ValueConverter to convert it // to ListValue. @@ -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(UTF16ToV8Value(json)); + return scope.Close(ToV8Value(json)); } // static diff --git a/renderer/api/atom_renderer_bindings.cc b/renderer/api/atom_renderer_bindings.cc index e0260b9a279..61d90878569 100644 --- a/renderer/api/atom_renderer_bindings.cc +++ b/renderer/api/atom_renderer_bindings.cc @@ -71,7 +71,7 @@ void AtomRendererBindings::OnBrowserMessage(const string16& channel, std::vector> arguments; arguments.reserve(1 + args.GetSize()); - arguments.push_back(UTF16ToV8Value(channel)); + arguments.push_back(ToV8Value(channel)); for (size_t i = 0; i < args.GetSize(); i++) { const base::Value* value; From 11ca836afc007f413f4666691af0bab0e32501d0 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 24 Sep 2013 09:41:54 +0800 Subject: [PATCH 29/34] Use the convient V8 wrapper in all files. --- atom.gyp | 1 - browser/api/atom_api_app.cc | 20 ++-- browser/api/atom_api_auto_updater.cc | 3 +- browser/api/atom_api_browser_ipc.cc | 10 +- browser/api/atom_api_crash_reporter.cc | 9 +- browser/api/atom_api_event.cc | 2 +- browser/api/atom_api_menu.cc | 79 +++++++-------- browser/api/atom_api_menu_mac.mm | 5 +- browser/api/atom_api_protocol.cc | 55 +++++----- browser/api/atom_api_window.cc | 135 +++++++++++++------------ browser/api/atom_browser_bindings.cc | 2 +- common/string16_conversions.h | 22 ---- common/v8_conversions.h | 44 ++++++-- renderer/api/atom_api_renderer_ipc.cc | 2 +- renderer/api/atom_renderer_bindings.cc | 2 +- 15 files changed, 198 insertions(+), 193 deletions(-) delete mode 100644 common/string16_conversions.h diff --git a/atom.gyp b/atom.gyp index bd2ce11b234..47896d76bb9 100644 --- a/atom.gyp +++ b/atom.gyp @@ -157,7 +157,6 @@ 'common/platform_util.h', 'common/platform_util_mac.mm', 'common/platform_util_win.cc', - 'common/string16_conversions.h', 'common/v8_conversions.h', 'common/v8_value_converter_impl.cc', 'common/v8_value_converter_impl.h', diff --git a/browser/api/atom_api_app.cc b/browser/api/atom_api_app.cc index 51a1362b4b6..909bff3950c 100644 --- a/browser/api/atom_api_app.cc +++ b/browser/api/atom_api_app.cc @@ -7,6 +7,7 @@ #include "base/values.h" #include "base/command_line.h" #include "browser/browser.h" +#include "common/v8_conversions.h" #include "vendor/node/src/node.h" namespace atom { @@ -111,14 +112,14 @@ v8::Handle App::GetVersion(const v8::Arguments &args) { v8::Handle App::AppendSwitch(const v8::Arguments &args) { v8::HandleScope scope; - if (!args[0]->IsString()) + std::string switch_string; + if (!FromV8Arguments(args, &switch_string)) return node::ThrowError("Bad argument"); - std::string switch_string(*v8::String::Utf8Value(args[0])); if (args.Length() == 1) { CommandLine::ForCurrentProcess()->AppendSwitch(switch_string); } else { - std::string value(*v8::String::Utf8Value(args[1])); + std::string value = FromV8Value(args[1]); CommandLine::ForCurrentProcess()->AppendSwitchASCII( switch_string, value); } @@ -130,10 +131,10 @@ v8::Handle App::AppendSwitch(const v8::Arguments &args) { v8::Handle App::AppendArgument(const v8::Arguments &args) { v8::HandleScope scope; - if (!args[0]->IsString()) + std::string value; + if (!FromV8Arguments(args, &value)) return node::ThrowError("Bad argument"); - std::string value(*v8::String::Utf8Value(args[0])); CommandLine::ForCurrentProcess()->AppendArg(value); return v8::Undefined(); @@ -143,7 +144,7 @@ v8::Handle App::AppendArgument(const v8::Arguments &args) { // static v8::Handle App::DockBounce(const v8::Arguments& args) { - std::string type(*v8::String::Utf8Value(args[0])); + std::string type = FromV8Value(args[0]); int request_id = -1; if (type == "critical") @@ -158,21 +159,20 @@ v8::Handle App::DockBounce(const v8::Arguments& args) { // static v8::Handle App::DockCancelBounce(const v8::Arguments& args) { - Browser::Get()->DockCancelBounce(args[0]->IntegerValue()); + Browser::Get()->DockCancelBounce(FromV8Value(args[0])); return v8::Undefined(); } // static v8::Handle App::DockSetBadgeText(const v8::Arguments& args) { - std::string label(*v8::String::Utf8Value(args[0])); - Browser::Get()->DockSetBadgeText(label); + Browser::Get()->DockSetBadgeText(FromV8Value(args[0])); return v8::Undefined(); } // static v8::Handle App::DockGetBadgeText(const v8::Arguments& args) { std::string text(Browser::Get()->DockGetBadgeText()); - return v8::String::New(text.data(), text.size()); + return ToV8Value(text); } #endif // defined(OS_MACOSX) diff --git a/browser/api/atom_api_auto_updater.cc b/browser/api/atom_api_auto_updater.cc index e5e2013b6ad..29bdacc056d 100644 --- a/browser/api/atom_api_auto_updater.cc +++ b/browser/api/atom_api_auto_updater.cc @@ -6,6 +6,7 @@ #include "base/values.h" #include "browser/auto_updater.h" +#include "common/v8_conversions.h" namespace atom { @@ -56,7 +57,7 @@ v8::Handle AutoUpdater::New(const v8::Arguments &args) { // static v8::Handle AutoUpdater::SetFeedURL(const v8::Arguments &args) { - auto_updater::AutoUpdater::SetFeedURL(*v8::String::Utf8Value(args[0])); + auto_updater::AutoUpdater::SetFeedURL(FromV8Value(args[0])); return v8::Undefined(); } diff --git a/browser/api/atom_api_browser_ipc.cc b/browser/api/atom_api_browser_ipc.cc index 763a3dfba70..7e43e1edb66 100644 --- a/browser/api/atom_api_browser_ipc.cc +++ b/browser/api/atom_api_browser_ipc.cc @@ -6,7 +6,7 @@ #include "base/values.h" #include "common/api/api_messages.h" -#include "common/string16_conversions.h" +#include "common/v8_conversions.h" #include "common/v8_value_converter_impl.h" #include "content/public/browser/render_view_host.h" #include "vendor/node/src/node.h" @@ -23,13 +23,11 @@ namespace api { v8::Handle BrowserIPC::Send(const v8::Arguments &args) { v8::HandleScope scope; - if (!args[0]->IsString() || !args[1]->IsNumber() || !args[2]->IsNumber()) + string16 channel; + int process_id, routing_id; + if (!FromV8Arguments(args, &channel, &process_id, &routing_id)) return node::ThrowTypeError("Bad argument"); - string16 channel(FromV8Value(args[0])); - int process_id = args[1]->IntegerValue(); - int routing_id = args[2]->IntegerValue(); - RenderViewHost* render_view_host(RenderViewHost::FromID( process_id, routing_id)); if (!render_view_host) diff --git a/browser/api/atom_api_crash_reporter.cc b/browser/api/atom_api_crash_reporter.cc index 2b23db1d1a6..1c5d85de28e 100644 --- a/browser/api/atom_api_crash_reporter.cc +++ b/browser/api/atom_api_crash_reporter.cc @@ -5,6 +5,7 @@ #include "browser/api/atom_api_crash_reporter.h" #include "browser/crash_reporter.h" +#include "common/v8_conversions.h" #include "vendor/node/src/node.h" #include "vendor/node/src/node_internals.h" @@ -14,22 +15,20 @@ namespace api { // static v8::Handle CrashReporter::SetCompanyName(const v8::Arguments &args) { - std::string name(*v8::String::Utf8Value(args[0])); - crash_reporter::CrashReporter::SetCompanyName(name); + crash_reporter::CrashReporter::SetCompanyName(FromV8Value(args[0])); return v8::Undefined(); } // static v8::Handle CrashReporter::SetSubmissionURL( const v8::Arguments &args) { - std::string url(*v8::String::Utf8Value(args[0])); - crash_reporter::CrashReporter::SetSubmissionURL(url); + crash_reporter::CrashReporter::SetSubmissionURL(FromV8Value(args[0])); return v8::Undefined(); } // static v8::Handle CrashReporter::SetAutoSubmit(const v8::Arguments &args) { - crash_reporter::CrashReporter::SetAutoSubmit(args[0]->BooleanValue()); + crash_reporter::CrashReporter::SetAutoSubmit(FromV8Value(args[0])); return v8::Undefined(); } diff --git a/browser/api/atom_api_event.cc b/browser/api/atom_api_event.cc index 7ab14467042..26f8981b1fc 100644 --- a/browser/api/atom_api_event.cc +++ b/browser/api/atom_api_event.cc @@ -6,7 +6,7 @@ #include "browser/native_window.h" #include "common/api/api_messages.h" -#include "common/string16_conversions.h" +#include "common/v8_conversions.h" using node::node_isolate; diff --git a/browser/api/atom_api_menu.cc b/browser/api/atom_api_menu.cc index afb7e157757..a057f307eb9 100644 --- a/browser/api/atom_api_menu.cc +++ b/browser/api/atom_api_menu.cc @@ -5,7 +5,6 @@ #include "browser/api/atom_api_menu.h" #include "browser/ui/accelerator_util.h" -#include "common/string16_conversions.h" #include "common/v8_conversions.h" #define UNWRAP_MEMNU_AND_CHECK \ @@ -83,7 +82,7 @@ bool Menu::GetAcceleratorForCommandId(int command_id, "getAcceleratorForCommandId", command_id); if (shortcut->IsString()) { - std::string shortcut_str(*v8::String::Utf8Value(shortcut)); + std::string shortcut_str = FromV8Value(shortcut); return accelerator_util::StringToAccelerator(shortcut_str, accelerator); } @@ -135,16 +134,15 @@ v8::Handle Menu::New(const v8::Arguments &args) { v8::Handle Menu::InsertItem(const v8::Arguments &args) { UNWRAP_MEMNU_AND_CHECK; - if (!args[0]->IsNumber() || !args[1]->IsNumber() || !args[2]->IsString()) + int index, command_id; + string16 label; + if (!FromV8Arguments(args, &index, &command_id, &label)) return node::ThrowTypeError("Bad argument"); - int index = args[0]->IntegerValue(); - if (index < 0) - self->model_->AddItem(args[1]->IntegerValue(), FromV8Value(args[2])); + self->model_->AddItem(command_id, label); else - self->model_->InsertItemAt( - index, args[1]->IntegerValue(), FromV8Value(args[2])); + self->model_->InsertItemAt(index, command_id, label); return v8::Undefined(); } @@ -153,16 +151,15 @@ v8::Handle Menu::InsertItem(const v8::Arguments &args) { v8::Handle Menu::InsertCheckItem(const v8::Arguments &args) { UNWRAP_MEMNU_AND_CHECK; - if (!args[0]->IsNumber() || !args[1]->IsNumber() || !args[2]->IsString()) + int index, command_id; + string16 label; + if (!FromV8Arguments(args, &index, &command_id, &label)) return node::ThrowTypeError("Bad argument"); - int index = args[0]->IntegerValue(); - int command_id = args[1]->IntegerValue(); - if (index < 0) - self->model_->AddCheckItem(command_id, FromV8Value(args[2])); + self->model_->AddCheckItem(command_id, label); else - self->model_->InsertCheckItemAt(index, command_id, FromV8Value(args[2])); + self->model_->InsertCheckItemAt(index, command_id, label); return v8::Undefined(); } @@ -171,21 +168,15 @@ v8::Handle Menu::InsertCheckItem(const v8::Arguments &args) { v8::Handle Menu::InsertRadioItem(const v8::Arguments &args) { UNWRAP_MEMNU_AND_CHECK; - if (!args[0]->IsNumber() || - !args[1]->IsNumber() || - !args[2]->IsString() || - !args[3]->IsNumber()) + int index, command_id, group_id; + string16 label; + if (!FromV8Arguments(args, &index, &command_id, &label, &group_id)) return node::ThrowTypeError("Bad argument"); - int index = args[0]->IntegerValue(); - int command_id = args[1]->IntegerValue(); - int group_id = args[3]->IntegerValue(); - if (index < 0) - self->model_->AddRadioItem(command_id, FromV8Value(args[2]), group_id); + self->model_->AddRadioItem(command_id, label, group_id); else - self->model_->InsertRadioItemAt( - index, command_id, FromV8Value(args[2]), group_id); + self->model_->InsertRadioItemAt(index, command_id, label, group_id); return v8::Undefined(); } @@ -194,11 +185,10 @@ v8::Handle Menu::InsertRadioItem(const v8::Arguments &args) { v8::Handle Menu::InsertSeparator(const v8::Arguments &args) { UNWRAP_MEMNU_AND_CHECK; - if (!args[0]->IsNumber()) + int index; + if (!FromV8Arguments(args, &index)) return node::ThrowTypeError("Bad argument"); - int index = args[0]->IntegerValue(); - if (index < 0) self->model_->AddSeparator(ui::NORMAL_SEPARATOR); else @@ -211,25 +201,20 @@ v8::Handle Menu::InsertSeparator(const v8::Arguments &args) { v8::Handle Menu::InsertSubMenu(const v8::Arguments &args) { UNWRAP_MEMNU_AND_CHECK; - if (!args[0]->IsNumber() || - !args[1]->IsNumber() || - !args[2]->IsString() || - !args[3]->IsObject()) + int index, command_id; + string16 label; + if (!FromV8Arguments(args, &index, &command_id, &label)) return node::ThrowTypeError("Bad argument"); Menu* submenu = ObjectWrap::Unwrap(args[3]->ToObject()); if (!submenu) return node::ThrowTypeError("The submenu is already destroyed"); - int index = args[0]->IntegerValue(); - int command_id = args[1]->IntegerValue(); - if (index < 0) - self->model_->AddSubMenu( - command_id, FromV8Value(args[2]), submenu->model_.get()); + self->model_->AddSubMenu(command_id, label, submenu->model_.get()); else self->model_->InsertSubMenuAt( - index, command_id, FromV8Value(args[2]), submenu->model_.get()); + index, command_id, label, submenu->model_.get()); return v8::Undefined(); } @@ -238,7 +223,9 @@ v8::Handle Menu::InsertSubMenu(const v8::Arguments &args) { v8::Handle Menu::SetIcon(const v8::Arguments &args) { UNWRAP_MEMNU_AND_CHECK; - if (!args[0]->IsNumber() || !args[1]->IsString()) + int index; + base::FilePath path; + if (!FromV8Arguments(args, &index, &path)) return node::ThrowTypeError("Bad argument"); // FIXME use webkit_glue's image decoder here. @@ -250,10 +237,12 @@ v8::Handle Menu::SetIcon(const v8::Arguments &args) { v8::Handle Menu::SetSublabel(const v8::Arguments &args) { UNWRAP_MEMNU_AND_CHECK; - if (!args[0]->IsNumber() || !args[1]->IsString()) + int index; + string16 label; + if (!FromV8Arguments(args, &index, &label)) return node::ThrowTypeError("Bad argument"); - self->model_->SetSublabel(args[0]->IntegerValue(), FromV8Value(args[1])); + self->model_->SetSublabel(index, label); return v8::Undefined(); } @@ -324,11 +313,11 @@ v8::Handle Menu::IsVisibleAt(const v8::Arguments &args) { v8::Handle Menu::Popup(const v8::Arguments &args) { UNWRAP_MEMNU_AND_CHECK; - Window* window = Window::Unwrap(args[0]->ToObject()); - if (!window) - return node::ThrowTypeError("Invalid window"); + atom::NativeWindow* window; + if (!FromV8Arguments(args, &window)) + return node::ThrowTypeError("Bad argument"); - self->Popup(window->window()); + self->Popup(window); return v8::Undefined(); } diff --git a/browser/api/atom_api_menu_mac.mm b/browser/api/atom_api_menu_mac.mm index acf4849435e..89270190baa 100644 --- a/browser/api/atom_api_menu_mac.mm +++ b/browser/api/atom_api_menu_mac.mm @@ -8,6 +8,7 @@ #include "base/mac/scoped_sending_event.h" #include "base/strings/sys_string_conversions.h" #include "browser/native_window.h" +#include "common/v8_conversions.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_view.h" @@ -95,10 +96,10 @@ v8::Handle Menu::SendActionToFirstResponder( const v8::Arguments &args) { v8::HandleScope scope; - if (!args[0]->IsString()) + std::string action; + if (!FromV8Arguments(args, &action)) return node::ThrowTypeError("Bad argument"); - std::string action(*v8::String::Utf8Value(args[0])); MenuMac::SendActionToFirstResponder(action); return v8::Undefined(); diff --git a/browser/api/atom_api_protocol.cc b/browser/api/atom_api_protocol.cc index 6f0af3dfeb7..a1ae29223e4 100644 --- a/browser/api/atom_api_protocol.cc +++ b/browser/api/atom_api_protocol.cc @@ -9,6 +9,7 @@ #include "browser/net/adapter_request_job.h" #include "browser/net/atom_url_request_context_getter.h" #include "browser/net/atom_url_request_job_factory.h" +#include "common/v8_conversions.h" #include "content/public/browser/browser_thread.h" #include "net/url_request/url_request_context.h" #include "vendor/node/src/node.h" @@ -36,9 +37,9 @@ static const char* kEarlyUseProtocolError = "This method can only be used" void EmitEventInUI(const std::string& event, const std::string& parameter) { v8::HandleScope scope; - v8::Local argv[] = { - v8::String::New(event.data(), event.size()), - v8::String::New(parameter.data(), parameter.size()), + v8::Handle argv[] = { + ToV8Value(event), + ToV8Value(parameter), }; node::MakeCallback(g_protocol_object, "emit", arraysize(argv), argv); } @@ -83,7 +84,7 @@ class CustomProtocolRequestJob : public AdapterRequestJob { // Determine the type of the job we are going to create. if (result->IsString()) { - std::string data = *v8::String::Utf8Value(result); + std::string data = FromV8Value(result); content::BrowserThread::PostTask( content::BrowserThread::IO, FROM_HERE, @@ -95,14 +96,12 @@ class CustomProtocolRequestJob : public AdapterRequestJob { return; } else if (result->IsObject()) { v8::Handle obj = result->ToObject(); - std::string name = *v8::String::Utf8Value(obj->GetConstructorName()); + std::string name = FromV8Value(obj->GetConstructorName()); if (name == "RequestStringJob") { - std::string mime_type = *v8::String::Utf8Value(obj->Get( + std::string mime_type = FromV8Value(obj->Get( v8::String::New("mimeType"))); - std::string charset = *v8::String::Utf8Value(obj->Get( - v8::String::New("charset"))); - std::string data = *v8::String::Utf8Value(obj->Get( - v8::String::New("data"))); + std::string charset = FromV8Value(obj->Get(v8::String::New("charset"))); + std::string data = FromV8Value(obj->Get(v8::String::New("data"))); content::BrowserThread::PostTask( content::BrowserThread::IO, @@ -114,8 +113,7 @@ class CustomProtocolRequestJob : public AdapterRequestJob { data)); return; } else if (name == "RequestFileJob") { - base::FilePath path = base::FilePath::FromUTF8Unsafe( - *v8::String::Utf8Value(obj->Get(v8::String::New("path")))); + base::FilePath path = FromV8Value(obj->Get(v8::String::New("path"))); content::BrowserThread::PostTask( content::BrowserThread::IO, @@ -183,7 +181,11 @@ class CustomProtocolHandler : public ProtocolHandler { // static v8::Handle Protocol::RegisterProtocol(const v8::Arguments& args) { - std::string scheme(*v8::String::Utf8Value(args[0])); + std::string scheme; + v8::Persistent callback; + if (!FromV8Arguments(args, &scheme, &callback)) + return node::ThrowTypeError("Bad argument"); + if (g_handlers.find(scheme) != g_handlers.end() || net::URLRequest::IsHandledProtocol(scheme)) return node::ThrowError("The scheme is already registered"); @@ -192,10 +194,7 @@ v8::Handle Protocol::RegisterProtocol(const v8::Arguments& args) { return node::ThrowError(kEarlyUseProtocolError); // Store the handler in a map. - if (!args[1]->IsFunction()) - return node::ThrowError("Handler must be a function"); - g_handlers[scheme] = v8::Persistent::New( - node::node_isolate, v8::Handle::Cast(args[1])); + g_handlers[scheme] = callback; content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, @@ -206,7 +205,9 @@ v8::Handle Protocol::RegisterProtocol(const v8::Arguments& args) { // static v8::Handle Protocol::UnregisterProtocol(const v8::Arguments& args) { - std::string scheme(*v8::String::Utf8Value(args[0])); + std::string scheme; + if (!FromV8Arguments(args, &scheme)) + return node::ThrowTypeError("Bad argument"); if (AtomBrowserContext::Get()->url_request_context_getter() == NULL) return node::ThrowError(kEarlyUseProtocolError); @@ -226,13 +227,16 @@ v8::Handle Protocol::UnregisterProtocol(const v8::Arguments& args) { // static v8::Handle Protocol::IsHandledProtocol(const v8::Arguments& args) { - return v8::Boolean::New(net::URLRequest::IsHandledProtocol( - *v8::String::Utf8Value(args[0]))); + return ToV8Value(net::URLRequest::IsHandledProtocol(FromV8Value(args[0]))); } // static v8::Handle Protocol::InterceptProtocol(const v8::Arguments& args) { - std::string scheme(*v8::String::Utf8Value(args[0])); + std::string scheme; + v8::Persistent callback; + if (!FromV8Arguments(args, &scheme, &callback)) + return node::ThrowTypeError("Bad argument"); + if (!GetRequestJobFactory()->HasProtocolHandler(scheme)) return node::ThrowError("Cannot intercept procotol"); @@ -243,10 +247,7 @@ v8::Handle Protocol::InterceptProtocol(const v8::Arguments& args) { return node::ThrowError(kEarlyUseProtocolError); // Store the handler in a map. - if (!args[1]->IsFunction()) - return node::ThrowError("Handler must be a function"); - g_handlers[scheme] = v8::Persistent::New( - node::node_isolate, v8::Handle::Cast(args[1])); + g_handlers[scheme] = callback; content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, @@ -256,7 +257,9 @@ v8::Handle Protocol::InterceptProtocol(const v8::Arguments& args) { // static v8::Handle Protocol::UninterceptProtocol(const v8::Arguments& args) { - std::string scheme(*v8::String::Utf8Value(args[0])); + std::string scheme; + if (!FromV8Arguments(args, &scheme)) + return node::ThrowTypeError("Bad argument"); if (AtomBrowserContext::Get()->url_request_context_getter() == NULL) return node::ThrowError(kEarlyUseProtocolError); diff --git a/browser/api/atom_api_window.cc b/browser/api/atom_api_window.cc index be582d87794..64ee74b9f9d 100644 --- a/browser/api/atom_api_window.cc +++ b/browser/api/atom_api_window.cc @@ -6,7 +6,7 @@ #include "base/values.h" #include "browser/native_window.h" -#include "common/string16_conversions.h" +#include "common/v8_conversions.h" #include "common/v8_value_converter_impl.h" #include "content/public/browser/navigation_entry.h" #include "content/public/browser/web_contents.h" @@ -133,7 +133,7 @@ v8::Handle Window::Focus(const v8::Arguments &args) { // static v8::Handle Window::IsFocused(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - return v8::Boolean::New(self->window_->IsFocused()); + return ToV8Value(self->window_->IsFocused()); } // static @@ -194,10 +194,11 @@ v8::Handle Window::Restore(const v8::Arguments &args) { v8::Handle Window::SetFullscreen(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 1 || !args[0]->IsBoolean()) + bool fs; + if (!FromV8Arguments(args, &fs)) return node::ThrowTypeError("Bad argument"); - self->window_->SetFullscreen(args[0]->BooleanValue()); + self->window_->SetFullscreen(fs); return v8::Undefined(); } @@ -205,18 +206,18 @@ v8::Handle Window::SetFullscreen(const v8::Arguments &args) { v8::Handle Window::IsFullscreen(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - return v8::Boolean::New(self->window_->IsFullscreen()); + return ToV8Value(self->window_->IsFullscreen()); } // static v8::Handle Window::SetSize(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 2) + int width, height; + if (!FromV8Arguments(args, &width, &height)) return node::ThrowTypeError("Bad argument"); - self->window_->SetSize( - gfx::Size(args[0]->IntegerValue(), args[1]->IntegerValue())); + self->window_->SetSize(gfx::Size(width, height)); return v8::Undefined(); } @@ -226,8 +227,8 @@ v8::Handle Window::GetSize(const v8::Arguments &args) { gfx::Size size = self->window_->GetSize(); v8::Handle ret = v8::Array::New(2); - ret->Set(0, v8::Integer::New(size.width())); - ret->Set(1, v8::Integer::New(size.height())); + ret->Set(0, ToV8Value(size.width())); + ret->Set(1, ToV8Value(size.height())); return ret; } @@ -236,11 +237,11 @@ v8::Handle Window::GetSize(const v8::Arguments &args) { v8::Handle Window::SetMinimumSize(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 2) + int width, height; + if (!FromV8Arguments(args, &width, &height)) return node::ThrowTypeError("Bad argument"); - self->window_->SetMinimumSize( - gfx::Size(args[0]->IntegerValue(), args[1]->IntegerValue())); + self->window_->SetMinimumSize(gfx::Size(width, height)); return v8::Undefined(); } @@ -250,8 +251,8 @@ v8::Handle Window::GetMinimumSize(const v8::Arguments &args) { gfx::Size size = self->window_->GetMinimumSize(); v8::Handle ret = v8::Array::New(2); - ret->Set(0, v8::Integer::New(size.width())); - ret->Set(1, v8::Integer::New(size.height())); + ret->Set(0, ToV8Value(size.width())); + ret->Set(1, ToV8Value(size.height())); return ret; } @@ -260,11 +261,12 @@ v8::Handle Window::GetMinimumSize(const v8::Arguments &args) { v8::Handle Window::SetMaximumSize(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 2) - return node::ThrowTypeError("Bad argument"); - self->window_->SetMaximumSize( - gfx::Size(args[0]->IntegerValue(), args[1]->IntegerValue())); + int width, height; + if (!FromV8Arguments(args, &width, &height)) + return node::ThrowTypeError("Bad argument"); + + self->window_->SetMaximumSize(gfx::Size(width, height)); return v8::Undefined(); } @@ -274,8 +276,8 @@ v8::Handle Window::GetMaximumSize(const v8::Arguments &args) { gfx::Size size = self->window_->GetMaximumSize(); v8::Handle ret = v8::Array::New(2); - ret->Set(0, v8::Integer::New(size.width())); - ret->Set(1, v8::Integer::New(size.height())); + ret->Set(0, ToV8Value(size.width())); + ret->Set(1, ToV8Value(size.height())); return ret; } @@ -284,10 +286,11 @@ v8::Handle Window::GetMaximumSize(const v8::Arguments &args) { v8::Handle Window::SetResizable(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 1 || !args[0]->IsBoolean()) + bool resizable; + if (!FromV8Arguments(args, &resizable)) return node::ThrowTypeError("Bad argument"); - self->window_->SetResizable(args[0]->BooleanValue()); + self->window_->SetResizable(resizable); return v8::Undefined(); } @@ -295,17 +298,18 @@ v8::Handle Window::SetResizable(const v8::Arguments &args) { v8::Handle Window::IsResizable(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - return v8::Boolean::New(self->window_->IsResizable()); + return ToV8Value(self->window_->IsResizable()); } // static v8::Handle Window::SetAlwaysOnTop(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 1 || !args[0]->IsBoolean()) + bool top; + if (!FromV8Arguments(args, &top)) return node::ThrowTypeError("Bad argument"); - self->window_->SetAlwaysOnTop(args[0]->BooleanValue()); + self->window_->SetAlwaysOnTop(top); return v8::Undefined(); } @@ -313,7 +317,7 @@ v8::Handle Window::SetAlwaysOnTop(const v8::Arguments &args) { v8::Handle Window::IsAlwaysOnTop(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - return v8::Boolean::New(self->window_->IsAlwaysOnTop()); + return ToV8Value(self->window_->IsAlwaysOnTop()); } // static @@ -329,11 +333,11 @@ v8::Handle Window::Center(const v8::Arguments &args) { v8::Handle Window::SetPosition(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 2) + int x, y; + if (!FromV8Arguments(args, &x, &y)) return node::ThrowTypeError("Bad argument"); - self->window_->SetPosition( - gfx::Point(args[0]->IntegerValue(), args[1]->IntegerValue())); + self->window_->SetPosition(gfx::Point(x, y)); return v8::Undefined(); } @@ -343,8 +347,8 @@ v8::Handle Window::GetPosition(const v8::Arguments &args) { gfx::Point pos = self->window_->GetPosition(); v8::Handle ret = v8::Array::New(2); - ret->Set(0, v8::Integer::New(pos.x())); - ret->Set(1, v8::Integer::New(pos.y())); + ret->Set(0, ToV8Value(pos.x())); + ret->Set(1, ToV8Value(pos.y())); return ret; } @@ -353,20 +357,18 @@ v8::Handle Window::GetPosition(const v8::Arguments &args) { v8::Handle Window::SetTitle(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 1 || !args[0]->IsString()) + std::string title; + if (!FromV8Arguments(args, &title)) return node::ThrowTypeError("Bad argument"); - self->window_->SetTitle(*v8::String::Utf8Value(args[0])); + self->window_->SetTitle(title); return v8::Undefined(); } // static v8::Handle Window::GetTitle(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - - std::string title = self->window_->GetTitle(); - - return v8::String::New(title.c_str(), title.size()); + return ToV8Value(self->window_->GetTitle()); } // static @@ -383,10 +385,11 @@ v8::Handle Window::FlashFrame(const v8::Arguments &args) { v8::Handle Window::SetKiosk(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 1 || !args[0]->IsBoolean()) + bool kiosk; + if (!FromV8Arguments(args, &kiosk)) return node::ThrowTypeError("Bad argument"); - self->window_->SetKiosk(args[0]->BooleanValue()); + self->window_->SetKiosk(kiosk); return v8::Undefined(); } @@ -394,7 +397,7 @@ v8::Handle Window::SetKiosk(const v8::Arguments &args) { v8::Handle Window::IsKiosk(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - return v8::Boolean::New(self->window_->IsKiosk()); + return ToV8Value(self->window_->IsKiosk()); } // static @@ -419,9 +422,11 @@ v8::Handle Window::CloseDevTools(const v8::Arguments &args) { v8::Handle Window::InspectElement(const v8::Arguments& args) { UNWRAP_WINDOW_AND_CHECK; - self->window_->InspectElement(args[0]->IntegerValue(), - args[1]->IntegerValue()); + int x, y; + if (!FromV8Arguments(args, &x, &y)) + return node::ThrowTypeError("Bad argument"); + self->window_->InspectElement(x, y); return v8::Undefined(); } @@ -446,7 +451,7 @@ v8::Handle Window::BlurWebView(const v8::Arguments &args) { // static v8::Handle Window::IsWebViewFocused(const v8::Arguments& args) { UNWRAP_WINDOW_AND_CHECK; - return v8::Boolean::New(self->window_->IsWebViewFocused()); + return ToV8Value(self->window_->IsWebViewFocused()); } // static @@ -463,24 +468,21 @@ v8::Handle Window::RestartHangMonitorTimeout( v8::Handle Window::GetPageTitle(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - string16 title = self->window_->GetWebContents()->GetTitle(); - - return ToV8Value(title); + return ToV8Value(self->window_->GetWebContents()->GetTitle()); } // static v8::Handle Window::IsLoading(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - return v8::Boolean::New(self->window_->GetWebContents()->IsLoading()); + return ToV8Value(self->window_->GetWebContents()->IsLoading()); } // static v8::Handle Window::IsWaitingForResponse(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - return v8::Boolean::New( - self->window_->GetWebContents()->IsWaitingForResponse()); + return ToV8Value(self->window_->GetWebContents()->IsWaitingForResponse()); } // static @@ -496,14 +498,14 @@ v8::Handle Window::Stop(const v8::Arguments &args) { v8::Handle Window::GetRoutingID(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - return v8::Integer::New(self->window_->GetWebContents()->GetRoutingID()); + return ToV8Value(self->window_->GetWebContents()->GetRoutingID()); } // static v8::Handle Window::GetProcessID(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - return v8::Integer::New( + return ToV8Value( self->window_->GetWebContents()->GetRenderProcessHost()->GetID()); } @@ -511,19 +513,20 @@ v8::Handle Window::GetProcessID(const v8::Arguments &args) { v8::Handle Window::IsCrashed(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - return v8::Boolean::New(self->window_->GetWebContents()->IsCrashed()); + return ToV8Value(self->window_->GetWebContents()->IsCrashed()); } // static v8::Handle Window::LoadURL(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 1 || !args[0]->IsString()) + std::string url; + if (!FromV8Arguments(args, &url)) return node::ThrowTypeError("Bad argument"); NavigationController& controller = self->window_->GetWebContents()->GetController(); - controller.LoadURL(GURL(*v8::String::Utf8Value(args[0])), + controller.LoadURL(GURL(url), content::Referrer(), content::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string()); @@ -541,7 +544,7 @@ v8::Handle Window::GetURL(const v8::Arguments &args) { if (controller.GetActiveEntry()) url = controller.GetActiveEntry()->GetVirtualURL().spec(); - return v8::String::New(url.c_str(), url.size()); + return ToV8Value(url); } // static @@ -551,7 +554,7 @@ v8::Handle Window::CanGoBack(const v8::Arguments &args) { NavigationController& controller = self->window_->GetWebContents()->GetController(); - return v8::Boolean::New(controller.CanGoBack()); + return ToV8Value(controller.CanGoBack()); } // static @@ -561,21 +564,21 @@ v8::Handle Window::CanGoForward(const v8::Arguments &args) { NavigationController& controller = self->window_->GetWebContents()->GetController(); - return v8::Boolean::New(controller.CanGoForward()); + return ToV8Value(controller.CanGoForward()); } // static v8::Handle Window::CanGoToOffset(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 1) + int offset; + if (!FromV8Arguments(args, &offset)) return node::ThrowTypeError("Bad argument"); NavigationController& controller = self->window_->GetWebContents()->GetController(); - int offset = args[0]->IntegerValue(); - return v8::Boolean::New(controller.CanGoToOffset(offset)); + return ToV8Value(controller.CanGoToOffset(offset)); } // static @@ -604,12 +607,13 @@ v8::Handle Window::GoForward(const v8::Arguments &args) { v8::Handle Window::GoToIndex(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 1) + int index; + if (!FromV8Arguments(args, &index)) return node::ThrowTypeError("Bad argument"); NavigationController& controller = self->window_->GetWebContents()->GetController(); - controller.GoToIndex(args[0]->IntegerValue()); + controller.GoToIndex(index); return v8::Undefined(); } @@ -618,12 +622,13 @@ v8::Handle Window::GoToIndex(const v8::Arguments &args) { v8::Handle Window::GoToOffset(const v8::Arguments &args) { UNWRAP_WINDOW_AND_CHECK; - if (args.Length() < 1) + int offset; + if (!FromV8Arguments(args, &offset)) return node::ThrowTypeError("Bad argument"); NavigationController& controller = self->window_->GetWebContents()->GetController(); - controller.GoToOffset(args[0]->IntegerValue()); + controller.GoToOffset(offset); return v8::Undefined(); } diff --git a/browser/api/atom_browser_bindings.cc b/browser/api/atom_browser_bindings.cc index 9c2189460f6..c2dc9e78cc0 100644 --- a/browser/api/atom_browser_bindings.cc +++ b/browser/api/atom_browser_bindings.cc @@ -9,7 +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_conversions.h" #include "common/v8_value_converter_impl.h" #include "content/public/browser/browser_thread.h" #include "vendor/node/src/node.h" diff --git a/common/string16_conversions.h b/common/string16_conversions.h deleted file mode 100644 index db2d5da0bff..00000000000 --- a/common/string16_conversions.h +++ /dev/null @@ -1,22 +0,0 @@ -// 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 "base/string16.h" -#include "v8/include/v8.h" - -// Converts a V8 value to a string16. -inline string16 FromV8Value(v8::Handle value) { - v8::String::Value s(value); - return string16(reinterpret_cast(*s), s.length()); -} - -// Converts string16 to V8 String. -inline v8::Handle ToV8Value(const string16& s) { - return v8::String::New(reinterpret_cast(s.data()), s.size()); -} - -#endif // COMMON_STRING16_CONVERSIONS_H_ diff --git a/common/v8_conversions.h b/common/v8_conversions.h index a82e5978c99..26cb0bc7271 100644 --- a/common/v8_conversions.h +++ b/common/v8_conversions.h @@ -9,6 +9,7 @@ #include #include "base/files/file_path.h" +#include "base/string16.h" #include "browser/api/atom_api_window.h" #include "v8/include/v8.h" @@ -16,12 +17,21 @@ struct FromV8Value { explicit FromV8Value(v8::Handle value) : value_(value) {} + operator int() { + return value_->IntegerValue(); + } + + operator bool() { + return value_->BooleanValue(); + } + operator std::string() { return *v8::String::Utf8Value(value_); } - operator int() { - return value_->IntegerValue(); + operator string16() { + v8::String::Value s(value_); + return string16(reinterpret_cast(*s), s.length()); } operator base::FilePath() { @@ -59,6 +69,22 @@ struct FromV8Value { }; // Convert arbitrary supported native type to V8 value. +inline v8::Handle ToV8Value(int i) { + return v8::Integer::New(i); +} + +inline v8::Handle ToV8Value(bool b) { + return v8::Boolean::New(b); +} + +inline v8::Handle ToV8Value(const std::string& s) { + return v8::String::New(s.data(), s.size()); +} + +inline v8::Handle ToV8Value(const string16& s) { + return v8::String::New(reinterpret_cast(s.data()), s.size()); +} + inline v8::Handle ToV8Value(const base::FilePath& path) { std::string path_string(path.AsUTF8Unsafe()); return v8::String::New(path_string.data(), path_string.size()); @@ -68,10 +94,6 @@ inline v8::Handle ToV8Value(void* whatever) { return v8::Undefined(); } -inline v8::Handle ToV8Value(int code) { - return v8::Integer::New(code); -} - inline v8::Handle ToV8Value(const std::vector& paths) { v8::Handle result = v8::Array::New(paths.size()); @@ -91,11 +113,21 @@ bool V8ValueCanBeConvertedTo(v8::Handle value) { return value->IsNumber(); } +template<> inline +bool V8ValueCanBeConvertedTo(v8::Handle value) { + return value->IsBoolean(); +} + template<> inline bool V8ValueCanBeConvertedTo(v8::Handle value) { return value->IsString(); } +template<> inline +bool V8ValueCanBeConvertedTo(v8::Handle value) { + return V8ValueCanBeConvertedTo(value); +} + template<> inline bool V8ValueCanBeConvertedTo(v8::Handle value) { return V8ValueCanBeConvertedTo(value); diff --git a/renderer/api/atom_api_renderer_ipc.cc b/renderer/api/atom_api_renderer_ipc.cc index 16ded068712..d1263479722 100644 --- a/renderer/api/atom_api_renderer_ipc.cc +++ b/renderer/api/atom_api_renderer_ipc.cc @@ -6,7 +6,7 @@ #include "base/values.h" #include "common/api/api_messages.h" -#include "common/string16_conversions.h" +#include "common/v8_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" diff --git a/renderer/api/atom_renderer_bindings.cc b/renderer/api/atom_renderer_bindings.cc index 61d90878569..6609d04eb66 100644 --- a/renderer/api/atom_renderer_bindings.cc +++ b/renderer/api/atom_renderer_bindings.cc @@ -8,7 +8,7 @@ #include "base/logging.h" #include "base/values.h" -#include "common/string16_conversions.h" +#include "common/v8_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" From 53c6d51d5604027fe01acb19a9e737f55fb58498 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 24 Sep 2013 10:13:43 +0800 Subject: [PATCH 30/34] win: Accept parent window in ShowOpenDialog. --- browser/ui/file_dialog_win.cc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/browser/ui/file_dialog_win.cc b/browser/ui/file_dialog_win.cc index b5c6d1aad4c..043386cc21a 100644 --- a/browser/ui/file_dialog_win.cc +++ b/browser/ui/file_dialog_win.cc @@ -162,7 +162,8 @@ class FileDialog { SetDefaultFolder(default_path); } - bool Show(HWND window) { + bool Show(atom::NativeWindow* parent_window) { + HWND window = parent_window ? parent_window->GetNativeWindow() : NULL; return dialog_->DoModal(window) == IDOK; } @@ -198,7 +199,8 @@ class FileDialog { } // namespace -bool ShowOpenDialog(const std::string& title, +bool ShowOpenDialog(atom::NativeWindow* parent_window, + const std::string& title, const base::FilePath& default_path, int properties, std::vector* paths) { @@ -214,7 +216,7 @@ bool ShowOpenDialog(const std::string& title, options, std::vector(), std::vector()); - if (!open_dialog.Show(::GetActiveWindow())) + if (!open_dialog.Show(parent_window)) return false; ATL::CComPtr items; @@ -247,7 +249,7 @@ bool ShowOpenDialog(const std::string& title, return true; } -bool ShowSaveDialog(atom::NativeWindow* window, +bool ShowSaveDialog(atom::NativeWindow* parent_window, const std::string& title, const base::FilePath& default_path, base::FilePath* path) { @@ -263,7 +265,7 @@ bool ShowSaveDialog(atom::NativeWindow* window, FOS_FORCEFILESYSTEM | FOS_PATHMUSTEXIST | FOS_OVERWRITEPROMPT, file_ext, std::vector()); - if (!save_dialog.Show(window->GetNativeWindow())) + if (!save_dialog.Show(parent_window)) return false; wchar_t file_name[MAX_PATH]; From 1b5e22f9c4ee0a6a32e9df9220a96ff3ebf80835 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 24 Sep 2013 14:41:11 +0800 Subject: [PATCH 31/34] Default template parameter is not supported on Windows. --- common/v8_conversions.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/common/v8_conversions.h b/common/v8_conversions.h index 26cb0bc7271..c59a88a850f 100644 --- a/common/v8_conversions.h +++ b/common/v8_conversions.h @@ -157,8 +157,8 @@ bool V8ValueCanBeConvertedTo>( } // Check and convert V8's Arguments to native types. -template inline -bool FromV8Arguments(const v8::Arguments& args, T1* value) { +template inline +bool FromV8Arguments(const v8::Arguments& args, T1* value, int index = 0) { if (!V8ValueCanBeConvertedTo(args[index])) return false; *value = static_cast(FromV8Value(args[index])); @@ -167,13 +167,13 @@ bool FromV8Arguments(const v8::Arguments& args, T1* value) { template inline bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2) { - return FromV8Arguments(args, a1) && FromV8Arguments(args, a2); + return FromV8Arguments(args, a1) && FromV8Arguments(args, a2, 1); } template inline bool FromV8Arguments(const v8::Arguments& args, T1* a1, T2* a2, T3* a3) { return FromV8Arguments(args, a1, a2) && - FromV8Arguments(args, a3); + FromV8Arguments(args, a3, 2); } template inline @@ -183,7 +183,7 @@ bool FromV8Arguments(const v8::Arguments& args, T3* a3, T4* a4) { return FromV8Arguments(args, a1, a2, a3) && - FromV8Arguments(args, a4); + FromV8Arguments(args, a4, 3); } template inline @@ -194,7 +194,7 @@ bool FromV8Arguments(const v8::Arguments& args, T4* a4, T5* a5) { return FromV8Arguments(args, a1, a2, a3, a4) && - FromV8Arguments(args, a5); + FromV8Arguments(args, a5, 4); } template(args, a1, a2, a3, a4, a5) && - FromV8Arguments(args, a6); + FromV8Arguments(args, a6, 5); } template(args, a1, a2, a3, a4, a5, a6) && - FromV8Arguments(args, a7); + FromV8Arguments(args, a7, 6); } #endif // COMMON_V8_CONVERSIONS_H_ From 40007345048206e48c826ae392583ec3170af024 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 24 Sep 2013 14:47:39 +0800 Subject: [PATCH 32/34] win: Add dummy implementation for asynchronous open/save dialog. --- browser/ui/file_dialog_win.cc | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/browser/ui/file_dialog_win.cc b/browser/ui/file_dialog_win.cc index 043386cc21a..fa5f6912d02 100644 --- a/browser/ui/file_dialog_win.cc +++ b/browser/ui/file_dialog_win.cc @@ -249,6 +249,20 @@ bool ShowOpenDialog(atom::NativeWindow* parent_window, return true; } +void ShowOpenDialog(atom::NativeWindow* parent_window, + const std::string& title, + const base::FilePath& default_path, + int properties, + const OpenDialogCallback& callback) { + std::vector paths; + bool result = ShowOpenDialog(parent_window, + title, + default_path, + properties, + &paths); + callback.Run(result, paths); +} + bool ShowSaveDialog(atom::NativeWindow* parent_window, const std::string& title, const base::FilePath& default_path, @@ -292,4 +306,13 @@ bool ShowSaveDialog(atom::NativeWindow* parent_window, return true; } +void ShowSaveDialog(atom::NativeWindow* parent_window, + const std::string& title, + const base::FilePath& default_path, + const SaveDialogCallback& callback) { + base::FilePath path; + bool result = ShowSaveDialog(parent_window, title, default_path, &path); + callback.Run(result, path); +} + } // namespace file_dialog From 085f0a2544853cc719509efb9d69a4e53ce3a8e4 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 24 Sep 2013 16:11:23 +0800 Subject: [PATCH 33/34] win: Implement asynchronous ShowMessageBox. --- browser/ui/message_box_win.cc | 58 +++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/browser/ui/message_box_win.cc b/browser/ui/message_box_win.cc index 99da31fabd8..68267c6ddb9 100644 --- a/browser/ui/message_box_win.cc +++ b/browser/ui/message_box_win.cc @@ -39,7 +39,14 @@ class MessageDialog : public base::MessageLoop::Dispatcher, const std::string& detail); virtual ~MessageDialog(); - int result() const { return result_; } + void Show(); + + int GetResult() const; + + void set_callback(const MessageBoxCallback& callback) { + delete_on_close_ = true; + callback_ = callback; + } private: // Overridden from MessageLoop::Dispatcher: @@ -64,11 +71,13 @@ class MessageDialog : public base::MessageLoop::Dispatcher, const ui::Event& event) OVERRIDE; bool should_close_; + bool delete_on_close_; int result_; string16 title_; views::Widget* widget_; views::MessageBoxView* message_box_view_; std::vector buttons_; + MessageBoxCallback callback_; DISALLOW_COPY_AND_ASSIGN(MessageDialog); }; @@ -83,6 +92,7 @@ MessageDialog::MessageDialog(NativeWindow* parent_window, const std::string& message, const std::string& detail) : should_close_(false), + delete_on_close_(false), result_(-1), title_(UTF8ToUTF16(title)), widget_(NULL), @@ -125,12 +135,30 @@ MessageDialog::MessageDialog(NativeWindow* parent_window, set_background(views::Background::CreateSolidBackground( skia::COLORREFToSkColor(GetSysColor(COLOR_WINDOW)))); - widget_->Show(); } MessageDialog::~MessageDialog() { } +void MessageDialog::Show() { + widget_->Show(); +} + +int MessageDialog::GetResult() const { + // When the dialog is closed without choosing anything, we think the user + // chose 'Cancel', otherwise we think the default behavior is chosen. + if (result_ == -1) { + for (size_t i = 0; i < buttons_.size(); ++i) + if (LowerCaseEqualsASCII(buttons_[i]->GetText(), "cancel")) { + return i; + } + + return 0; + } else { + return result_; + } +} + //////////////////////////////////////////////////////////////////////////////// // MessageDialog, private: @@ -146,6 +174,11 @@ string16 MessageDialog::GetWindowTitle() const { void MessageDialog::WindowClosing() { should_close_ = true; + + if (delete_on_close_) { + callback_.Run(GetResult()); + base::MessageLoop::current()->DeleteSoon(FROM_HERE, this); + } } views::Widget* MessageDialog::GetWidget() { @@ -233,6 +266,7 @@ int ShowMessageBox(NativeWindow* parent_window, const std::string& message, const std::string& detail) { MessageDialog dialog(parent_window, type, buttons, title, message, detail); + dialog.Show(); { base::MessageLoop::ScopedNestableTaskAllower allow( base::MessageLoopForUI::current()); @@ -240,18 +274,7 @@ int ShowMessageBox(NativeWindow* parent_window, run_loop.Run(); } - // When the dialog is closed without choosing anything, we think the user - // chose 'Cancel', otherwise we think the default behavior is chosen. - if (dialog.result() == -1) { - for (size_t i = 0; i < buttons.size(); ++i) - if (LowerCaseEqualsASCII(buttons[i], "cancel")) { - return i; - } - - return 0; - } else { - return dialog.result(); - } + return dialog.GetResult(); } void ShowMessageBox(NativeWindow* parent_window, @@ -261,8 +284,11 @@ void ShowMessageBox(NativeWindow* parent_window, const std::string& message, const std::string& detail, const MessageBoxCallback& callback) { - callback.Run(ShowMessageBox( - parent_window, type, buttons, title, message, detail)); + // The dialog would be deleted when the dialog is closed. + MessageDialog* dialog = new MessageDialog( + parent_window, type, buttons, title, message, detail); + dialog->set_callback(callback); + dialog->Show(); } } // namespace atom From 14de58a6b73cc3ca7e7ad9e260f092cf4700f86d Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 24 Sep 2013 18:01:12 +0800 Subject: [PATCH 34/34] Calling asynchronous functions in renderer now doesn't block browser. --- browser/api/lib/dialog.coffee | 4 ++++ browser/atom/rpc-server.coffee | 17 +++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/browser/api/lib/dialog.coffee b/browser/api/lib/dialog.coffee index 7cf538b2f41..5b333f4a13b 100644 --- a/browser/api/lib/dialog.coffee +++ b/browser/api/lib/dialog.coffee @@ -1,4 +1,5 @@ binding = process.atomBinding 'dialog' +v8Util = process.atomBinding 'v8_util' BrowserWindow = require 'browser-window' fileDialogProperties = @@ -72,3 +73,6 @@ module.exports = String(options.detail), window, callback + +# Mark standard asynchronous functions. +v8Util.setHiddenValue f, 'asynchronous', true for k, f of module.exports diff --git a/browser/atom/rpc-server.coffee b/browser/atom/rpc-server.coffee index 0e5c707c403..04d5fe2c948 100644 --- a/browser/atom/rpc-server.coffee +++ b/browser/atom/rpc-server.coffee @@ -61,6 +61,17 @@ unwrapArgs = (processId, routingId, args) -> args.map metaToValue +# Call a function and send reply asynchronously if it's a an asynchronous +# style function and the caller didn't pass a callback. +callFunction = (event, processId, routingId, func, caller, args) -> + if v8Util.getHiddenValue(func, 'asynchronous') and typeof args[args.length - 1] isnt 'function' + args.push (ret) -> + event.returnValue = valueToMeta processId, routingId, ret + func.apply caller, args + else + ret = func.apply caller, args + event.returnValue = valueToMeta processId, routingId, ret + ipc.on 'ATOM_BROWSER_REQUIRE', (event, processId, routingId, module) -> try event.returnValue = valueToMeta processId, routingId, require(module) @@ -100,8 +111,7 @@ ipc.on 'ATOM_BROWSER_FUNCTION_CALL', (event, processId, routingId, id, args) -> try args = unwrapArgs processId, routingId, args func = objectsRegistry.get id - ret = func.apply global, args - event.returnValue = valueToMeta processId, routingId, ret + callFunction event, processId, routingId, func, global, args catch e event.returnValue = errorToMeta e @@ -119,8 +129,7 @@ ipc.on 'ATOM_BROWSER_MEMBER_CALL', (event, processId, routingId, id, method, arg try args = unwrapArgs processId, routingId, args obj = objectsRegistry.get id - ret = obj[method].apply(obj, args) - event.returnValue = valueToMeta processId, routingId, ret + callFunction event, processId, routingId, obj[method], obj, args catch e event.returnValue = errorToMeta e