From 9df89cf79b7679b73f6c4b43c853a07076ddf9ac Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 15:34:41 +0800 Subject: [PATCH 01/12] Add dummy LoginHandler --- .../atom_resource_dispatcher_host_delegate.cc | 8 ++++ .../atom_resource_dispatcher_host_delegate.h | 3 ++ atom/browser/login_handler.cc | 22 ++++++++++ atom/browser/login_handler.h | 43 +++++++++++++++++++ filenames.gypi | 2 + 5 files changed, 78 insertions(+) create mode 100644 atom/browser/login_handler.cc create mode 100644 atom/browser/login_handler.h diff --git a/atom/browser/atom_resource_dispatcher_host_delegate.cc b/atom/browser/atom_resource_dispatcher_host_delegate.cc index 46904d2ff99d..aaba1f31045b 100644 --- a/atom/browser/atom_resource_dispatcher_host_delegate.cc +++ b/atom/browser/atom_resource_dispatcher_host_delegate.cc @@ -4,6 +4,7 @@ #include "atom/browser/atom_resource_dispatcher_host_delegate.h" +#include "atom/browser/login_handler.h" #include "atom/common/platform_util.h" #include "content/public/browser/browser_thread.h" #include "net/base/escape.h" @@ -29,4 +30,11 @@ bool AtomResourceDispatcherHostDelegate::HandleExternalProtocol( return true; } +content::ResourceDispatcherHostLoginDelegate* +AtomResourceDispatcherHostDelegate::CreateLoginDelegate( + net::AuthChallengeInfo* auth_info, + net::URLRequest* request) { + return new LoginHandler(auth_info, request); +} + } // namespace atom diff --git a/atom/browser/atom_resource_dispatcher_host_delegate.h b/atom/browser/atom_resource_dispatcher_host_delegate.h index 876554f0f964..a90b366bc75b 100644 --- a/atom/browser/atom_resource_dispatcher_host_delegate.h +++ b/atom/browser/atom_resource_dispatcher_host_delegate.h @@ -21,6 +21,9 @@ class AtomResourceDispatcherHostDelegate bool is_main_frame, ui::PageTransition transition, bool has_user_gesture) override; + content::ResourceDispatcherHostLoginDelegate* CreateLoginDelegate( + net::AuthChallengeInfo* auth_info, + net::URLRequest* request) override; }; } // namespace atom diff --git a/atom/browser/login_handler.cc b/atom/browser/login_handler.cc new file mode 100644 index 000000000000..1980cc133abc --- /dev/null +++ b/atom/browser/login_handler.cc @@ -0,0 +1,22 @@ +// Copyright (c) 2015 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/login_handler.h" + +#include "net/base/auth.h" + +namespace atom { + +LoginHandler::LoginHandler(net::AuthChallengeInfo* auth_info, + net::URLRequest* request) + : auth_info_(auth_info), request_(request), weak_factory_(this) { +} + +LoginHandler::~LoginHandler() { +} + +void LoginHandler::OnRequestCancelled() { +} + +} // namespace atom diff --git a/atom/browser/login_handler.h b/atom/browser/login_handler.h new file mode 100644 index 000000000000..421e2e43837d --- /dev/null +++ b/atom/browser/login_handler.h @@ -0,0 +1,43 @@ +// Copyright (c) 2015 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_LOGIN_HANDLER_H_ +#define ATOM_BROWSER_LOGIN_HANDLER_H_ + +#include "base/memory/weak_ptr.h" +#include "content/public/browser/resource_dispatcher_host_login_delegate.h" + +namespace net { +class AuthChallengeInfo; +class URLRequest; +} + +namespace atom { + +class LoginHandler : public content::ResourceDispatcherHostLoginDelegate { + public: + LoginHandler(net::AuthChallengeInfo* auth_info, net::URLRequest* request); + + protected: + ~LoginHandler() override; + + // content::ResourceDispatcherHostLoginDelegate: + void OnRequestCancelled() override; + + private: + // Who/where/what asked for the authentication. + scoped_refptr auth_info_; + + // The request that wants login data. + // This should only be accessed on the IO loop. + net::URLRequest* request_; + + base::WeakPtrFactory weak_factory_; + + DISALLOW_COPY_AND_ASSIGN(LoginHandler); +}; + +} // namespace atom + +#endif // ATOM_BROWSER_LOGIN_HANDLER_H_ diff --git a/filenames.gypi b/filenames.gypi index e6d180d652e1..a294a1e8083f 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -150,6 +150,8 @@ 'atom/browser/common_web_contents_delegate.h', 'atom/browser/javascript_environment.cc', 'atom/browser/javascript_environment.h', + 'atom/browser/login_handler.cc', + 'atom/browser/login_handler.h', 'atom/browser/mac/atom_application.h', 'atom/browser/mac/atom_application.mm', 'atom/browser/mac/atom_application_delegate.h', From 3881ad1c4b49db71a24b6ed2e1208ce6e954e1c0 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 17:36:01 +0800 Subject: [PATCH 02/12] Don't leak FunctionTemplate when converting C++ callback --- .../common/native_mate_converters/callback.cc | 62 +++++++++++++++++++ atom/common/native_mate_converters/callback.h | 33 +++++++++- filenames.gypi | 1 + 3 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 atom/common/native_mate_converters/callback.cc diff --git a/atom/common/native_mate_converters/callback.cc b/atom/common/native_mate_converters/callback.cc new file mode 100644 index 000000000000..0be90f63e757 --- /dev/null +++ b/atom/common/native_mate_converters/callback.cc @@ -0,0 +1,62 @@ +// Copyright (c) 2015 GitHub, Inc. All rights reserved. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/common/native_mate_converters/callback.h" + +namespace mate { + +namespace internal { + +namespace { + +struct TranslaterHolder { + Translater translater; +}; + +// Cached JavaScript version of |CallTranslater|. +v8::Persistent g_call_translater; + +void CallTranslater(v8::Local external, mate::Arguments* args) { + TranslaterHolder* holder = static_cast(external->Value()); + holder->translater.Run(args); +} + +// func.bind(func, arg1). +// NB(zcbenz): Using C++11 version crashes VS. +v8::Local BindFunctionWith(v8::Isolate* isolate, + v8::Local context, + v8::Local func, + v8::Local arg1) { + v8::MaybeLocal bind = func->Get(mate::StringToV8(isolate, "bind")); + CHECK(!bind.IsEmpty()); + v8::Local bind_func = + v8::Local::Cast(bind.ToLocalChecked()); + v8::Local converted[] = { func, arg1 }; + return bind_func->Call( + context, func, arraysize(converted), converted).ToLocalChecked(); +} + +} // namespace + +v8::Local CreateFunctionFromTranslater( + v8::Isolate* isolate, const Translater& translater) { + // The FunctionTemplate is cached. + if (g_call_translater.IsEmpty()) + g_call_translater.Reset( + isolate, + mate::CreateFunctionTemplate(isolate, base::Bind(&CallTranslater))); + + v8::Local call_translater = + v8::Local::New(isolate, g_call_translater); + TranslaterHolder* holder = new TranslaterHolder; + holder->translater = translater; + return BindFunctionWith(isolate, + isolate->GetCurrentContext(), + call_translater->GetFunction(), + v8::External::New(isolate, holder)); +} + +} // namespace internal + +} // namespace mate diff --git a/atom/common/native_mate_converters/callback.h b/atom/common/native_mate_converters/callback.h index 68ea911fe143..4349b9997b25 100644 --- a/atom/common/native_mate_converters/callback.h +++ b/atom/common/native_mate_converters/callback.h @@ -20,6 +20,7 @@ namespace internal { typedef scoped_refptr > SafeV8Function; +// Helper to invoke a V8 function with C++ parameters. template struct V8FunctionInvoker {}; @@ -81,13 +82,41 @@ struct V8FunctionInvoker { } }; +// Helper to pass a C++ funtion to JavaScript. +using Translater = base::Callback; +v8::Local CreateFunctionFromTranslater( + v8::Isolate* isolate, const Translater& translater); + +// Calls callback with Arguments. +template +struct NativeFunctionInvoker {}; + +template +struct NativeFunctionInvoker { + static void Go(base::Callback val, Arguments* args) { + using Indices = typename IndicesGenerator::type; + Invoker invoker(args, 0); + if (invoker.IsOK()) + invoker.DispatchToCallback(val); + } +}; + +// Create a static function that accepts generic callback. +template +Translater ConvertToTranslater(const base::Callback& val) { + return base::Bind(&NativeFunctionInvoker::Go, val); +} + } // namespace internal template struct Converter > { static v8::Local ToV8(v8::Isolate* isolate, - const base::Callback& val) { - return CreateFunctionTemplate(isolate, val)->GetFunction(); + const base::Callback& val) { + // We don't use CreateFunctionTemplate here because it creates a new + // FunctionTemplate everytime, which is cached by V8 and causes leaks. + internal::Translater translater = internal::ConvertToTranslater(val); + return internal::CreateFunctionFromTranslater(isolate, translater); } static bool FromV8(v8::Isolate* isolate, v8::Local val, diff --git a/filenames.gypi b/filenames.gypi index a294a1e8083f..65bf6b79a2c2 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -304,6 +304,7 @@ 'atom/common/native_mate_converters/accelerator_converter.h', 'atom/common/native_mate_converters/blink_converter.cc', 'atom/common/native_mate_converters/blink_converter.h', + 'atom/common/native_mate_converters/callback.cc', 'atom/common/native_mate_converters/callback.h', 'atom/common/native_mate_converters/file_path_converter.h', 'atom/common/native_mate_converters/gfx_converter.cc', From 5d8a31c1401bccf21bac1d50c111871f292258e7 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 18:19:36 +0800 Subject: [PATCH 03/12] spec: Return early on error --- spec/api-protocol-spec.coffee | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/api-protocol-spec.coffee b/spec/api-protocol-spec.coffee index 02fd8d5e402a..8990f8029ee1 100644 --- a/spec/api-protocol-spec.coffee +++ b/spec/api-protocol-spec.coffee @@ -26,6 +26,7 @@ describe 'protocol module', -> callback(text) callback() protocol.registerStringProtocol protocolName, doubleHandler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -36,6 +37,7 @@ describe 'protocol module', -> it 'sends error when callback is called with nothing', (done) -> protocol.registerBufferProtocol protocolName, emptyHandler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -48,6 +50,7 @@ describe 'protocol module', -> handler = (request, callback) -> setImmediate -> callback(text) protocol.registerStringProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -66,6 +69,7 @@ describe 'protocol module', -> it 'sends string as response', (done) -> handler = (request, callback) -> callback(text) protocol.registerStringProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -77,6 +81,7 @@ describe 'protocol module', -> it 'sends object as response', (done) -> handler = (request, callback) -> callback(data: text, mimeType: 'text/html') protocol.registerStringProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data, statux, request) -> @@ -88,6 +93,7 @@ describe 'protocol module', -> it 'fails when sending object other than string', (done) -> handler = (request, callback) -> callback(new Date) protocol.registerBufferProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -102,6 +108,7 @@ describe 'protocol module', -> it 'sends Buffer as response', (done) -> handler = (request, callback) -> callback(buffer) protocol.registerBufferProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -113,6 +120,7 @@ describe 'protocol module', -> it 'sends object as response', (done) -> handler = (request, callback) -> callback(data: buffer, mimeType: 'text/html') protocol.registerBufferProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data, statux, request) -> @@ -124,6 +132,7 @@ describe 'protocol module', -> it 'fails when sending string', (done) -> handler = (request, callback) -> callback(text) protocol.registerBufferProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -142,6 +151,7 @@ describe 'protocol module', -> it 'sends file path as response', (done) -> handler = (request, callback) -> callback(filePath) protocol.registerFileProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -153,6 +163,7 @@ describe 'protocol module', -> it 'sends object as response', (done) -> handler = (request, callback) -> callback(path: filePath) protocol.registerFileProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data, statux, request) -> @@ -164,6 +175,7 @@ describe 'protocol module', -> it 'can send normal file', (done) -> handler = (request, callback) -> callback(normalPath) protocol.registerFileProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -176,6 +188,7 @@ describe 'protocol module', -> fakeFilePath = path.join __dirname, 'fixtures', 'asar', 'a.asar', 'not-exist' handler = (request, callback) -> callback(fakeFilePath) protocol.registerBufferProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -187,6 +200,7 @@ describe 'protocol module', -> it 'fails when sending unsupported content', (done) -> handler = (request, callback) -> callback(new Date) protocol.registerBufferProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -206,6 +220,7 @@ describe 'protocol module', -> url = "http://127.0.0.1:#{port}" handler = (request, callback) -> callback({url}) protocol.registerHttpProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -217,6 +232,7 @@ describe 'protocol module', -> it 'fails when sending invalid url', (done) -> handler = (request, callback) -> callback({url: 'url'}) protocol.registerHttpProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -228,6 +244,7 @@ describe 'protocol module', -> it 'fails when sending unsupported content', (done) -> handler = (request, callback) -> callback(new Date) protocol.registerHttpProtocol protocolName, handler, (error) -> + return done(error) if error $.ajax url: "#{protocolName}://fake-host" success: (data) -> @@ -288,6 +305,7 @@ describe 'protocol module', -> callback(text) callback() protocol.interceptStringProtocol 'http', doubleHandler, (error) -> + return done(error) if error $.ajax url: 'http://fake-host' success: (data) -> @@ -298,6 +316,7 @@ describe 'protocol module', -> it 'sends error when callback is called with nothing', (done) -> protocol.interceptBufferProtocol 'http', emptyHandler, (error) -> + return done(error) if error $.ajax url: 'http://fake-host' success: (data) -> @@ -310,6 +329,7 @@ describe 'protocol module', -> it 'can intercept http protocol', (done) -> handler = (request, callback) -> callback(text) protocol.interceptStringProtocol 'http', handler, (error) -> + return done(error) if error $.ajax url: 'http://fake-host' success: (data) -> @@ -322,6 +342,7 @@ describe 'protocol module', -> handler = (request, callback) -> callback({mimeType: 'application/json', data: '{"value": 1}'}) protocol.interceptStringProtocol 'http', handler, (error) -> + return done(error) if error $.ajax url: 'http://fake-host' success: (data) -> @@ -335,6 +356,7 @@ describe 'protocol module', -> it 'can intercept http protocol', (done) -> handler = (request, callback) -> callback(new Buffer(text)) protocol.interceptBufferProtocol 'http', handler, (error) -> + return done(error) if error $.ajax url: 'http://fake-host' success: (data) -> From 984774773670fbbcf58f7bbed07cc3138a0a5cf1 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 18:25:55 +0800 Subject: [PATCH 04/12] Use the callback converter in JsAsker --- atom/browser/net/js_asker.cc | 69 +++++------------------------------- 1 file changed, 9 insertions(+), 60 deletions(-) diff --git a/atom/browser/net/js_asker.cc b/atom/browser/net/js_asker.cc index d838ae39638f..444124b24b61 100644 --- a/atom/browser/net/js_asker.cc +++ b/atom/browser/net/js_asker.cc @@ -8,7 +8,6 @@ #include "atom/common/native_mate_converters/callback.h" #include "atom/common/native_mate_converters/v8_value_converter.h" -#include "native_mate/function_template.h" namespace atom { @@ -16,18 +15,12 @@ namespace internal { namespace { -struct CallbackHolder { - ResponseCallback callback; -}; - -// Cached JavaScript version of |HandlerCallback|. -v8::Persistent g_handler_callback_; - // The callback which is passed to |handler|. -void HandlerCallback(v8::Isolate* isolate, - v8::Local external, +void HandlerCallback(const ResponseCallback& callback, v8::Local state, mate::Arguments* args) { + v8::Isolate* isolate = args->isolate(); + // Check if the callback has already been called. v8::Local called_symbol = mate::StringToSymbol(isolate, "called"); if (state->Has(called_symbol)) @@ -36,14 +29,11 @@ void HandlerCallback(v8::Isolate* isolate, state->Set(called_symbol, v8::Boolean::New(isolate, true)); // If there is no argument passed then we failed. - scoped_ptr holder( - static_cast(external->Value())); - CHECK(holder); v8::Local value; if (!args->GetNext(&value)) { content::BrowserThread::PostTask( content::BrowserThread::IO, FROM_HERE, - base::Bind(holder->callback, false, nullptr)); + base::Bind(callback, false, nullptr)); return; } @@ -53,42 +43,7 @@ void HandlerCallback(v8::Isolate* isolate, scoped_ptr options(converter.FromV8Value(value, context)); content::BrowserThread::PostTask( content::BrowserThread::IO, FROM_HERE, - base::Bind(holder->callback, true, base::Passed(&options))); -} - -// func.bind(func, arg1, arg2). -// NB(zcbenz): Using C++11 version crashes VS. -v8::Local BindFunctionWith(v8::Isolate* isolate, - v8::Local context, - v8::Local func, - v8::Local arg1, - v8::Local arg2) { - v8::MaybeLocal bind = func->Get(mate::StringToV8(isolate, "bind")); - CHECK(!bind.IsEmpty()); - v8::Local bind_func = - v8::Local::Cast(bind.ToLocalChecked()); - v8::Local converted[] = { func, arg1, arg2 }; - return bind_func->Call( - context, func, arraysize(converted), converted).ToLocalChecked(); -} - -// Generate the callback that will be passed to |handler|. -v8::MaybeLocal GenerateCallback(v8::Isolate* isolate, - v8::Local context, - const ResponseCallback& callback) { - // The FunctionTemplate is cached. - if (g_handler_callback_.IsEmpty()) - g_handler_callback_.Reset( - isolate, - mate::CreateFunctionTemplate(isolate, base::Bind(&HandlerCallback))); - - v8::Local handler_callback = - v8::Local::New(isolate, g_handler_callback_); - CallbackHolder* holder = new CallbackHolder; - holder->callback = callback; - return BindFunctionWith(isolate, context, handler_callback->GetFunction(), - v8::External::New(isolate, holder), - v8::Object::New(isolate)); + base::Bind(callback, true, base::Passed(&options))); } } // namespace @@ -102,16 +57,10 @@ void AskForOptions(v8::Isolate* isolate, v8::HandleScope handle_scope(isolate); v8::Local context = isolate->GetCurrentContext(); v8::Context::Scope context_scope(context); - // We don't convert the callback to C++ directly because creating - // FunctionTemplate will cause memory leak since V8 never releases it. So we - // have to create the function object in JavaScript to work around it. - v8::MaybeLocal wrapped_callback = GenerateCallback( - isolate, context, callback); - if (wrapped_callback.IsEmpty()) { - callback.Run(false, nullptr); - return; - } - handler.Run(request, wrapped_callback.ToLocalChecked()); + v8::Local state = v8::Object::New(isolate); + handler.Run(request, + mate::ConvertToV8(isolate, + base::Bind(&HandlerCallback, callback, state))); } bool IsErrorOptions(base::Value* value, int* error) { From 54dad72d922d203e70878803dc9b75ead2ce3600 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 18:32:21 +0800 Subject: [PATCH 05/12] Don't leak TranslaterHolder --- atom/common/native_mate_converters/callback.cc | 9 +++++---- atom/common/native_mate_converters/callback.h | 11 +++-------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/atom/common/native_mate_converters/callback.cc b/atom/common/native_mate_converters/callback.cc index 0be90f63e757..dbfa3b4babef 100644 --- a/atom/common/native_mate_converters/callback.cc +++ b/atom/common/native_mate_converters/callback.cc @@ -4,21 +4,22 @@ #include "atom/common/native_mate_converters/callback.h" +#include "native_mate/wrappable.h" + namespace mate { namespace internal { namespace { -struct TranslaterHolder { +struct TranslaterHolder : public Wrappable { Translater translater; }; // Cached JavaScript version of |CallTranslater|. v8::Persistent g_call_translater; -void CallTranslater(v8::Local external, mate::Arguments* args) { - TranslaterHolder* holder = static_cast(external->Value()); +void CallTranslater(TranslaterHolder* holder, mate::Arguments* args) { holder->translater.Run(args); } @@ -54,7 +55,7 @@ v8::Local CreateFunctionFromTranslater( return BindFunctionWith(isolate, isolate->GetCurrentContext(), call_translater->GetFunction(), - v8::External::New(isolate, holder)); + holder->GetWrapper(isolate)); } } // namespace internal diff --git a/atom/common/native_mate_converters/callback.h b/atom/common/native_mate_converters/callback.h index 4349b9997b25..228fc0d3bb1d 100644 --- a/atom/common/native_mate_converters/callback.h +++ b/atom/common/native_mate_converters/callback.h @@ -101,21 +101,16 @@ struct NativeFunctionInvoker { } }; -// Create a static function that accepts generic callback. -template -Translater ConvertToTranslater(const base::Callback& val) { - return base::Bind(&NativeFunctionInvoker::Go, val); -} - } // namespace internal template -struct Converter > { +struct Converter> { static v8::Local ToV8(v8::Isolate* isolate, const base::Callback& val) { // We don't use CreateFunctionTemplate here because it creates a new // FunctionTemplate everytime, which is cached by V8 and causes leaks. - internal::Translater translater = internal::ConvertToTranslater(val); + internal::Translater translater = base::Bind( + &internal::NativeFunctionInvoker::Go, val); return internal::CreateFunctionFromTranslater(isolate, translater); } static bool FromV8(v8::Isolate* isolate, From d05255179a6b451041472354066f847384dfe787 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 19:34:01 +0800 Subject: [PATCH 06/12] Add login event for "app" module --- atom/browser/api/atom_api_app.cc | 25 +++++++++++--- atom/browser/api/atom_api_app.h | 1 + atom/browser/browser.cc | 4 +++ atom/browser/browser.h | 5 +++ atom/browser/browser_observer.h | 5 +++ atom/browser/login_handler.cc | 59 +++++++++++++++++++++++++++++++- atom/browser/login_handler.h | 15 ++++++-- 7 files changed, 106 insertions(+), 8 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index c79dea9f7c5c..436d76fb9022 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -13,10 +13,11 @@ #include "atom/browser/api/atom_api_menu.h" #include "atom/browser/api/atom_api_session.h" +#include "atom/browser/api/atom_api_web_contents.h" #include "atom/browser/atom_browser_context.h" #include "atom/browser/atom_browser_main_parts.h" #include "atom/browser/browser.h" -#include "atom/browser/api/atom_api_web_contents.h" +#include "atom/browser/login_handler.h" #include "atom/common/native_mate_converters/callback.h" #include "atom/common/native_mate_converters/file_path_converter.h" #include "atom/common/node_includes.h" @@ -132,8 +133,6 @@ void OnClientCertificateSelected( v8::Isolate* isolate, std::shared_ptr delegate, mate::Arguments* args) { - v8::Locker locker(isolate); - v8::HandleScope handle_scope(isolate); mate::Dictionary cert_data; if (!(args->Length() == 1 && args->GetNext(&cert_data))) { args->ThrowError(); @@ -147,10 +146,18 @@ void OnClientCertificateSelected( net::X509Certificate::CreateCertificateListFromBytes( encoded_data.data(), encoded_data.size(), net::X509Certificate::FORMAT_AUTO); - delegate->ContinueWithCertificate(certs[0].get()); } +void PassLoginInformation(scoped_refptr login_handler, + mate::Arguments* args) { + base::string16 username, password; + if (args->GetNext(&username) && args->GetNext(&password)) + login_handler->Login(username, password); + else + login_handler->CancelAuth(); +} + } // namespace App::App() { @@ -233,6 +240,16 @@ void App::OnSelectCertificate( cert_request_info->client_certs[0].get()); } +void App::OnLogin(LoginHandler* login_handler) { + bool prevent_default = + Emit("login", base::Bind(&PassLoginInformation, + make_scoped_refptr(login_handler))); + + // Default behavior is to alwasy cancel the auth. + if (!prevent_default) + login_handler->CancelAuth(); +} + void App::OnGpuProcessCrashed(base::TerminationStatus exit_code) { Emit("gpu-process-crashed"); } diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index 94cd9bbc68f9..63cda4447e32 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -50,6 +50,7 @@ class App : public mate::EventEmitter, content::WebContents* web_contents, net::SSLCertRequestInfo* cert_request_info, scoped_ptr delegate) override; + void OnLogin(LoginHandler* login_handler) override; // content::GpuDataManagerObserver: void OnGpuProcessCrashed(base::TerminationStatus exit_code) override; diff --git a/atom/browser/browser.cc b/atom/browser/browser.cc index 79ebe91a87e6..739921fda90f 100644 --- a/atom/browser/browser.cc +++ b/atom/browser/browser.cc @@ -134,6 +134,10 @@ void Browser::ClientCertificateSelector( delegate.Pass())); } +void Browser::RequestLogin(LoginHandler* login_handler) { + FOR_EACH_OBSERVER(BrowserObserver, observers_, OnLogin(login_handler)); +} + void Browser::NotifyAndShutdown() { if (is_shutdown_) return; diff --git a/atom/browser/browser.h b/atom/browser/browser.h index bae281d4d370..8719e18f3140 100644 --- a/atom/browser/browser.h +++ b/atom/browser/browser.h @@ -29,6 +29,8 @@ class MenuModel; namespace atom { +class LoginHandler; + // This class is used for control application-wide operations. class Browser : public WindowListObserver { public: @@ -122,6 +124,9 @@ class Browser : public WindowListObserver { net::SSLCertRequestInfo* cert_request_info, scoped_ptr delegate); + // Request basic auth login. + void RequestLogin(LoginHandler* login_handler); + void AddObserver(BrowserObserver* obs) { observers_.AddObserver(obs); } diff --git a/atom/browser/browser_observer.h b/atom/browser/browser_observer.h index 45e86e620f84..7dccbfbac3c5 100644 --- a/atom/browser/browser_observer.h +++ b/atom/browser/browser_observer.h @@ -20,6 +20,8 @@ class SSLCertRequestInfo; namespace atom { +class LoginHandler; + class BrowserObserver { public: // The browser is about to close all windows. @@ -57,6 +59,9 @@ class BrowserObserver { net::SSLCertRequestInfo* cert_request_info, scoped_ptr delegate) {} + // The browser requests HTTP login. + virtual void OnLogin(LoginHandler* login_handler) {} + protected: virtual ~BrowserObserver() {} }; diff --git a/atom/browser/login_handler.cc b/atom/browser/login_handler.cc index 1980cc133abc..ededcd0c8513 100644 --- a/atom/browser/login_handler.cc +++ b/atom/browser/login_handler.cc @@ -4,19 +4,76 @@ #include "atom/browser/login_handler.h" +#include "atom/browser/browser.h" +#include "content/public/browser/browser_thread.h" +#include "content/public/browser/resource_dispatcher_host.h" #include "net/base/auth.h" +#include "net/url_request/url_request.h" + +using content::BrowserThread; namespace atom { +namespace { + +// Helper to remove the ref from an net::URLRequest to the LoginHandler. +// Should only be called from the IO thread, since it accesses an +// net::URLRequest. +void ResetLoginHandlerForRequest(net::URLRequest* request) { + content::ResourceDispatcherHost::Get()->ClearLoginDelegateForRequest(request); +} + +} // namespace + LoginHandler::LoginHandler(net::AuthChallengeInfo* auth_info, net::URLRequest* request) - : auth_info_(auth_info), request_(request), weak_factory_(this) { + : auth_info_(auth_info), request_(request) { + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(&Browser::RequestLogin, + base::Unretained(Browser::Get()), + make_scoped_refptr(this))); } LoginHandler::~LoginHandler() { } +void LoginHandler::Login(const base::string16& username, + const base::string16& password) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&LoginHandler::DoLogin, this, username, password)); +} + +void LoginHandler::CancelAuth() { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, + base::Bind(&LoginHandler::DoCancelAuth, this)); +} + void LoginHandler::OnRequestCancelled() { + request_ = nullptr; +} + +void LoginHandler::DoCancelAuth() { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + + if (request_) { + request_->CancelAuth(); + // Verify that CancelAuth doesn't destroy the request via our delegate. + DCHECK(request_ != nullptr); + ResetLoginHandlerForRequest(request_); + } +} + +void LoginHandler::DoLogin(const base::string16& username, + const base::string16& password) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + + if (request_) { + request_->SetAuth(net::AuthCredentials(username, password)); + ResetLoginHandlerForRequest(request_); + } } } // namespace atom diff --git a/atom/browser/login_handler.h b/atom/browser/login_handler.h index 421e2e43837d..077e4b6cf950 100644 --- a/atom/browser/login_handler.h +++ b/atom/browser/login_handler.h @@ -5,7 +5,7 @@ #ifndef ATOM_BROWSER_LOGIN_HANDLER_H_ #define ATOM_BROWSER_LOGIN_HANDLER_H_ -#include "base/memory/weak_ptr.h" +#include "base/strings/string16.h" #include "content/public/browser/resource_dispatcher_host_login_delegate.h" namespace net { @@ -15,10 +15,17 @@ class URLRequest; namespace atom { +// Handles the HTTP basic auth, must be created on IO thread. class LoginHandler : public content::ResourceDispatcherHostLoginDelegate { public: LoginHandler(net::AuthChallengeInfo* auth_info, net::URLRequest* request); + // The auth is cancelled, must be called on UI thread. + void CancelAuth(); + + // Login with |username| and |password|, must be called on UI thread. + void Login(const base::string16& username, const base::string16& password); + protected: ~LoginHandler() override; @@ -26,6 +33,10 @@ class LoginHandler : public content::ResourceDispatcherHostLoginDelegate { void OnRequestCancelled() override; private: + // Must be called on IO thread. + void DoCancelAuth(); + void DoLogin(const base::string16& username, const base::string16& password); + // Who/where/what asked for the authentication. scoped_refptr auth_info_; @@ -33,8 +44,6 @@ class LoginHandler : public content::ResourceDispatcherHostLoginDelegate { // This should only be accessed on the IO loop. net::URLRequest* request_; - base::WeakPtrFactory weak_factory_; - DISALLOW_COPY_AND_ASSIGN(LoginHandler); }; From 131531219e172f70d374f315b7301a7c2b1e93f9 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 20:13:06 +0800 Subject: [PATCH 07/12] Pass auth_info and request in "login" event --- atom/browser/api/atom_api_app.cc | 7 ++-- atom/browser/api/atom_api_protocol.cc | 17 +--------- atom/browser/login_handler.h | 3 ++ .../content_converter.cc | 34 +++++++++++++++++++ .../content_converter.h | 31 +++++++++++++++++ filenames.gypi | 2 ++ 6 files changed, 75 insertions(+), 19 deletions(-) create mode 100644 atom/common/native_mate_converters/content_converter.cc create mode 100644 atom/common/native_mate_converters/content_converter.h diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 436d76fb9022..db1f99352309 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -19,6 +19,7 @@ #include "atom/browser/browser.h" #include "atom/browser/login_handler.h" #include "atom/common/native_mate_converters/callback.h" +#include "atom/common/native_mate_converters/content_converter.h" #include "atom/common/native_mate_converters/file_path_converter.h" #include "atom/common/node_includes.h" #include "atom/common/options_switches.h" @@ -241,9 +242,9 @@ void App::OnSelectCertificate( } void App::OnLogin(LoginHandler* login_handler) { - bool prevent_default = - Emit("login", base::Bind(&PassLoginInformation, - make_scoped_refptr(login_handler))); + bool prevent_default = Emit( + "login", login_handler->request(), login_handler->auth_info(), + base::Bind(&PassLoginInformation, make_scoped_refptr(login_handler))); // Default behavior is to alwasy cancel the auth. if (!prevent_default) diff --git a/atom/browser/api/atom_api_protocol.cc b/atom/browser/api/atom_api_protocol.cc index 661ab1b5cbdd..f6cc6d796557 100644 --- a/atom/browser/api/atom_api_protocol.cc +++ b/atom/browser/api/atom_api_protocol.cc @@ -12,27 +12,12 @@ #include "atom/browser/net/url_request_fetch_job.h" #include "atom/browser/net/url_request_string_job.h" #include "atom/common/native_mate_converters/callback.h" +#include "atom/common/native_mate_converters/content_converter.h" #include "atom/common/node_includes.h" #include "native_mate/dictionary.h" using content::BrowserThread; -namespace mate { - -template<> -struct Converter { - static v8::Local ToV8(v8::Isolate* isolate, - const net::URLRequest* val) { - return mate::ObjectTemplateBuilder(isolate) - .SetValue("method", val->method()) - .SetValue("url", val->url().spec()) - .SetValue("referrer", val->referrer()) - .Build()->NewInstance(); - } -}; - -} // namespace mate - namespace atom { namespace api { diff --git a/atom/browser/login_handler.h b/atom/browser/login_handler.h index 077e4b6cf950..60e0a7a5298d 100644 --- a/atom/browser/login_handler.h +++ b/atom/browser/login_handler.h @@ -26,6 +26,9 @@ class LoginHandler : public content::ResourceDispatcherHostLoginDelegate { // Login with |username| and |password|, must be called on UI thread. void Login(const base::string16& username, const base::string16& password); + const net::AuthChallengeInfo* auth_info() const { return auth_info_.get(); } + const net::URLRequest* request() const { return request_; } + protected: ~LoginHandler() override; diff --git a/atom/common/native_mate_converters/content_converter.cc b/atom/common/native_mate_converters/content_converter.cc new file mode 100644 index 000000000000..0ab9f272041e --- /dev/null +++ b/atom/common/native_mate_converters/content_converter.cc @@ -0,0 +1,34 @@ +// Copyright (c) 2015 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/common/native_mate_converters/content_converter.h" + +#include "native_mate/dictionary.h" +#include "net/url_request/url_request.h" + +namespace mate { + +// static +v8::Local Converter::ToV8( + v8::Isolate* isolate, const net::URLRequest* val) { + mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate); + dict.Set("method", val->method()); + dict.Set("url", val->url().spec()); + dict.Set("referrer", val->referrer()); + return mate::ConvertToV8(isolate, dict); +}; + +// static +v8::Local Converter::ToV8( + v8::Isolate* isolate, const net::AuthChallengeInfo* val) { + mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate); + dict.Set("isProxy", val->is_proxy); + dict.Set("scheme", val->scheme); + dict.Set("host", val->challenger.host()); + dict.Set("port", static_cast(val->challenger.port())); + dict.Set("realm", val->realm); + return mate::ConvertToV8(isolate, dict); +}; + +} // namespace mate diff --git a/atom/common/native_mate_converters/content_converter.h b/atom/common/native_mate_converters/content_converter.h new file mode 100644 index 000000000000..0d4d5fe2cce0 --- /dev/null +++ b/atom/common/native_mate_converters/content_converter.h @@ -0,0 +1,31 @@ +// Copyright (c) 2015 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_COMMON_NATIVE_MATE_CONVERTERS_CONTENT_CONVERTER_H_ +#define ATOM_COMMON_NATIVE_MATE_CONVERTERS_CONTENT_CONVERTER_H_ + +#include "native_mate/converter.h" + +namespace net { +class AuthChallengeInfo; +class URLRequest; +} + +namespace mate { + +template<> +struct Converter { + static v8::Local ToV8(v8::Isolate* isolate, + const net::URLRequest* val); +}; + +template<> +struct Converter { + static v8::Local ToV8(v8::Isolate* isolate, + const net::AuthChallengeInfo* val); +}; + +} // namespace mate + +#endif // ATOM_COMMON_NATIVE_MATE_CONVERTERS_CONTENT_CONVERTER_H_ diff --git a/filenames.gypi b/filenames.gypi index 65bf6b79a2c2..f66485edd19d 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -306,6 +306,8 @@ 'atom/common/native_mate_converters/blink_converter.h', 'atom/common/native_mate_converters/callback.cc', 'atom/common/native_mate_converters/callback.h', + 'atom/common/native_mate_converters/content_converter.cc', + 'atom/common/native_mate_converters/content_converter.h', 'atom/common/native_mate_converters/file_path_converter.h', 'atom/common/native_mate_converters/gfx_converter.cc', 'atom/common/native_mate_converters/gfx_converter.h', From 4eac6b31b15c0a5df839be3c2f4238eef065b453 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 20:21:56 +0800 Subject: [PATCH 08/12] Also pass "webContents" in "login" event --- atom/browser/api/atom_api_app.cc | 1 + atom/browser/login_handler.cc | 18 +++++++++++++++++- atom/browser/login_handler.h | 12 ++++++++++++ .../content_converter.cc | 4 ++-- 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index db1f99352309..5380d52ad693 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -244,6 +244,7 @@ void App::OnSelectCertificate( void App::OnLogin(LoginHandler* login_handler) { bool prevent_default = Emit( "login", login_handler->request(), login_handler->auth_info(), + api::WebContents::CreateFrom(isolate(), login_handler->GetWebContents()), base::Bind(&PassLoginInformation, make_scoped_refptr(login_handler))); // Default behavior is to alwasy cancel the auth. diff --git a/atom/browser/login_handler.cc b/atom/browser/login_handler.cc index ededcd0c8513..ca6e5de57df0 100644 --- a/atom/browser/login_handler.cc +++ b/atom/browser/login_handler.cc @@ -6,7 +6,10 @@ #include "atom/browser/browser.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/render_frame_host.h" #include "content/public/browser/resource_dispatcher_host.h" +#include "content/public/browser/resource_request_info.h" +#include "content/public/browser/web_contents.h" #include "net/base/auth.h" #include "net/url_request/url_request.h" @@ -27,7 +30,12 @@ void ResetLoginHandlerForRequest(net::URLRequest* request) { LoginHandler::LoginHandler(net::AuthChallengeInfo* auth_info, net::URLRequest* request) - : auth_info_(auth_info), request_(request) { + : auth_info_(auth_info), + request_(request), + render_process_host_id_(0), + render_frame_id_(0) { + content::ResourceRequestInfo::ForRequest(request_)->GetAssociatedRenderFrame( + &render_process_host_id_, &render_frame_id_); BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(&Browser::RequestLogin, base::Unretained(Browser::Get()), @@ -37,6 +45,14 @@ LoginHandler::LoginHandler(net::AuthChallengeInfo* auth_info, LoginHandler::~LoginHandler() { } +content::WebContents* LoginHandler::GetWebContents() const { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + + content::RenderFrameHost* rfh = content::RenderFrameHost::FromID( + render_process_host_id_, render_frame_id_); + return content::WebContents::FromRenderFrameHost(rfh); +} + void LoginHandler::Login(const base::string16& username, const base::string16& password) { DCHECK_CURRENTLY_ON(BrowserThread::UI); diff --git a/atom/browser/login_handler.h b/atom/browser/login_handler.h index 60e0a7a5298d..899bc8ca450e 100644 --- a/atom/browser/login_handler.h +++ b/atom/browser/login_handler.h @@ -8,6 +8,10 @@ #include "base/strings/string16.h" #include "content/public/browser/resource_dispatcher_host_login_delegate.h" +namespace content { +class WebContents; +} + namespace net { class AuthChallengeInfo; class URLRequest; @@ -20,6 +24,10 @@ class LoginHandler : public content::ResourceDispatcherHostLoginDelegate { public: LoginHandler(net::AuthChallengeInfo* auth_info, net::URLRequest* request); + // Returns the WebContents associated with the request, must be called on UI + // thread. + content::WebContents* GetWebContents() const; + // The auth is cancelled, must be called on UI thread. void CancelAuth(); @@ -47,6 +55,10 @@ class LoginHandler : public content::ResourceDispatcherHostLoginDelegate { // This should only be accessed on the IO loop. net::URLRequest* request_; + // Cached from the net::URLRequest, in case it goes NULL on us. + int render_process_host_id_; + int render_frame_id_; + DISALLOW_COPY_AND_ASSIGN(LoginHandler); }; diff --git a/atom/common/native_mate_converters/content_converter.cc b/atom/common/native_mate_converters/content_converter.cc index 0ab9f272041e..b6f3a2c1cc03 100644 --- a/atom/common/native_mate_converters/content_converter.cc +++ b/atom/common/native_mate_converters/content_converter.cc @@ -17,7 +17,7 @@ v8::Local Converter::ToV8( dict.Set("url", val->url().spec()); dict.Set("referrer", val->referrer()); return mate::ConvertToV8(isolate, dict); -}; +} // static v8::Local Converter::ToV8( @@ -29,6 +29,6 @@ v8::Local Converter::ToV8( dict.Set("port", static_cast(val->challenger.port())); dict.Set("realm", val->realm); return mate::ConvertToV8(isolate, dict); -}; +} } // namespace mate From f40a3f72d72dc9659dce0208abd298f4253a183a Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 20:44:46 +0800 Subject: [PATCH 09/12] Converted callback can only be called for once --- atom/browser/net/js_asker.cc | 16 ++-------- .../common/native_mate_converters/callback.cc | 29 ++++++++++++++----- spec/api-protocol-spec.coffee | 12 +++++--- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/atom/browser/net/js_asker.cc b/atom/browser/net/js_asker.cc index 444124b24b61..8f0d1d2b9577 100644 --- a/atom/browser/net/js_asker.cc +++ b/atom/browser/net/js_asker.cc @@ -16,18 +16,7 @@ namespace internal { namespace { // The callback which is passed to |handler|. -void HandlerCallback(const ResponseCallback& callback, - v8::Local state, - mate::Arguments* args) { - v8::Isolate* isolate = args->isolate(); - - // Check if the callback has already been called. - v8::Local called_symbol = mate::StringToSymbol(isolate, "called"); - if (state->Has(called_symbol)) - return; // no nothing - else - state->Set(called_symbol, v8::Boolean::New(isolate, true)); - +void HandlerCallback(const ResponseCallback& callback, mate::Arguments* args) { // If there is no argument passed then we failed. v8::Local value; if (!args->GetNext(&value)) { @@ -57,10 +46,9 @@ void AskForOptions(v8::Isolate* isolate, v8::HandleScope handle_scope(isolate); v8::Local context = isolate->GetCurrentContext(); v8::Context::Scope context_scope(context); - v8::Local state = v8::Object::New(isolate); handler.Run(request, mate::ConvertToV8(isolate, - base::Bind(&HandlerCallback, callback, state))); + base::Bind(&HandlerCallback, callback))); } bool IsErrorOptions(base::Value* value, int* error) { diff --git a/atom/common/native_mate_converters/callback.cc b/atom/common/native_mate_converters/callback.cc index dbfa3b4babef..3bcb748689fb 100644 --- a/atom/common/native_mate_converters/callback.cc +++ b/atom/common/native_mate_converters/callback.cc @@ -4,23 +4,36 @@ #include "atom/common/native_mate_converters/callback.h" -#include "native_mate/wrappable.h" - namespace mate { namespace internal { namespace { -struct TranslaterHolder : public Wrappable { +struct TranslaterHolder { Translater translater; }; // Cached JavaScript version of |CallTranslater|. v8::Persistent g_call_translater; -void CallTranslater(TranslaterHolder* holder, mate::Arguments* args) { +void CallTranslater(v8::Local external, + v8::Local state, + mate::Arguments* args) { + v8::Isolate* isolate = args->isolate(); + + // Check if the callback has already been called. + v8::Local called_symbol = mate::StringToSymbol(isolate, "called"); + if (state->Has(called_symbol)) { + args->ThrowError("callback can only be called for once"); + return; + } else { + state->Set(called_symbol, v8::Boolean::New(isolate, true)); + } + + TranslaterHolder* holder = static_cast(external->Value()); holder->translater.Run(args); + delete holder; } // func.bind(func, arg1). @@ -28,12 +41,13 @@ void CallTranslater(TranslaterHolder* holder, mate::Arguments* args) { v8::Local BindFunctionWith(v8::Isolate* isolate, v8::Local context, v8::Local func, - v8::Local arg1) { + v8::Local arg1, + v8::Local arg2) { v8::MaybeLocal bind = func->Get(mate::StringToV8(isolate, "bind")); CHECK(!bind.IsEmpty()); v8::Local bind_func = v8::Local::Cast(bind.ToLocalChecked()); - v8::Local converted[] = { func, arg1 }; + v8::Local converted[] = { func, arg1, arg2 }; return bind_func->Call( context, func, arraysize(converted), converted).ToLocalChecked(); } @@ -55,7 +69,8 @@ v8::Local CreateFunctionFromTranslater( return BindFunctionWith(isolate, isolate->GetCurrentContext(), call_translater->GetFunction(), - holder->GetWrapper(isolate)); + v8::External::New(isolate, holder), + v8::Object::New(isolate)); } } // namespace internal diff --git a/spec/api-protocol-spec.coffee b/spec/api-protocol-spec.coffee index 8990f8029ee1..4ac7786b057b 100644 --- a/spec/api-protocol-spec.coffee +++ b/spec/api-protocol-spec.coffee @@ -23,8 +23,10 @@ describe 'protocol module', -> it 'does not crash when handler is called twice', (done) -> doubleHandler = (request, callback) -> - callback(text) - callback() + try + callback(text) + callback() + catch protocol.registerStringProtocol protocolName, doubleHandler, (error) -> return done(error) if error $.ajax @@ -302,8 +304,10 @@ describe 'protocol module', -> it 'does not crash when handler is called twice', (done) -> doubleHandler = (request, callback) -> - callback(text) - callback() + try + callback(text) + callback() + catch protocol.interceptStringProtocol 'http', doubleHandler, (error) -> return done(error) if error $.ajax From 569e87035add289e89a80e1aa74363dd85a964c0 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 21:00:39 +0800 Subject: [PATCH 10/12] Also emit "login" on WebContents --- atom/browser/api/atom_api_app.cc | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 5380d52ad693..21a553e4d1d5 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -242,12 +242,26 @@ void App::OnSelectCertificate( } void App::OnLogin(LoginHandler* login_handler) { - bool prevent_default = Emit( - "login", login_handler->request(), login_handler->auth_info(), - api::WebContents::CreateFrom(isolate(), login_handler->GetWebContents()), + // Convert the args explicitly since they will be passed for twice. + v8::Locker locker(isolate()); + v8::HandleScope handle_scope(isolate()); + auto web_contents = + WebContents::CreateFrom(isolate(), login_handler->GetWebContents()); + auto request = mate::ConvertToV8(isolate(), login_handler->request()); + auto auth_info = mate::ConvertToV8(isolate(), login_handler->auth_info()); + auto callback = mate::ConvertToV8( + isolate(), base::Bind(&PassLoginInformation, make_scoped_refptr(login_handler))); - // Default behavior is to alwasy cancel the auth. + bool prevent_default = + Emit("login", web_contents, request, auth_info, callback); + + // Also pass it to WebContents. + if (!prevent_default) + prevent_default = + web_contents->Emit("login", request, auth_info, callback); + + // Default behavior is to always cancel the auth. if (!prevent_default) login_handler->CancelAuth(); } From 862c3187f58daae649d9c360147025e9f5909279 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 21:14:00 +0800 Subject: [PATCH 11/12] docs: login event --- docs/api/app.md | 47 +++++++++++++++++++++++++++++++++------- docs/api/web-contents.md | 21 ++++++++++++++++++ 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/docs/api/app.md b/docs/api/app.md index bb1509b68688..dbb46698f926 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -133,18 +133,23 @@ Emitted when a new [browserWindow](browser-window.md) is created. ### Event: 'select-certificate' -Emitted when a client certificate is requested. - Returns: * `event` Event -* `webContents` [WebContents](browser-window.md#class-webcontents) -* `url` String +* `webContents` [WebContents](web-contents.md) +* `url` URL * `certificateList` [Objects] * `data` PEM encoded data * `issuerName` Issuer's Common Name * `callback` Function +Emitted when a client certificate is requested. + +The `url` corresponds to the navigation entry requesting the client certificate +and `callback` needs to be called with an entry filtered from the list. Using +`event.preventDefault()` prevents the application from using the first +certificate from the store. + ```javascript app.on('select-certificate', function(event, host, url, list, callback) { event.preventDefault(); @@ -152,10 +157,36 @@ app.on('select-certificate', function(event, host, url, list, callback) { }) ``` -The `url` corresponds to the navigation entry requesting the client certificate -and `callback` needs to be called with an entry filtered from the list. Using -`event.preventDefault()` prevents the application from using the first -certificate from the store. +### Event: 'login' + +Returns: + +* `event` Event +* `webContents` [WebContents](web-contents.md) +* `request` Object + * `method` String + * `url` URL + * `referrer` URL +* `authInfo` Object + * `isProxy` Boolean + * `scheme` String + * `host` String + * `port` Integer + * `realm` String +* `callback` Function + +Emitted when `webContents` wants to do basic auth. + +The default behavior is to cancel all authentications, to override this you +should prevent the default behavior with `event.preventDefault()` and call +`callback(username, password)` with the credentials. + +```javascript +app.on('login', function(event, webContents, request, authInfo, callback) { + event.preventDefault(); + callback('username', 'secret'); +}) +``` ### Event: 'gpu-process-crashed' diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index 3113356e34b4..52a06b87edb1 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -167,6 +167,27 @@ Emitted when DevTools is closed. Emitted when DevTools is focused / opened. +### Event: 'login' + +Returns: + +* `event` Event +* `request` Object + * `method` String + * `url` URL + * `referrer` URL +* `authInfo` Object + * `isProxy` Boolean + * `scheme` String + * `host` String + * `port` Integer + * `realm` String +* `callback` Function + +Emitted when `webContents` wants to do basic auth. + +The usage is the same with [the `login` event of `app`](app.md#event-login). + ## Instance Methods The `webContents` object has the following instance methods: From 51ba37440d9008ed4a0cb5293ec15d3331367017 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 28 Oct 2015 21:20:08 +0800 Subject: [PATCH 12/12] Guard against multiple calls of auth --- atom/browser/login_handler.cc | 16 +++++++++++++++- atom/browser/login_handler.h | 9 +++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/atom/browser/login_handler.cc b/atom/browser/login_handler.cc index ca6e5de57df0..7a1a77cc2b13 100644 --- a/atom/browser/login_handler.cc +++ b/atom/browser/login_handler.cc @@ -30,7 +30,8 @@ void ResetLoginHandlerForRequest(net::URLRequest* request) { LoginHandler::LoginHandler(net::AuthChallengeInfo* auth_info, net::URLRequest* request) - : auth_info_(auth_info), + : handled_auth_(false), + auth_info_(auth_info), request_(request), render_process_host_id_(0), render_frame_id_(0) { @@ -56,6 +57,8 @@ content::WebContents* LoginHandler::GetWebContents() const { void LoginHandler::Login(const base::string16& username, const base::string16& password) { DCHECK_CURRENTLY_ON(BrowserThread::UI); + if (TestAndSetAuthHandled()) + return; BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, base::Bind(&LoginHandler::DoLogin, this, username, password)); @@ -63,14 +66,25 @@ void LoginHandler::Login(const base::string16& username, void LoginHandler::CancelAuth() { DCHECK_CURRENTLY_ON(BrowserThread::UI); + if (TestAndSetAuthHandled()) + return; BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(&LoginHandler::DoCancelAuth, this)); } void LoginHandler::OnRequestCancelled() { + TestAndSetAuthHandled(); request_ = nullptr; } +// Marks authentication as handled and returns the previous handled state. +bool LoginHandler::TestAndSetAuthHandled() { + base::AutoLock lock(handled_auth_lock_); + bool was_handled = handled_auth_; + handled_auth_ = true; + return was_handled; +} + void LoginHandler::DoCancelAuth() { DCHECK_CURRENTLY_ON(BrowserThread::IO); diff --git a/atom/browser/login_handler.h b/atom/browser/login_handler.h index 899bc8ca450e..52ec1abf5b1a 100644 --- a/atom/browser/login_handler.h +++ b/atom/browser/login_handler.h @@ -6,6 +6,7 @@ #define ATOM_BROWSER_LOGIN_HANDLER_H_ #include "base/strings/string16.h" +#include "base/synchronization/lock.h" #include "content/public/browser/resource_dispatcher_host_login_delegate.h" namespace content { @@ -48,6 +49,14 @@ class LoginHandler : public content::ResourceDispatcherHostLoginDelegate { void DoCancelAuth(); void DoLogin(const base::string16& username, const base::string16& password); + // Marks authentication as handled and returns the previous handled + // state. + bool TestAndSetAuthHandled(); + + // True if we've handled auth (Login or CancelAuth has been called). + bool handled_auth_; + mutable base::Lock handled_auth_lock_; + // Who/where/what asked for the authentication. scoped_refptr auth_info_;