Browse Source

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
Micha Hanselmann 5 years ago
parent
commit
b80429ab7f

+ 6 - 2
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",

+ 40 - 26
shell/browser/api/atom_api_dialog.cc

@@ -6,18 +6,14 @@
 #include <utility>
 #include <vector>
 
-#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<v8::Promise> ShowMessageBox(
     const electron::MessageBoxSettings& settings,
-    mate::Arguments* args) {
+    gin::Arguments* args) {
   v8::Isolate* isolate = args->isolate();
   electron::util::Promise promise(isolate);
   v8::Local<v8::Promise> handle = promise.GetHandle();
@@ -52,7 +49,7 @@ v8::Local<v8::Promise> ShowMessageBox(
 }
 
 void ShowOpenDialogSync(const file_dialog::DialogSettings& settings,
-                        mate::Arguments* args) {
+                        gin::Arguments* args) {
   std::vector<base::FilePath> paths;
   if (file_dialog::ShowOpenDialogSync(settings, &paths))
     args->Return(paths);
@@ -60,7 +57,7 @@ void ShowOpenDialogSync(const file_dialog::DialogSettings& settings,
 
 v8::Local<v8::Promise> ShowOpenDialog(
     const file_dialog::DialogSettings& settings,
-    mate::Arguments* args) {
+    gin::Arguments* args) {
   electron::util::Promise promise(args->isolate());
   v8::Local<v8::Promise> handle = promise.GetHandle();
   file_dialog::ShowOpenDialog(settings, std::move(promise));
@@ -68,7 +65,7 @@ v8::Local<v8::Promise> 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<v8::Promise> ShowSaveDialog(
     const file_dialog::DialogSettings& settings,
-    mate::Arguments* args) {
+    gin::Arguments* args) {
   electron::util::Promise promise(args->isolate());
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
@@ -88,17 +85,34 @@ void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> 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
 }
 

+ 27 - 0
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 <typename Sig>
+v8::Local<v8::Function> ConvertCallbackToV8Leaked(
+    v8::Isolate* isolate,
+    const base::RepeatingCallback<Sig>& 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_

+ 33 - 0
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<file_dialog::DialogSettings> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const file_dialog::DialogSettings& in) {
+    return mate::ConvertToV8(isolate, in);
+  }
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     file_dialog::DialogSettings* out) {
+    return mate::ConvertFromV8(isolate, val, out);
+  }
+};
+
+}  // namespace gin
+
+#endif  // SHELL_COMMON_GIN_CONVERTERS_FILE_DIALOG_CONVERTER_GIN_ADAPTER_H_

+ 29 - 0
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<gfx::ImageSkia> {
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     gfx::ImageSkia* out) {
+    return mate::ConvertFromV8(isolate, val, out);
+  }
+};
+
+}  // namespace gin
+
+#endif  // SHELL_COMMON_GIN_CONVERTERS_IMAGE_CONVERTER_GIN_ADAPTER_H_

+ 7 - 9
shell/common/native_mate_converters/message_box_converter.cc → 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<electron::MessageBoxSettings>::FromV8(
     v8::Isolate* isolate,
     v8::Local<v8::Value> 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<electron::MessageBoxSettings>::FromV8(
   return true;
 }
 
-}  // namespace mate
+}  // namespace gin

+ 33 - 0
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<electron::NativeWindow*> {
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     electron::NativeWindow** out) {
+    return mate::ConvertFromV8(isolate, val, out);
+  }
+};
+
+template <>
+struct Converter<electron::MessageBoxSettings> {
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     electron::MessageBoxSettings* out);
+};
+
+}  // namespace gin
+
+#endif  // SHELL_COMMON_GIN_CONVERTERS_MESSAGE_BOX_CONVERTER_H_

+ 34 - 0
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<scoped_refptr<net::X509Certificate>> {
+  static v8::Local<v8::Value> ToV8(
+      v8::Isolate* isolate,
+      const scoped_refptr<net::X509Certificate>& val) {
+    return mate::ConvertToV8(isolate, val);
+  }
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     scoped_refptr<net::X509Certificate>* out) {
+    return mate::ConvertFromV8(isolate, val, out);
+  }
+};
+
+}  // namespace gin
+
+#endif  // SHELL_COMMON_GIN_CONVERTERS_NET_CONVERTER_GIN_ADAPTER_H_

+ 0 - 22
shell/common/native_mate_converters/message_box_converter.h

@@ -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<electron::MessageBoxSettings> {
-  static bool FromV8(v8::Isolate* isolate,
-                     v8::Local<v8::Value> val,
-                     electron::MessageBoxSettings* out);
-};
-
-}  // namespace mate
-
-#endif  // SHELL_COMMON_NATIVE_MATE_CONVERTERS_MESSAGE_BOX_CONVERTER_H_