From 5480cf58c2f8897a58535917fbd23eed327bf06a Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 26 Sep 2013 11:27:09 +0800 Subject: [PATCH] Add spec for destroying synchronous event. --- browser/api/atom_api_event.cc | 13 ++++++++++--- browser/api/atom_api_event.h | 1 + spec/api/ipc.coffee | 13 +++++++++++++ spec/fixtures/api/send-sync-message.html | 9 +++++++++ 4 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 spec/fixtures/api/send-sync-message.html diff --git a/browser/api/atom_api_event.cc b/browser/api/atom_api_event.cc index 26f8981b1fc4..0d69fe3d6d9f 100644 --- a/browser/api/atom_api_event.cc +++ b/browser/api/atom_api_event.cc @@ -23,7 +23,7 @@ Event::Event() } Event::~Event() { - if (sender_) + if (sender_ != NULL) sender_->RemoveObserver(this); } @@ -40,6 +40,7 @@ v8::Handle Event::CreateV8Object() { NODE_SET_PROTOTYPE_METHOD(t, "preventDefault", PreventDefault); NODE_SET_PROTOTYPE_METHOD(t, "sendReply", SendReply); + NODE_SET_PROTOTYPE_METHOD(t, "destroy", Destroy); } v8::Handle v8_event = @@ -87,8 +88,8 @@ v8::Handle Event::SendReply(const v8::Arguments& args) { if (event == NULL) return node::ThrowError("Event is already destroyed"); - if (event->message_ == NULL) - return node::ThrowError("Can only send reply to synchronous events once"); + if (event->message_ == NULL || event->sender_ == NULL) + return node::ThrowError("Can only send reply to synchronous events"); string16 json = FromV8Value(args[0]); @@ -99,6 +100,12 @@ v8::Handle Event::SendReply(const v8::Arguments& args) { return v8::Undefined(); } +// static +v8::Handle Event::Destroy(const v8::Arguments& args) { + delete Unwrap(args.This()); + return v8::Undefined(); +} + } // namespace api } // namespace atom diff --git a/browser/api/atom_api_event.h b/browser/api/atom_api_event.h index 7007892b349e..7abe240c9799 100644 --- a/browser/api/atom_api_event.h +++ b/browser/api/atom_api_event.h @@ -49,6 +49,7 @@ class Event : public node::ObjectWrap, static v8::Handle PreventDefault(const v8::Arguments& args); static v8::Handle SendReply(const v8::Arguments& args); + static v8::Handle Destroy(const v8::Arguments& args); static v8::Persistent constructor_template_; diff --git a/spec/api/ipc.coffee b/spec/api/ipc.coffee index c50635f98887..b182da212081 100644 --- a/spec/api/ipc.coffee +++ b/spec/api/ipc.coffee @@ -2,6 +2,9 @@ assert = require 'assert' ipc = require 'ipc' path = require 'path' remote = require 'remote' +BrowserWindow = remote.require 'browser-window' + +fixtures = path.resolve __dirname, '..', 'fixtures' describe 'ipc', -> fixtures = path.join __dirname, '..', 'fixtures' @@ -58,3 +61,13 @@ describe 'ipc', -> it 'can be replied by setting event.returnValue', -> msg = ipc.sendChannelSync 'echo', 'test' assert.equal msg, 'test' + + it 'does not crash when reply is not sent and both browser and event are destroyed', (done) -> + w = new BrowserWindow(show: false) + remote.require('ipc').once 'send-sync-message', (event) -> + event.returnValue = null + + w.destroy() + event.destroy() + done() + w.loadUrl 'file://' + path.join(fixtures, 'api', 'send-sync-message.html') diff --git a/spec/fixtures/api/send-sync-message.html b/spec/fixtures/api/send-sync-message.html new file mode 100644 index 000000000000..55d80ef092c3 --- /dev/null +++ b/spec/fixtures/api/send-sync-message.html @@ -0,0 +1,9 @@ + + + + + +