From 3949f0bd5081984f1dc9ebc62e8b36b5deaac82b Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Wed, 1 May 2019 15:19:11 -0700 Subject: [PATCH] refactor: convert crash reporter to gin (#17952) --- atom/browser/api/atom_api_web_contents.cc | 1 + atom/browser/auto_updater_mac.mm | 2 +- atom/common/api/atom_api_crash_reporter.cc | 61 +++++++--------- atom/common/crash_reporter/crash_reporter.cc | 1 + atom/common/gin_util.h | 29 ++++++++ .../file_path_converter.h | 20 +++++- .../native_mate_converters/map_converter.h | 70 +++++++++++++++++++ .../string16_converter.h | 25 ++++++- filenames.gni | 2 + native_mate/native_mate/converter.h | 35 ---------- 10 files changed, 174 insertions(+), 72 deletions(-) create mode 100644 atom/common/gin_util.h create mode 100644 atom/common/native_mate_converters/map_converter.h diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 247adfcbfac..5ec1a9aa680 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -40,6 +40,7 @@ #include "atom/common/native_mate_converters/gfx_converter.h" #include "atom/common/native_mate_converters/gurl_converter.h" #include "atom/common/native_mate_converters/image_converter.h" +#include "atom/common/native_mate_converters/map_converter.h" #include "atom/common/native_mate_converters/net_converter.h" #include "atom/common/native_mate_converters/network_converter.h" #include "atom/common/native_mate_converters/once_callback.h" diff --git a/atom/browser/auto_updater_mac.mm b/atom/browser/auto_updater_mac.mm index ff8ffb86373..80bb61b480c 100644 --- a/atom/browser/auto_updater_mac.mm +++ b/atom/browser/auto_updater_mac.mm @@ -10,11 +10,11 @@ #import #include "atom/browser/browser.h" +#include "atom/common/native_mate_converters/map_converter.h" #include "atom/common/native_mate_converters/value_converter.h" #include "base/bind.h" #include "base/strings/sys_string_conversions.h" #include "base/time/time.h" -#include "native_mate/converter.h" #include "native_mate/dictionary.h" namespace auto_updater { diff --git a/atom/common/api/atom_api_crash_reporter.cc b/atom/common/api/atom_api_crash_reporter.cc index fab4d03addd..7bc05148ed1 100644 --- a/atom/common/api/atom_api_crash_reporter.cc +++ b/atom/common/api/atom_api_crash_reporter.cc @@ -6,65 +6,58 @@ #include #include "atom/common/crash_reporter/crash_reporter.h" +#include "atom/common/gin_util.h" #include "atom/common/native_mate_converters/file_path_converter.h" +#include "atom/common/native_mate_converters/map_converter.h" #include "base/bind.h" -#include "native_mate/dictionary.h" +#include "gin/data_object_builder.h" +#include "gin/dictionary.h" #include "atom/common/node_includes.h" using crash_reporter::CrashReporter; -namespace mate { +namespace gin { template <> struct Converter { static v8::Local ToV8( v8::Isolate* isolate, const CrashReporter::UploadReportResult& reports) { - mate::Dictionary dict(isolate, v8::Object::New(isolate)); - dict.Set("date", + return gin::DataObjectBuilder(isolate) + .Set("date", v8::Date::New(isolate->GetCurrentContext(), reports.first * 1000.0) - .ToLocalChecked()); - dict.Set("id", reports.second); - return dict.GetHandle(); + .ToLocalChecked()) + .Set("id", reports.second) + .Build(); } }; -} // namespace mate +} // namespace gin namespace { -void AddExtraParameter(const std::string& key, const std::string& value) { - CrashReporter::GetInstance()->AddExtraParameter(key, value); -} - -void RemoveExtraParameter(const std::string& key) { - CrashReporter::GetInstance()->RemoveExtraParameter(key); -} - -std::map GetParameters() { - return CrashReporter::GetInstance()->GetParameters(); -} - void Initialize(v8::Local exports, v8::Local unused, v8::Local context, void* priv) { - mate::Dictionary dict(context->GetIsolate(), exports); + using gin_util::SetMethod; auto reporter = base::Unretained(CrashReporter::GetInstance()); - dict.SetMethod("start", base::BindRepeating(&CrashReporter::Start, reporter)); - dict.SetMethod("addExtraParameter", &AddExtraParameter); - dict.SetMethod("removeExtraParameter", &RemoveExtraParameter); - dict.SetMethod("getParameters", &GetParameters); - dict.SetMethod( - "getUploadedReports", - base::BindRepeating(&CrashReporter::GetUploadedReports, reporter)); - dict.SetMethod( - "setUploadToServer", - base::BindRepeating(&CrashReporter::SetUploadToServer, reporter)); - dict.SetMethod( - "getUploadToServer", - base::BindRepeating(&CrashReporter::GetUploadToServer, reporter)); + SetMethod(exports, "start", + base::BindRepeating(&CrashReporter::Start, reporter)); + SetMethod(exports, "addExtraParameter", + base::BindRepeating(&CrashReporter::AddExtraParameter, reporter)); + SetMethod( + exports, "removeExtraParameter", + base::BindRepeating(&CrashReporter::RemoveExtraParameter, reporter)); + SetMethod(exports, "getParameters", + base::BindRepeating(&CrashReporter::GetParameters, reporter)); + SetMethod(exports, "getUploadedReports", + base::BindRepeating(&CrashReporter::GetUploadedReports, reporter)); + SetMethod(exports, "setUploadToServer", + base::BindRepeating(&CrashReporter::SetUploadToServer, reporter)); + SetMethod(exports, "getUploadToServer", + base::BindRepeating(&CrashReporter::GetUploadToServer, reporter)); } } // namespace diff --git a/atom/common/crash_reporter/crash_reporter.cc b/atom/common/crash_reporter/crash_reporter.cc index f804f983e59..ecb4a847374 100644 --- a/atom/common/crash_reporter/crash_reporter.cc +++ b/atom/common/crash_reporter/crash_reporter.cc @@ -7,6 +7,7 @@ #include "atom/browser/browser.h" #include "atom/common/atom_version.h" #include "atom/common/native_mate_converters/file_path_converter.h" +#include "atom/common/native_mate_converters/map_converter.h" #include "base/command_line.h" #include "base/files/file_util.h" #include "base/strings/string_number_conversions.h" diff --git a/atom/common/gin_util.h b/atom/common/gin_util.h new file mode 100644 index 00000000000..d2d928f7954 --- /dev/null +++ b/atom/common/gin_util.h @@ -0,0 +1,29 @@ +// Copyright (c) 2018 Slack Technologies, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_COMMON_GIN_UTIL_H_ +#define ATOM_COMMON_GIN_UTIL_H_ + +#include "gin/converter.h" +#include "gin/function_template.h" + +namespace gin_util { + +template +bool SetMethod(v8::Local recv, + const base::StringPiece& key, + const T& callback) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + auto context = isolate->GetCurrentContext(); + return recv + ->Set(context, gin::StringToV8(isolate, key), + gin::CreateFunctionTemplate(isolate, callback) + ->GetFunction(context) + .ToLocalChecked()) + .ToChecked(); +} + +} // namespace gin_util + +#endif // ATOM_COMMON_GIN_UTIL_H_ diff --git a/atom/common/native_mate_converters/file_path_converter.h b/atom/common/native_mate_converters/file_path_converter.h index c283d3370a6..87a77c16d45 100644 --- a/atom/common/native_mate_converters/file_path_converter.h +++ b/atom/common/native_mate_converters/file_path_converter.h @@ -10,7 +10,7 @@ #include "atom/common/native_mate_converters/string16_converter.h" #include "base/files/file_path.h" -namespace mate { +namespace gin { template <> struct Converter { @@ -34,6 +34,24 @@ struct Converter { } }; +} // namespace gin + +namespace mate { + +template <> +struct Converter { + static v8::Local ToV8(v8::Isolate* isolate, + const base::FilePath& val) { + return gin::Converter::ToV8(isolate, + val.value()); + } + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + base::FilePath* out) { + return gin::Converter::FromV8(isolate, val, out); + } +}; + } // namespace mate #endif // ATOM_COMMON_NATIVE_MATE_CONVERTERS_FILE_PATH_CONVERTER_H_ diff --git a/atom/common/native_mate_converters/map_converter.h b/atom/common/native_mate_converters/map_converter.h new file mode 100644 index 00000000000..a06e3ec80d6 --- /dev/null +++ b/atom/common/native_mate_converters/map_converter.h @@ -0,0 +1,70 @@ +// Copyright (c) 2019 Slack Technologies, 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_MAP_CONVERTER_H_ +#define ATOM_COMMON_NATIVE_MATE_CONVERTERS_MAP_CONVERTER_H_ + +#include +#include +#include + +#include "gin/converter.h" + +namespace gin { + +template +struct Converter> { + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + std::map* out) { + if (!val->IsObject()) + return false; + + v8::Local context = isolate->GetCurrentContext(); + v8::Local dict = val->ToObject(context).ToLocalChecked(); + v8::Local keys = + dict->GetOwnPropertyNames(context).ToLocalChecked(); + for (uint32_t i = 0; i < keys->Length(); ++i) { + v8::Local key = keys->Get(context, i).ToLocalChecked(); + T value; + if (Converter::FromV8( + isolate, dict->Get(context, key).ToLocalChecked(), &value)) + (*out)[gin::V8ToString(isolate, key)] = std::move(value); + } + return true; + } + static v8::Local ToV8(v8::Isolate* isolate, + const std::map& val) { + v8::Local result = v8::Object::New(isolate); + auto context = isolate->GetCurrentContext(); + for (auto i = val.begin(); i != val.end(); i++) { + result + ->Set(context, Converter::ToV8(isolate, i->first), + Converter::ToV8(isolate, i->second)) + .Check(); + } + return result; + } +}; + +} // namespace gin + +namespace mate { + +template +struct Converter> { + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + std::map* out) { + return gin::Converter>::FromV8(isolate, val, out); + } + static v8::Local ToV8(v8::Isolate* isolate, + const std::map& val) { + return gin::Converter>::ToV8(isolate, val); + } +}; + +} // namespace mate + +#endif // ATOM_COMMON_NATIVE_MATE_CONVERTERS_MAP_CONVERTER_H_ diff --git a/atom/common/native_mate_converters/string16_converter.h b/atom/common/native_mate_converters/string16_converter.h index 7cadcbbcb3c..14ef5c11009 100644 --- a/atom/common/native_mate_converters/string16_converter.h +++ b/atom/common/native_mate_converters/string16_converter.h @@ -6,9 +6,10 @@ #define ATOM_COMMON_NATIVE_MATE_CONVERTERS_STRING16_CONVERTER_H_ #include "base/strings/string16.h" +#include "gin/converter.h" #include "native_mate/converter.h" -namespace mate { +namespace gin { template <> struct Converter { @@ -36,6 +37,28 @@ inline v8::Local StringToV8(v8::Isolate* isolate, return ConvertToV8(isolate, input).As(); } +} // namespace gin + +namespace mate { + +template <> +struct Converter { + static v8::Local ToV8(v8::Isolate* isolate, + const base::string16& val) { + return gin::Converter::ToV8(isolate, val); + } + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + base::string16* out) { + return gin::Converter::FromV8(isolate, val, out); + } +}; + +inline v8::Local StringToV8(v8::Isolate* isolate, + const base::string16& input) { + return ConvertToV8(isolate, input).As(); +} + } // namespace mate #endif // ATOM_COMMON_NATIVE_MATE_CONVERTERS_STRING16_CONVERTER_H_ diff --git a/filenames.gni b/filenames.gni index 06292e72e0b..10bf1f91ce8 100644 --- a/filenames.gni +++ b/filenames.gni @@ -605,6 +605,7 @@ filenames = { "atom/common/crash_reporter/win/crash_service_main.h", "atom/common/draggable_region.cc", "atom/common/draggable_region.h", + "atom/common/gin_util.h", "atom/common/heap_snapshot.cc", "atom/common/heap_snapshot.h", "atom/common/key_weak_map.h", @@ -630,6 +631,7 @@ filenames = { "atom/common/native_mate_converters/gurl_converter.h", "atom/common/native_mate_converters/image_converter.cc", "atom/common/native_mate_converters/image_converter.h", + "atom/common/native_mate_converters/map_converter.h", "atom/common/native_mate_converters/net_converter.cc", "atom/common/native_mate_converters/net_converter.h", "atom/common/native_mate_converters/network_converter.cc", diff --git a/native_mate/native_mate/converter.h b/native_mate/native_mate/converter.h index c13afbf1058..55ebbe2104d 100644 --- a/native_mate/native_mate/converter.h +++ b/native_mate/native_mate/converter.h @@ -277,41 +277,6 @@ struct Converter> { } }; -template -struct Converter> { - static bool FromV8(v8::Isolate* isolate, - v8::Local val, - std::map* out) { - if (!val->IsObject()) - return false; - - v8::Local context = isolate->GetCurrentContext(); - v8::Local dict = val->ToObject(context).ToLocalChecked(); - v8::Local keys = - dict->GetOwnPropertyNames(context).ToLocalChecked(); - for (uint32_t i = 0; i < keys->Length(); ++i) { - v8::Local key = keys->Get(context, i).ToLocalChecked(); - T value; - if (Converter::FromV8( - isolate, dict->Get(context, key).ToLocalChecked(), &value)) - (*out)[gin::V8ToString(isolate, key)] = std::move(value); - } - return true; - } - static v8::Local ToV8(v8::Isolate* isolate, - const std::map& val) { - v8::Local result = v8::Object::New(isolate); - auto context = isolate->GetCurrentContext(); - for (auto i = val.begin(); i != val.end(); i++) { - result - ->Set(context, Converter::ToV8(isolate, i->first), - Converter::ToV8(isolate, i->second)) - .Check(); - } - return result; - } -}; - // Convenience functions that deduce T. template v8::Local ConvertToV8(v8::Isolate* isolate, const T& input) {