From b80429ab7f5142a4d9a568fff0137248ae252c24 Mon Sep 17 00:00:00 2001 From: Micha Hanselmann Date: Thu, 1 Aug 2019 07:57:41 -0700 Subject: [PATCH] refactor: migrate dialog API to //gin (#19482) * get ShowMessageBoxSync working with gin * move more dialog methods * all methods moved * cleanup * add util func for template creation --- filenames.gni | 8 ++- shell/browser/api/atom_api_dialog.cc | 66 +++++++++++-------- shell/common/api/gin_utils.h | 27 ++++++++ .../file_dialog_converter_gin_adapter.h | 33 ++++++++++ .../image_converter_gin_adapter.h | 29 ++++++++ .../message_box_converter.cc | 16 ++--- .../gin_converters/message_box_converter.h | 33 ++++++++++ .../net_converter_gin_adapter.h | 34 ++++++++++ .../message_box_converter.h | 22 ------- 9 files changed, 209 insertions(+), 59 deletions(-) create mode 100644 shell/common/api/gin_utils.h create mode 100644 shell/common/gin_converters/file_dialog_converter_gin_adapter.h create mode 100644 shell/common/gin_converters/image_converter_gin_adapter.h rename shell/common/{native_mate_converters => gin_converters}/message_box_converter.cc (69%) create mode 100644 shell/common/gin_converters/message_box_converter.h create mode 100644 shell/common/gin_converters/net_converter_gin_adapter.h delete mode 100644 shell/common/native_mate_converters/message_box_converter.h diff --git a/filenames.gni b/filenames.gni index d870a211ab2e..27db8b6fd793 100644 --- a/filenames.gni +++ b/filenames.gni @@ -471,6 +471,7 @@ filenames = { "shell/common/api/event_emitter_caller.cc", "shell/common/api/event_emitter_caller.h", "shell/common/api/features.cc", + "shell/common/api/gin_utils.h", "shell/common/api/locker.cc", "shell/common/api/locker.h", "shell/common/api/object_life_monitor.cc", @@ -508,6 +509,11 @@ filenames = { "shell/common/crash_reporter/linux/crash_dump_handler.h", "shell/common/crash_reporter/win/crash_service_main.cc", "shell/common/crash_reporter/win/crash_service_main.h", + "shell/common/gin_converters/file_dialog_converter_gin_adapter.h", + "shell/common/gin_converters/image_converter_gin_adapter.h", + "shell/common/gin_converters/message_box_converter.cc", + "shell/common/gin_converters/message_box_converter.h", + "shell/common/gin_converters/net_converter_gin_adapter.h", "shell/common/gin_util.h", "shell/common/heap_snapshot.cc", "shell/common/heap_snapshot.h", @@ -530,8 +536,6 @@ filenames = { "shell/common/native_mate_converters/content_converter.h", "shell/common/native_mate_converters/file_dialog_converter.cc", "shell/common/native_mate_converters/file_dialog_converter.h", - "shell/common/native_mate_converters/message_box_converter.cc", - "shell/common/native_mate_converters/message_box_converter.h", "shell/common/native_mate_converters/file_path_converter.h", "shell/common/native_mate_converters/gfx_converter.cc", "shell/common/native_mate_converters/gfx_converter.h", diff --git a/shell/browser/api/atom_api_dialog.cc b/shell/browser/api/atom_api_dialog.cc index c9dd53ace687..48328976e6ac 100644 --- a/shell/browser/api/atom_api_dialog.cc +++ b/shell/browser/api/atom_api_dialog.cc @@ -6,18 +6,14 @@ #include #include -#include "native_mate/dictionary.h" -#include "shell/browser/api/atom_api_browser_window.h" -#include "shell/browser/native_window.h" +#include "gin/dictionary.h" #include "shell/browser/ui/certificate_trust.h" #include "shell/browser/ui/file_dialog.h" #include "shell/browser/ui/message_box.h" -#include "shell/common/native_mate_converters/callback.h" -#include "shell/common/native_mate_converters/file_dialog_converter.h" -#include "shell/common/native_mate_converters/file_path_converter.h" -#include "shell/common/native_mate_converters/image_converter.h" -#include "shell/common/native_mate_converters/message_box_converter.h" -#include "shell/common/native_mate_converters/net_converter.h" +#include "shell/common/api/gin_utils.h" +#include "shell/common/gin_converters/file_dialog_converter_gin_adapter.h" +#include "shell/common/gin_converters/message_box_converter.h" +#include "shell/common/gin_converters/net_converter_gin_adapter.h" #include "shell/common/node_includes.h" #include "shell/common/promise_util.h" @@ -30,17 +26,18 @@ int ShowMessageBoxSync(const electron::MessageBoxSettings& settings) { void ResolvePromiseObject(electron::util::Promise promise, int result, bool checkbox_checked) { - mate::Dictionary dict = mate::Dictionary::CreateEmpty(promise.isolate()); + v8::Isolate* isolate = promise.isolate(); + gin::Dictionary dict = gin::Dictionary::CreateEmpty(isolate); dict.Set("response", result); dict.Set("checkboxChecked", checkbox_checked); - promise.Resolve(dict.GetHandle()); + promise.Resolve(gin::ConvertToV8(isolate, dict)); } v8::Local ShowMessageBox( const electron::MessageBoxSettings& settings, - mate::Arguments* args) { + gin::Arguments* args) { v8::Isolate* isolate = args->isolate(); electron::util::Promise promise(isolate); v8::Local handle = promise.GetHandle(); @@ -52,7 +49,7 @@ v8::Local ShowMessageBox( } void ShowOpenDialogSync(const file_dialog::DialogSettings& settings, - mate::Arguments* args) { + gin::Arguments* args) { std::vector paths; if (file_dialog::ShowOpenDialogSync(settings, &paths)) args->Return(paths); @@ -60,7 +57,7 @@ void ShowOpenDialogSync(const file_dialog::DialogSettings& settings, v8::Local ShowOpenDialog( const file_dialog::DialogSettings& settings, - mate::Arguments* args) { + gin::Arguments* args) { electron::util::Promise promise(args->isolate()); v8::Local handle = promise.GetHandle(); file_dialog::ShowOpenDialog(settings, std::move(promise)); @@ -68,7 +65,7 @@ v8::Local ShowOpenDialog( } void ShowSaveDialogSync(const file_dialog::DialogSettings& settings, - mate::Arguments* args) { + gin::Arguments* args) { base::FilePath path; if (file_dialog::ShowSaveDialogSync(settings, &path)) args->Return(path); @@ -76,7 +73,7 @@ void ShowSaveDialogSync(const file_dialog::DialogSettings& settings, v8::Local ShowSaveDialog( const file_dialog::DialogSettings& settings, - mate::Arguments* args) { + gin::Arguments* args) { electron::util::Promise promise(args->isolate()); v8::Local handle = promise.GetHandle(); @@ -88,17 +85,34 @@ void Initialize(v8::Local exports, v8::Local unused, v8::Local context, void* priv) { - mate::Dictionary dict(context->GetIsolate(), exports); - dict.SetMethod("showMessageBoxSync", &ShowMessageBoxSync); - dict.SetMethod("showMessageBox", &ShowMessageBox); - dict.SetMethod("showErrorBox", &electron::ShowErrorBox); - dict.SetMethod("showOpenDialogSync", &ShowOpenDialogSync); - dict.SetMethod("showOpenDialog", &ShowOpenDialog); - dict.SetMethod("showSaveDialogSync", &ShowSaveDialogSync); - dict.SetMethod("showSaveDialog", &ShowSaveDialog); + v8::Isolate* isolate = context->GetIsolate(); + gin::Dictionary dict(isolate, exports); + dict.Set("showMessageBoxSync", + gin::ConvertCallbackToV8Leaked( + isolate, base::BindRepeating(&ShowMessageBoxSync))); + dict.Set("showMessageBox", + gin::ConvertCallbackToV8Leaked( + isolate, base::BindRepeating(&ShowMessageBox))); + dict.Set("showErrorBox", + gin::ConvertCallbackToV8Leaked( + isolate, base::BindRepeating(&electron::ShowErrorBox))); + dict.Set("showOpenDialogSync", + gin::ConvertCallbackToV8Leaked( + isolate, base::BindRepeating(&ShowOpenDialogSync))); + dict.Set("showOpenDialog", + gin::ConvertCallbackToV8Leaked( + isolate, base::BindRepeating(&ShowOpenDialog))); + dict.Set("showSaveDialogSync", + gin::ConvertCallbackToV8Leaked( + isolate, base::BindRepeating(&ShowSaveDialogSync))); + dict.Set("showSaveDialog", + gin::ConvertCallbackToV8Leaked( + isolate, base::BindRepeating(&ShowSaveDialog))); #if defined(OS_MACOSX) || defined(OS_WIN) - dict.SetMethod("showCertificateTrustDialog", - &certificate_trust::ShowCertificateTrust); + dict.Set("showCertificateTrustDialog", + gin::ConvertCallbackToV8Leaked( + isolate, + base::BindRepeating(&certificate_trust::ShowCertificateTrust))); #endif } diff --git a/shell/common/api/gin_utils.h b/shell/common/api/gin_utils.h new file mode 100644 index 000000000000..b3a3978ee8ee --- /dev/null +++ b/shell/common/api/gin_utils.h @@ -0,0 +1,27 @@ +// Copyright (c) 2019 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef SHELL_COMMON_API_GIN_UTILS_H_ +#define SHELL_COMMON_API_GIN_UTILS_H_ + +#include "gin/function_template.h" + +namespace gin { + +// NOTE: V8 caches FunctionTemplates. Therefore it is user's responsibility +// to ensure this function is called for one type only ONCE in the program's +// whole lifetime, otherwise we would have memory leak. +template +v8::Local ConvertCallbackToV8Leaked( + v8::Isolate* isolate, + const base::RepeatingCallback& callback) { + // LOG(WARNING) << "Leaky conversion from callback to V8 triggered."; + return gin::CreateFunctionTemplate(isolate, callback) + ->GetFunction(isolate->GetCurrentContext()) + .ToLocalChecked(); +} + +} // namespace gin + +#endif // SHELL_COMMON_API_GIN_UTILS_H_ diff --git a/shell/common/gin_converters/file_dialog_converter_gin_adapter.h b/shell/common/gin_converters/file_dialog_converter_gin_adapter.h new file mode 100644 index 000000000000..c451b90c5b36 --- /dev/null +++ b/shell/common/gin_converters/file_dialog_converter_gin_adapter.h @@ -0,0 +1,33 @@ +// Copyright (c) 2019 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef SHELL_COMMON_GIN_CONVERTERS_FILE_DIALOG_CONVERTER_GIN_ADAPTER_H_ +#define SHELL_COMMON_GIN_CONVERTERS_FILE_DIALOG_CONVERTER_GIN_ADAPTER_H_ + +#include "gin/converter.h" +#include "shell/common/native_mate_converters/file_dialog_converter.h" + +// TODO(deermichel): replace adapter with real implementation after removing +// mate +// -- this adapter forwards all conversions to the existing mate converter -- +// (other direction might be preferred, but this is safer for now :D) + +namespace gin { + +template <> +struct Converter { + static v8::Local ToV8(v8::Isolate* isolate, + const file_dialog::DialogSettings& in) { + return mate::ConvertToV8(isolate, in); + } + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + file_dialog::DialogSettings* out) { + return mate::ConvertFromV8(isolate, val, out); + } +}; + +} // namespace gin + +#endif // SHELL_COMMON_GIN_CONVERTERS_FILE_DIALOG_CONVERTER_GIN_ADAPTER_H_ diff --git a/shell/common/gin_converters/image_converter_gin_adapter.h b/shell/common/gin_converters/image_converter_gin_adapter.h new file mode 100644 index 000000000000..2ff62b21415e --- /dev/null +++ b/shell/common/gin_converters/image_converter_gin_adapter.h @@ -0,0 +1,29 @@ +// Copyright (c) 2019 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef SHELL_COMMON_GIN_CONVERTERS_IMAGE_CONVERTER_GIN_ADAPTER_H_ +#define SHELL_COMMON_GIN_CONVERTERS_IMAGE_CONVERTER_GIN_ADAPTER_H_ + +#include "gin/converter.h" +#include "shell/common/native_mate_converters/image_converter.h" + +// TODO(deermichel): replace adapter with real implementation after removing +// mate +// -- this adapter forwards all conversions to the existing mate converter -- +// (other direction might be preferred, but this is safer for now :D) + +namespace gin { + +template <> +struct Converter { + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + gfx::ImageSkia* out) { + return mate::ConvertFromV8(isolate, val, out); + } +}; + +} // namespace gin + +#endif // SHELL_COMMON_GIN_CONVERTERS_IMAGE_CONVERTER_GIN_ADAPTER_H_ diff --git a/shell/common/native_mate_converters/message_box_converter.cc b/shell/common/gin_converters/message_box_converter.cc similarity index 69% rename from shell/common/native_mate_converters/message_box_converter.cc rename to shell/common/gin_converters/message_box_converter.cc index 9ecff1ff9059..7653b1bdac73 100644 --- a/shell/common/native_mate_converters/message_box_converter.cc +++ b/shell/common/gin_converters/message_box_converter.cc @@ -1,21 +1,19 @@ -// Copyright (c) 2015 GitHub, Inc. +// Copyright (c) 2019 GitHub, Inc. // Use of this source code is governed by the MIT license that can be // found in the LICENSE file. -#include "shell/common/native_mate_converters/message_box_converter.h" +#include "shell/common/gin_converters/message_box_converter.h" -#include "native_mate/dictionary.h" -#include "shell/browser/api/atom_api_browser_window.h" -#include "shell/common/native_mate_converters/file_path_converter.h" -#include "shell/common/native_mate_converters/image_converter.h" +#include "gin/dictionary.h" +#include "shell/common/gin_converters/image_converter_gin_adapter.h" -namespace mate { +namespace gin { bool Converter::FromV8( v8::Isolate* isolate, v8::Local val, electron::MessageBoxSettings* out) { - mate::Dictionary dict; + gin::Dictionary dict(nullptr); int type = 0; if (!ConvertFromV8(isolate, val, &dict)) return false; @@ -35,4 +33,4 @@ bool Converter::FromV8( return true; } -} // namespace mate +} // namespace gin diff --git a/shell/common/gin_converters/message_box_converter.h b/shell/common/gin_converters/message_box_converter.h new file mode 100644 index 000000000000..c301728d45ab --- /dev/null +++ b/shell/common/gin_converters/message_box_converter.h @@ -0,0 +1,33 @@ +// Copyright (c) 2019 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef SHELL_COMMON_GIN_CONVERTERS_MESSAGE_BOX_CONVERTER_H_ +#define SHELL_COMMON_GIN_CONVERTERS_MESSAGE_BOX_CONVERTER_H_ + +#include "gin/converter.h" +#include "shell/browser/api/atom_api_browser_window.h" +#include "shell/browser/ui/message_box.h" + +namespace gin { + +// TODO(deermichel): remove adapter after removing mate +template <> +struct Converter { + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + electron::NativeWindow** out) { + return mate::ConvertFromV8(isolate, val, out); + } +}; + +template <> +struct Converter { + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + electron::MessageBoxSettings* out); +}; + +} // namespace gin + +#endif // SHELL_COMMON_GIN_CONVERTERS_MESSAGE_BOX_CONVERTER_H_ diff --git a/shell/common/gin_converters/net_converter_gin_adapter.h b/shell/common/gin_converters/net_converter_gin_adapter.h new file mode 100644 index 000000000000..8e490f230890 --- /dev/null +++ b/shell/common/gin_converters/net_converter_gin_adapter.h @@ -0,0 +1,34 @@ +// Copyright (c) 2019 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef SHELL_COMMON_GIN_CONVERTERS_NET_CONVERTER_GIN_ADAPTER_H_ +#define SHELL_COMMON_GIN_CONVERTERS_NET_CONVERTER_GIN_ADAPTER_H_ + +#include "gin/converter.h" +#include "shell/common/native_mate_converters/net_converter.h" + +// TODO(deermichel): replace adapter with real implementation after removing +// mate +// -- this adapter forwards all conversions to the existing mate converter -- +// (other direction might be preferred, but this is safer for now :D) + +namespace gin { + +template <> +struct Converter> { + static v8::Local ToV8( + v8::Isolate* isolate, + const scoped_refptr& val) { + return mate::ConvertToV8(isolate, val); + } + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + scoped_refptr* out) { + return mate::ConvertFromV8(isolate, val, out); + } +}; + +} // namespace gin + +#endif // SHELL_COMMON_GIN_CONVERTERS_NET_CONVERTER_GIN_ADAPTER_H_ diff --git a/shell/common/native_mate_converters/message_box_converter.h b/shell/common/native_mate_converters/message_box_converter.h deleted file mode 100644 index 8e9189fac241..000000000000 --- a/shell/common/native_mate_converters/message_box_converter.h +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright (c) 2019 GitHub, Inc. -// Use of this source code is governed by the MIT license that can be -// found in the LICENSE file. - -#ifndef SHELL_COMMON_NATIVE_MATE_CONVERTERS_MESSAGE_BOX_CONVERTER_H_ -#define SHELL_COMMON_NATIVE_MATE_CONVERTERS_MESSAGE_BOX_CONVERTER_H_ - -#include "native_mate/converter.h" -#include "shell/browser/ui/message_box.h" - -namespace mate { - -template <> -struct Converter { - static bool FromV8(v8::Isolate* isolate, - v8::Local val, - electron::MessageBoxSettings* out); -}; - -} // namespace mate - -#endif // SHELL_COMMON_NATIVE_MATE_CONVERTERS_MESSAGE_BOX_CONVERTER_H_