Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: remove native_mate (Part 3) #20131

Merged
merged 13 commits into from
Sep 6, 2019
Merged

chore: remove native_mate (Part 3) #20131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
954eb41
use gin converter in atom_api_menu
zcbenz Sep 4, 2019
1d86ccb
please only put necessary includes in header
zcbenz Sep 4, 2019
4b8ac4d
remove some simple uses of callback_converter_deprecated.h
zcbenz Sep 4, 2019
229278c
use gin callback converter in file_dialog code
zcbenz Sep 4, 2019
83b9a37
use gin in ErrorThrower
zcbenz Sep 4, 2019
cd13e4e
use gin in atom_bundle_mover
zcbenz Sep 4, 2019
725a400
fix mistake in node stream
zcbenz Sep 5, 2019
b938e0a
deprecate native_mate version of event_emitter_caller
zcbenz Sep 5, 2019
6c9d148
use gin in node_bindings
zcbenz Sep 5, 2019
649f767
remove usages of native_mate event_emitter_caller.h except for EventE…
zcbenz Sep 5, 2019
7f596f6
fix compilation on Windows
zcbenz Sep 5, 2019
4422aa1
gin::Arguments behaves differently on GetNext
zcbenz Sep 5, 2019
025f0ea
just use StringToV8
zcbenz Sep 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions filenames.gni
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,8 @@ filenames = {
"shell/common/api/electron_bindings.cc",
"shell/common/api/electron_bindings.h",
"shell/common/api/constructor.h",
"shell/common/api/event_emitter_caller.cc",
"shell/common/api/event_emitter_caller.h",
"shell/common/api/event_emitter_caller_deprecated.cc",
"shell/common/api/event_emitter_caller_deprecated.h",
"shell/common/api/features.cc",
"shell/common/api/locker.cc",
"shell/common/api/locker.h",
Expand Down Expand Up @@ -494,6 +494,10 @@ filenames = {
"shell/common/gin_helper/destroyable.cc",
"shell/common/gin_helper/destroyable.h",
"shell/common/gin_helper/dictionary.h",
"shell/common/gin_helper/error_thrower.cc",
"shell/common/gin_helper/error_thrower.h",
"shell/common/gin_helper/event_emitter_caller.cc",
"shell/common/gin_helper/event_emitter_caller.h",
"shell/common/gin_helper/function_template.cc",
"shell/common/gin_helper/function_template.h",
"shell/common/heap_snapshot.cc",
Expand All @@ -503,8 +507,6 @@ filenames = {
"shell/common/keyboard_util.h",
"shell/common/deprecate_util.cc",
"shell/common/deprecate_util.h",
"shell/common/error_util.cc",
"shell/common/error_util.h",
"shell/common/mouse_util.cc",
"shell/common/mouse_util.h",
"shell/common/mac/main_application_bundle.h",
Expand Down
1 change: 1 addition & 0 deletions native_mate/native_mate/arguments.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ class Arguments {
v8::Local<v8::Value> ThrowTypeError(const std::string& message) const;

v8::Isolate* isolate() const { return isolate_; }
const v8::FunctionCallbackInfo<v8::Value>& info() const { return *info_; }

private:
v8::Isolate* isolate_ = nullptr;
Expand Down
8 changes: 3 additions & 5 deletions native_mate/native_mate/function_template.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@
#ifndef NATIVE_MATE_NATIVE_MATE_FUNCTION_TEMPLATE_H_
#define NATIVE_MATE_NATIVE_MATE_FUNCTION_TEMPLATE_H_

#include "../shell/common/error_util.h"
#include "../shell/common/gin_helper/destroyable.h"
#include "../shell/common/gin_helper/error_thrower.h"
#include "base/callback.h"
#include "base/logging.h"
#include "native_mate/arguments.h"
#include "native_mate/wrappable_base.h"
#include "v8/include/v8.h"

// =============================== NOTICE ===============================
// Do not add code here, native_mate is being removed. Any new code
Expand Down Expand Up @@ -122,8 +120,8 @@ inline bool GetNextArgument(Arguments* args,
inline bool GetNextArgument(Arguments* args,
int create_flags,
bool is_first,
electron::util::ErrorThrower* result) {
*result = electron::util::ErrorThrower(args->isolate());
gin_helper::ErrorThrower* result) {
*result = gin_helper::ErrorThrower(args->isolate());
return true;
}

Expand Down
1 change: 1 addition & 0 deletions patches/chromium/.patches
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ web_contents.patch
webview_cross_drag.patch
disable_user_gesture_requirement_for_beforeunload_dialogs.patch
gin_enable_disable_v8_platform.patch
gin_dictionary_default_constructor.patch
blink-worker-enable-csp-in-file-scheme.patch
disable-redraw-lock.patch
v8_context_snapshot_generator.patch
Expand Down
37 changes: 37 additions & 0 deletions patches/chromium/gin_dictionary_default_constructor.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Thu, 4 Oct 2018 14:57:02 -0700
Subject: gin_dictionary_default_constructor.patch

Add default constructor for gin::Dictionary.

This is required for automatically converting arguments for functions that
take gin::Dictionary as parameter.

diff --git a/gin/dictionary.cc b/gin/dictionary.cc
index 95e00072700c..7643347890a5 100644
--- a/gin/dictionary.cc
+++ b/gin/dictionary.cc
@@ -6,6 +6,10 @@

namespace gin {

+Dictionary::Dictionary()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For such call sites, can we resort to having gin::Arguments as the argument and extract gin::Dictionary from that in the callback, which would avoid this patch. ex:

void AtomDownloadManagerDelegate::OnDownloadSaveDialogDone(..., gin::Arguments* args) {
  gin::Dictionary result(args->isolate());
  // extract result from args.
}

Ideally it would be better if we rewrite ArgumentsHolder to not expect a default constructor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an alternative (because I don't like the idea of duplicate that dictionary construction code everywhere). Can we just extend gin::Dictionary into electron::util::Dictionary and implement the empty constructor there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have an extended gin_helper::Dictionary and it should be possible to use it to avoid patching gin. I'll put it in the next PR.

+ : isolate_(nullptr) {
+}
+
Dictionary::Dictionary(v8::Isolate* isolate)
: isolate_(isolate) {
}
diff --git a/gin/dictionary.h b/gin/dictionary.h
index 2645d328b4c1..43b227dd7e48 100644
--- a/gin/dictionary.h
+++ b/gin/dictionary.h
@@ -24,6 +24,7 @@ namespace gin {
//
class GIN_EXPORT Dictionary {
public:
+ Dictionary();
explicit Dictionary(v8::Isolate* isolate);
Dictionary(v8::Isolate* isolate, v8::Local<v8::Object> object);
Dictionary(const Dictionary& other);
1 change: 1 addition & 0 deletions script/lint.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const BLACKLIST = new Set([
['shell', 'browser', 'mac', 'atom_application_delegate.h'],
['shell', 'browser', 'resources', 'win', 'resource.h'],
['shell', 'browser', 'notifications', 'mac', 'notification_center_delegate.h'],
['shell', 'browser', 'ui', 'cocoa', 'atom_bundle_mover.h'],
['shell', 'browser', 'ui', 'cocoa', 'atom_menu_controller.h'],
['shell', 'browser', 'ui', 'cocoa', 'atom_ns_window.h'],
['shell', 'browser', 'ui', 'cocoa', 'atom_ns_window_delegate.h'],
Expand Down
23 changes: 13 additions & 10 deletions shell/browser/api/atom_api_app.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "content/public/browser/gpu_data_manager.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/common/content_switches.h"
#include "gin/arguments.h"
#include "media/audio/audio_manager.h"
#include "native_mate/object_template_builder.h"
#include "net/ssl/client_cert_identity.h"
Expand Down Expand Up @@ -846,7 +847,7 @@ void App::SetAppPath(const base::FilePath& app_path) {
}

#if !defined(OS_MACOSX)
void App::SetAppLogsPath(util::ErrorThrower thrower,
void App::SetAppLogsPath(gin_helper::ErrorThrower thrower,
base::Optional<base::FilePath> custom_path) {
if (custom_path.has_value()) {
if (!custom_path->IsAbsolute()) {
Expand All @@ -865,7 +866,7 @@ void App::SetAppLogsPath(util::ErrorThrower thrower,
}
#endif

base::FilePath App::GetPath(util::ErrorThrower thrower,
base::FilePath App::GetPath(gin_helper::ErrorThrower thrower,
const std::string& name) {
bool succeed = false;
base::FilePath path;
Expand All @@ -888,7 +889,7 @@ base::FilePath App::GetPath(util::ErrorThrower thrower,
return path;
}

void App::SetPath(util::ErrorThrower thrower,
void App::SetPath(gin_helper::ErrorThrower thrower,
const std::string& name,
const base::FilePath& path) {
if (!path.IsAbsolute()) {
Expand Down Expand Up @@ -1031,7 +1032,7 @@ bool App::Relaunch(mate::Arguments* js_args) {
return relauncher::RelaunchApp(argv);
}

void App::DisableHardwareAcceleration(util::ErrorThrower thrower) {
void App::DisableHardwareAcceleration(gin_helper::ErrorThrower thrower) {
if (Browser::Get()->is_ready()) {
thrower.ThrowError(
"app.disableHardwareAcceleration() can only be called "
Expand All @@ -1041,7 +1042,7 @@ void App::DisableHardwareAcceleration(util::ErrorThrower thrower) {
content::GpuDataManager::GetInstance()->DisableHardwareAcceleration();
}

void App::DisableDomainBlockingFor3DAPIs(util::ErrorThrower thrower) {
void App::DisableDomainBlockingFor3DAPIs(gin_helper::ErrorThrower thrower) {
if (Browser::Get()->is_ready()) {
thrower.ThrowError(
"app.disableDomainBlockingFor3DAPIs() can only be called "
Expand All @@ -1057,7 +1058,7 @@ bool App::IsAccessibilitySupportEnabled() {
return ax_state->IsAccessibleBrowser();
}

void App::SetAccessibilitySupportEnabled(util::ErrorThrower thrower,
void App::SetAccessibilitySupportEnabled(gin_helper::ErrorThrower thrower,
bool enabled) {
if (!Browser::Get()->is_ready()) {
thrower.ThrowError(
Expand Down Expand Up @@ -1314,7 +1315,7 @@ static void RemoveNoSandboxSwitch(base::CommandLine* command_line) {
}
}

void App::EnableSandbox(util::ErrorThrower thrower) {
void App::EnableSandbox(gin_helper::ErrorThrower thrower) {
if (Browser::Get()->is_ready()) {
thrower.ThrowError(
"app.enableSandbox() can only be called "
Expand Down Expand Up @@ -1343,12 +1344,14 @@ bool App::CanBrowserClientUseCustomSiteInstance() {
}

#if defined(OS_MACOSX)
bool App::MoveToApplicationsFolder(mate::Arguments* args) {
return ui::cocoa::AtomBundleMover::Move(args);
bool App::MoveToApplicationsFolder(gin_helper::ErrorThrower thrower,
mate::Arguments* args) {
gin::Arguments gin_args(args->info());
return AtomBundleMover::Move(thrower, &gin_args);
}

bool App::IsInApplicationsFolder() {
return ui::cocoa::AtomBundleMover::IsCurrentAppInApplicationsFolder();
return AtomBundleMover::IsCurrentAppInApplicationsFolder();
}

int DockBounce(mate::Arguments* args) {
Expand Down
22 changes: 12 additions & 10 deletions shell/browser/api/atom_api_app.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@
#include "shell/browser/atom_browser_client.h"
#include "shell/browser/browser.h"
#include "shell/browser/browser_observer.h"
#include "shell/common/error_util.h"
#include "shell/common/native_mate_converters/callback_converter_deprecated.h"
#include "shell/common/gin_helper/error_thrower.h"
#include "shell/common/promise_util.h"

#if defined(USE_NSS_CERTS)
Expand Down Expand Up @@ -165,12 +164,13 @@ class App : public AtomBrowserClient::Delegate,
void ChildProcessLaunched(int process_type, base::ProcessHandle handle);
void ChildProcessDisconnected(base::ProcessId pid);

void SetAppLogsPath(util::ErrorThrower thrower,
void SetAppLogsPath(gin_helper::ErrorThrower thrower,
base::Optional<base::FilePath> custom_path);

// Get/Set the pre-defined path in PathService.
base::FilePath GetPath(util::ErrorThrower thrower, const std::string& name);
void SetPath(util::ErrorThrower thrower,
base::FilePath GetPath(gin_helper::ErrorThrower thrower,
const std::string& name);
void SetPath(gin_helper::ErrorThrower thrower,
const std::string& name,
const base::FilePath& path);

Expand All @@ -183,10 +183,11 @@ class App : public AtomBrowserClient::Delegate,
bool RequestSingleInstanceLock();
void ReleaseSingleInstanceLock();
bool Relaunch(mate::Arguments* args);
void DisableHardwareAcceleration(util::ErrorThrower thrower);
void DisableDomainBlockingFor3DAPIs(util::ErrorThrower thrower);
void DisableHardwareAcceleration(gin_helper::ErrorThrower thrower);
void DisableDomainBlockingFor3DAPIs(gin_helper::ErrorThrower thrower);
bool IsAccessibilitySupportEnabled();
void SetAccessibilitySupportEnabled(util::ErrorThrower thrower, bool enabled);
void SetAccessibilitySupportEnabled(gin_helper::ErrorThrower thrower,
bool enabled);
Browser::LoginItemSettings GetLoginItemSettings(mate::Arguments* args);
#if defined(USE_NSS_CERTS)
void ImportCertificate(const base::DictionaryValue& options,
Expand All @@ -199,14 +200,15 @@ class App : public AtomBrowserClient::Delegate,
v8::Local<v8::Value> GetGPUFeatureStatus(v8::Isolate* isolate);
v8::Local<v8::Promise> GetGPUInfo(v8::Isolate* isolate,
const std::string& info_type);
void EnableSandbox(util::ErrorThrower thrower);
void EnableSandbox(gin_helper::ErrorThrower thrower);
void SetUserAgentFallback(const std::string& user_agent);
std::string GetUserAgentFallback();
void SetBrowserClientCanUseCustomSiteInstance(bool should_disable);
bool CanBrowserClientUseCustomSiteInstance();

#if defined(OS_MACOSX)
bool MoveToApplicationsFolder(mate::Arguments* args);
bool MoveToApplicationsFolder(gin_helper::ErrorThrower,
mate::Arguments* args);
bool IsInApplicationsFolder();
v8::Local<v8::Value> GetDockAPI(v8::Isolate* isolate);
v8::Global<v8::Value> dock_;
Expand Down
2 changes: 1 addition & 1 deletion shell/browser/api/atom_api_app_mac.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace api {

void App::SetAppLogsPath(util::ErrorThrower thrower,
void App::SetAppLogsPath(gin_helper::ErrorThrower thrower,
base::Optional<base::FilePath> custom_path) {
if (custom_path.has_value()) {
if (!custom_path->IsAbsolute()) {
Expand Down
10 changes: 6 additions & 4 deletions shell/browser/api/atom_api_auto_updater.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@
#include "shell/browser/browser.h"
#include "shell/browser/native_window.h"
#include "shell/browser/window_list.h"
#include "shell/common/api/event_emitter_caller.h"
#include "shell/common/native_mate_converters/callback_converter_deprecated.h"
#include "shell/common/gin_helper/event_emitter_caller.h"
#include "shell/common/node_includes.h"

// TODO(zcbenz): Remove this after removing mate::ObjectTemplateBuilder.
#include "shell/common/native_mate_converters/callback_converter_deprecated.h"

namespace mate {

template <>
Expand Down Expand Up @@ -48,7 +50,7 @@ void AutoUpdater::OnError(const std::string& message) {
v8::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
auto error = v8::Exception::Error(mate::StringToV8(isolate(), message));
mate::EmitEvent(
gin_helper::EmitEvent(
isolate(), GetWrapper(), "error",
error->ToObject(isolate()->GetCurrentContext()).ToLocalChecked(),
// Message is also emitted to keep compatibility with old code.
Expand Down Expand Up @@ -76,7 +78,7 @@ void AutoUpdater::OnError(const std::string& message,
mate::StringToV8(isolate(), domain))
.Check();

mate::EmitEvent(isolate(), GetWrapper(), "error", errorObject, message);
gin_helper::EmitEvent(isolate(), GetWrapper(), "error", errorObject, message);
}

void AutoUpdater::OnCheckingForUpdate() {
Expand Down
1 change: 0 additions & 1 deletion shell/browser/api/atom_api_browser_window.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include "shell/browser/window_list.h"
#include "shell/common/api/constructor.h"
#include "shell/common/color_util.h"
#include "shell/common/native_mate_converters/callback_converter_deprecated.h"
#include "shell/common/native_mate_converters/value_converter.h"
#include "shell/common/node_includes.h"
#include "shell/common/options_switches.h"
Expand Down
1 change: 0 additions & 1 deletion shell/browser/api/atom_api_cookies.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include "net/cookies/cookie_util.h"
#include "shell/browser/atom_browser_context.h"
#include "shell/browser/cookie_change_notifier.h"
#include "shell/common/native_mate_converters/callback_converter_deprecated.h"
#include "shell/common/native_mate_converters/gurl_converter.h"
#include "shell/common/native_mate_converters/value_converter.h"

Expand Down
1 change: 0 additions & 1 deletion shell/browser/api/atom_api_debugger.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "content/public/browser/devtools_agent_host.h"
#include "content/public/browser/web_contents.h"
#include "native_mate/dictionary.h"
#include "shell/common/native_mate_converters/callback_converter_deprecated.h"
#include "shell/common/native_mate_converters/value_converter.h"
#include "shell/common/node_includes.h"

Expand Down
5 changes: 3 additions & 2 deletions shell/browser/api/atom_api_dialog.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "shell/browser/ui/message_box.h"
#include "shell/common/gin_converters/callback_converter.h"
#include "shell/common/gin_converters/file_dialog_converter.h"
#include "shell/common/gin_converters/file_path_converter.h"
#include "shell/common/gin_converters/message_box_converter.h"
#include "shell/common/gin_converters/native_window_converter.h"
#include "shell/common/gin_converters/net_converter.h"
Expand Down Expand Up @@ -59,7 +60,7 @@ void ShowOpenDialogSync(const file_dialog::DialogSettings& settings,
v8::Local<v8::Promise> ShowOpenDialog(
const file_dialog::DialogSettings& settings,
gin::Arguments* args) {
electron::util::Promise<mate::Dictionary> promise(args->isolate());
electron::util::Promise<gin::Dictionary> promise(args->isolate());
v8::Local<v8::Promise> handle = promise.GetHandle();
file_dialog::ShowOpenDialog(settings, std::move(promise));
return handle;
Expand All @@ -75,7 +76,7 @@ void ShowSaveDialogSync(const file_dialog::DialogSettings& settings,
v8::Local<v8::Promise> ShowSaveDialog(
const file_dialog::DialogSettings& settings,
gin::Arguments* args) {
electron::util::Promise<mate::Dictionary> promise(args->isolate());
electron::util::Promise<gin::Dictionary> promise(args->isolate());
v8::Local<v8::Promise> handle = promise.GetHandle();

file_dialog::ShowSaveDialog(settings, std::move(promise));
Expand Down
1 change: 0 additions & 1 deletion shell/browser/api/atom_api_download_item.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "native_mate/dictionary.h"
#include "net/base/filename_util.h"
#include "shell/browser/atom_browser_main_parts.h"
#include "shell/common/native_mate_converters/callback_converter_deprecated.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/gurl_converter.h"
Expand Down
1 change: 0 additions & 1 deletion shell/browser/api/atom_api_in_app_purchase.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <vector>

#include "native_mate/dictionary.h"
#include "shell/common/native_mate_converters/callback_converter_deprecated.h"
#include "shell/common/node_includes.h"

namespace mate {
Expand Down