From e2442fb052b4d170bc4d9312350971287eb1188b Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 13 Jun 2016 09:27:13 +0900 Subject: [PATCH 1/3] Move converter for std::map to native_mate --- atom/browser/api/atom_api_auto_updater.cc | 1 - atom/common/api/atom_api_crash_reporter.cc | 1 - .../string_map_converter.h | 35 ------------------- filenames.gypi | 1 - vendor/native_mate | 2 +- 5 files changed, 1 insertion(+), 39 deletions(-) delete mode 100644 atom/common/native_mate_converters/string_map_converter.h diff --git a/atom/browser/api/atom_api_auto_updater.cc b/atom/browser/api/atom_api_auto_updater.cc index a1cd0dd3e2b5..cb9bce147212 100644 --- a/atom/browser/api/atom_api_auto_updater.cc +++ b/atom/browser/api/atom_api_auto_updater.cc @@ -9,7 +9,6 @@ #include "atom/browser/native_window.h" #include "atom/browser/window_list.h" #include "atom/common/native_mate_converters/callback.h" -#include "atom/common/native_mate_converters/string_map_converter.h" #include "atom/common/node_includes.h" #include "native_mate/dictionary.h" #include "native_mate/object_template_builder.h" diff --git a/atom/common/api/atom_api_crash_reporter.cc b/atom/common/api/atom_api_crash_reporter.cc index 5db1461f88c0..184a70c72c14 100644 --- a/atom/common/api/atom_api_crash_reporter.cc +++ b/atom/common/api/atom_api_crash_reporter.cc @@ -6,7 +6,6 @@ #include #include "atom/common/crash_reporter/crash_reporter.h" -#include "atom/common/native_mate_converters/string_map_converter.h" #include "base/bind.h" #include "native_mate/dictionary.h" diff --git a/atom/common/native_mate_converters/string_map_converter.h b/atom/common/native_mate_converters/string_map_converter.h deleted file mode 100644 index 54458ab67b31..000000000000 --- a/atom/common/native_mate_converters/string_map_converter.h +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (c) 2016 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_STRING_MAP_CONVERTER_H_ -#define ATOM_COMMON_NATIVE_MATE_CONVERTERS_STRING_MAP_CONVERTER_H_ - -#include -#include - -#include "native_mate/converter.h" - -namespace mate { - -template<> -struct Converter > { - static bool FromV8(v8::Isolate* isolate, - v8::Local val, - std::map* out) { - if (!val->IsObject()) - return false; - - v8::Local dict = val->ToObject(); - v8::Local keys = dict->GetOwnPropertyNames(); - for (uint32_t i = 0; i < keys->Length(); ++i) { - v8::Local key = keys->Get(i); - (*out)[V8ToString(key)] = V8ToString(dict->Get(key)); - } - return true; - } -}; - -} // namespace mate - -#endif // ATOM_COMMON_NATIVE_MATE_CONVERTERS_STRING_MAP_CONVERTER_H_ diff --git a/filenames.gypi b/filenames.gypi index ed25a4038a33..2b87e36fa4d2 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -377,7 +377,6 @@ 'atom/common/native_mate_converters/net_converter.cc', 'atom/common/native_mate_converters/net_converter.h', 'atom/common/native_mate_converters/string16_converter.h', - 'atom/common/native_mate_converters/string_map_converter.h', 'atom/common/native_mate_converters/ui_base_types_converter.h', 'atom/common/native_mate_converters/v8_value_converter.cc', 'atom/common/native_mate_converters/v8_value_converter.h', diff --git a/vendor/native_mate b/vendor/native_mate index 4ad6ecd19617..e75f2aa087db 160000 --- a/vendor/native_mate +++ b/vendor/native_mate @@ -1 +1 @@ -Subproject commit 4ad6ecd19617ac33c09e93ccb6d8e652ac1ac126 +Subproject commit e75f2aa087db346efc4b530f9e1ce7d3a72a3434 From a3786f66c916abf22d876d195d8ae3c9649c7d30 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 13 Jun 2016 09:32:24 +0900 Subject: [PATCH 2/3] Do not add JS wrapper for setFeedURL When possible we should avoid adding wrappers in JS, it usually makes code more difficult to mantain --- atom/browser/api/atom_api_auto_updater.cc | 8 +++++++- atom/browser/api/atom_api_auto_updater.h | 2 ++ docs/api/auto-updater.md | 2 +- lib/browser/api/auto-updater/auto-updater-native.js | 4 ---- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/atom/browser/api/atom_api_auto_updater.cc b/atom/browser/api/atom_api_auto_updater.cc index cb9bce147212..54e2bd9f746a 100644 --- a/atom/browser/api/atom_api_auto_updater.cc +++ b/atom/browser/api/atom_api_auto_updater.cc @@ -79,6 +79,12 @@ void AutoUpdater::OnWindowAllClosed() { QuitAndInstall(); } +void AutoUpdater::SetFeedURL(const std::string& url, mate::Arguments* args) { + auto_updater::AutoUpdater::HeaderMap headers; + args->GetNext(&headers); + auto_updater::AutoUpdater::SetFeedURL(url, headers); +} + void AutoUpdater::QuitAndInstall() { // If we don't have any window then quitAndInstall immediately. WindowList* window_list = WindowList::GetInstance(); @@ -102,8 +108,8 @@ mate::Handle AutoUpdater::Create(v8::Isolate* isolate) { void AutoUpdater::BuildPrototype( v8::Isolate* isolate, v8::Local prototype) { mate::ObjectTemplateBuilder(isolate, prototype) - .SetMethod("_setFeedURL", &auto_updater::AutoUpdater::SetFeedURL) .SetMethod("checkForUpdates", &auto_updater::AutoUpdater::CheckForUpdates) + .SetMethod("setFeedURL", &AutoUpdater::SetFeedURL) .SetMethod("quitAndInstall", &AutoUpdater::QuitAndInstall); } diff --git a/atom/browser/api/atom_api_auto_updater.h b/atom/browser/api/atom_api_auto_updater.h index 857647258adf..e8c135f292ea 100644 --- a/atom/browser/api/atom_api_auto_updater.h +++ b/atom/browser/api/atom_api_auto_updater.h @@ -10,6 +10,7 @@ #include "atom/browser/api/event_emitter.h" #include "atom/browser/auto_updater.h" #include "atom/browser/window_list_observer.h" +#include "native_mate/arguments.h" #include "native_mate/handle.h" namespace atom { @@ -43,6 +44,7 @@ class AutoUpdater : public mate::EventEmitter, void OnWindowAllClosed() override; private: + void SetFeedURL(const std::string& url, mate::Arguments* args); void QuitAndInstall(); DISALLOW_COPY_AND_ASSIGN(AutoUpdater); diff --git a/docs/api/auto-updater.md b/docs/api/auto-updater.md index a1752769473f..268404dee95a 100644 --- a/docs/api/auto-updater.md +++ b/docs/api/auto-updater.md @@ -97,7 +97,7 @@ The `autoUpdater` object has the following methods: ### `autoUpdater.setFeedURL(url[, requestHeaders])` * `url` String -* `requestHeaders` Object - HTTP request headers (_OS X_) +* `requestHeaders` Object _OS X_ - HTTP request headers. Sets the `url` and initialize the auto updater. diff --git a/lib/browser/api/auto-updater/auto-updater-native.js b/lib/browser/api/auto-updater/auto-updater-native.js index e2c79a910106..1fff05dbd65c 100644 --- a/lib/browser/api/auto-updater/auto-updater-native.js +++ b/lib/browser/api/auto-updater/auto-updater-native.js @@ -3,8 +3,4 @@ const autoUpdater = process.atomBinding('auto_updater').autoUpdater Object.setPrototypeOf(autoUpdater, EventEmitter.prototype) -autoUpdater.setFeedURL = function (url, headers) { - return autoUpdater._setFeedURL(url, headers || {}) -} - module.exports = autoUpdater From 26c4fc34cbb9ffc971cfdd37db7568f84c0c98fd Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 13 Jun 2016 09:38:49 +0900 Subject: [PATCH 3/3] Reset SQRLUpdater when setFeedURL is called --- atom/browser/auto_updater_mac.mm | 43 ++++++++++++++++---------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/atom/browser/auto_updater_mac.mm b/atom/browser/auto_updater_mac.mm index bf5030b35a33..005627e6c089 100644 --- a/atom/browser/auto_updater_mac.mm +++ b/atom/browser/auto_updater_mac.mm @@ -38,34 +38,33 @@ void AutoUpdater::SetFeedURL(const std::string& feed, NSURL* url = [NSURL URLWithString:base::SysUTF8ToNSString(feed)]; NSMutableURLRequest* urlRequest = [NSMutableURLRequest requestWithURL:url]; - for (auto&& it : requestHeaders) { + for (const auto& it : requestHeaders) { [urlRequest setValue:base::SysUTF8ToNSString(it.second) forHTTPHeaderField:base::SysUTF8ToNSString(it.first)]; } - if (g_updater == nil) { - // Initialize the SQRLUpdater. - @try { - g_updater = [[SQRLUpdater alloc] initWithUpdateRequest:urlRequest]; - } @catch (NSException* error) { - delegate->OnError(base::SysNSStringToUTF8(error.reason)); - return; - } + if (g_updater) + [g_updater release]; - [[g_updater rac_valuesForKeyPath:@"state" observer:g_updater] - subscribeNext:^(NSNumber *stateNumber) { - int state = [stateNumber integerValue]; - // Dispatching the event on main thread. - dispatch_async(dispatch_get_main_queue(), ^{ - if (state == SQRLUpdaterStateCheckingForUpdate) - delegate->OnCheckingForUpdate(); - else if (state == SQRLUpdaterStateDownloadingUpdate) - delegate->OnUpdateAvailable(); - }); - }]; - } else { - g_updater.updateRequest = urlRequest; + // Initialize the SQRLUpdater. + @try { + g_updater = [[SQRLUpdater alloc] initWithUpdateRequest:urlRequest]; + } @catch (NSException* error) { + delegate->OnError(base::SysNSStringToUTF8(error.reason)); + return; } + + [[g_updater rac_valuesForKeyPath:@"state" observer:g_updater] + subscribeNext:^(NSNumber *stateNumber) { + int state = [stateNumber integerValue]; + // Dispatching the event on main thread. + dispatch_async(dispatch_get_main_queue(), ^{ + if (state == SQRLUpdaterStateCheckingForUpdate) + delegate->OnCheckingForUpdate(); + else if (state == SQRLUpdaterStateDownloadingUpdate) + delegate->OnUpdateAvailable(); + }); + }]; } // static