Fixing code review issues: refactoring emit methods using CustomEmit.

This commit is contained in:
ali.ibrahim 2016-10-26 11:10:15 +02:00
parent e9db926b48
commit 6d92457095
4 changed files with 53 additions and 60 deletions

View file

@ -6,6 +6,7 @@
#include <string>
#include "atom/browser/api/atom_api_session.h"
#include "atom/browser/net/atom_url_request.h"
#include "atom/common/api/event_emitter_caller.h"
#include "atom/common/native_mate_converters/callback.h"
#include "atom/common/native_mate_converters/net_converter.h"
#include "atom/common/native_mate_converters/string16_converter.h"
@ -52,43 +53,6 @@ struct Converter<scoped_refptr<const net::IOBufferWithSize>> {
} // namespace mate
namespace {
template <typename... ArgTypes>
std::array<v8::Local<v8::Value>, sizeof...(ArgTypes)> BuildArgsArray(
v8::Isolate* isolate,
ArgTypes... args) {
std::array<v8::Local<v8::Value>, sizeof...(ArgTypes)> result = {
{mate::ConvertToV8(isolate, args)...}};
return result;
}
template <typename... ArgTypes>
void EmitRequestEvent(v8::Isolate* isolate,
v8::Local<v8::Object> object,
ArgTypes... args) {
v8::HandleScope handle_scope(isolate);
auto arguments = BuildArgsArray(isolate, args...);
v8::Local<v8::Function> _emitRequestEvent;
if (mate::Dictionary(isolate, object)
.Get("_emitRequestEvent", &_emitRequestEvent))
_emitRequestEvent->Call(object, arguments.size(), arguments.data());
}
template <typename... ArgTypes>
void EmitResponseEvent(v8::Isolate* isolate,
v8::Local<v8::Object> object,
ArgTypes... args) {
v8::HandleScope handle_scope(isolate);
auto arguments = BuildArgsArray(isolate, args...);
v8::Local<v8::Function> _emitResponseEvent;
if (mate::Dictionary(isolate, object)
.Get("_emitResponseEvent", &_emitResponseEvent))
_emitResponseEvent->Call(object, arguments.size(), arguments.data());
}
} // namespace
namespace atom {
namespace api {
@ -249,7 +213,7 @@ bool URLRequest::Write(scoped_refptr<const net::IOBufferWithSize> buffer,
if (is_last) {
request_state_.SetFlag(RequestStateFlags::kFinished);
EmitRequestEvent(isolate(), GetWrapper(), true, "finish");
EmitRequestEvent(true, "finish");
}
DCHECK(atom_request_);
@ -273,10 +237,10 @@ void URLRequest::Cancel() {
// Really cancel if it was started.
atom_request_->Cancel();
}
EmitRequestEvent(isolate(), GetWrapper(), true, "abort");
EmitRequestEvent(true, "abort");
if (response_state_.Started() && !response_state_.Ended()) {
EmitResponseEvent(isolate(), GetWrapper(), true, "aborted");
EmitResponseEvent(true, "aborted");
}
Close();
}
@ -385,10 +349,10 @@ void URLRequest::OnError(const std::string& error, bool isRequestError) {
auto error_object = v8::Exception::Error(mate::StringToV8(isolate(), error));
if (isRequestError) {
request_state_.SetFlag(RequestStateFlags::kFailed);
EmitRequestEvent(isolate(), GetWrapper(), false, "error", error_object);
EmitRequestEvent(false, "error", error_object);
} else {
response_state_.SetFlag(ResponseStateFlags::kFailed);
EmitResponseEvent(isolate(), GetWrapper(), false, "error", error_object);
EmitResponseEvent(false, "error", error_object);
}
Close();
}
@ -431,9 +395,9 @@ void URLRequest::Close() {
request_state_.SetFlag(RequestStateFlags::kClosed);
if (response_state_.Started()) {
// Emit a close event if we really have a response object.
EmitResponseEvent(isolate(), GetWrapper(), true, "close");
EmitResponseEvent(true, "close");
}
EmitRequestEvent(isolate(), GetWrapper(), true, "close");
EmitRequestEvent(true, "close");
}
Unpin();
if (atom_request_) {
@ -454,6 +418,18 @@ void URLRequest::Unpin() {
wrapper_.Reset();
}
template <typename... Args>
void URLRequest::EmitRequestEvent(Args... args) {
v8::HandleScope handle_scope(isolate());
mate::CustomEmit(isolate(), GetWrapper(), "_emitRequestEvent", args...);
}
template <typename... Args>
void URLRequest::EmitResponseEvent(Args... args) {
v8::HandleScope handle_scope(isolate());
mate::CustomEmit(isolate(), GetWrapper(), "_emitResponseEvent", args...);
}
} // namespace api
} // namespace atom

View file

@ -183,6 +183,10 @@ class URLRequest : public mate::EventEmitter<URLRequest> {
void Close();
void Pin();
void Unpin();
template <typename... Args>
void EmitRequestEvent(Args... args);
template <typename... Args>
void EmitResponseEvent(Args... args);
scoped_refptr<AtomURLRequest> atom_request_;
RequestState request_state_;

View file

@ -11,16 +11,16 @@ namespace mate {
namespace internal {
v8::Local<v8::Value> CallEmitWithArgs(v8::Isolate* isolate,
v8::Local<v8::Object> obj,
ValueVector* args) {
v8::Local<v8::Value> CallMethodWithArgs(v8::Isolate* isolate,
v8::Local<v8::Object> obj,
const char* method,
ValueVector* args) {
// Perform microtask checkpoint after running JavaScript.
v8::MicrotasksScope script_scope(
isolate, v8::MicrotasksScope::kRunMicrotasks);
v8::MicrotasksScope script_scope(isolate,
v8::MicrotasksScope::kRunMicrotasks);
// Use node::MakeCallback to call the callback, and it will also run pending
// tasks in Node.js.
return node::MakeCallback(
isolate, obj, "emit", args->size(), &args->front());
return node::MakeCallback(isolate, obj, method, args->size(), &args->front());
}
} // namespace internal

View file

@ -15,37 +15,50 @@ namespace internal {
using ValueVector = std::vector<v8::Local<v8::Value>>;
v8::Local<v8::Value> CallEmitWithArgs(v8::Isolate* isolate,
v8::Local<v8::Object> obj,
ValueVector* args);
v8::Local<v8::Value> CallMethodWithArgs(v8::Isolate* isolate,
v8::Local<v8::Object> obj,
const char* method,
ValueVector* args);
} // namespace internal
// obj.emit.apply(obj, name, args...);
// The caller is responsible of allocating a HandleScope.
template<typename StringType, typename... Args>
template <typename StringType>
v8::Local<v8::Value> EmitEvent(v8::Isolate* isolate,
v8::Local<v8::Object> obj,
const StringType& name,
const internal::ValueVector& args) {
internal::ValueVector concatenated_args = { StringToV8(isolate, name) };
internal::ValueVector concatenated_args = {StringToV8(isolate, name)};
concatenated_args.reserve(1 + args.size());
concatenated_args.insert(concatenated_args.end(), args.begin(), args.end());
return internal::CallEmitWithArgs(isolate, obj, &concatenated_args);
return internal::CallMethodWithArgs(isolate, obj, "emit", &concatenated_args);
}
// obj.emit(name, args...);
// The caller is responsible of allocating a HandleScope.
template<typename StringType, typename... Args>
template <typename StringType, typename... Args>
v8::Local<v8::Value> EmitEvent(v8::Isolate* isolate,
v8::Local<v8::Object> obj,
const StringType& name,
const Args&... args) {
internal::ValueVector converted_args = {
StringToV8(isolate, name),
StringToV8(isolate, name), ConvertToV8(isolate, args)...,
};
return internal::CallMethodWithArgs(isolate, obj, "emit", &converted_args);
}
// obj.custom_emit(args...)
template <typename... Args>
v8::Local<v8::Value> CustomEmit(v8::Isolate* isolate,
v8::Local<v8::Object> object,
const char* custom_emit,
const Args&... args) {
internal::ValueVector converted_args = {
ConvertToV8(isolate, args)...,
};
return internal::CallEmitWithArgs(isolate, obj, &converted_args);
return internal::CallMethodWithArgs(isolate, object, custom_emit,
&converted_args);
}
} // namespace mate