From 7b8eb3e09c48dc9a2e091eb7a521ae14de66a87d Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 8 Nov 2016 11:03:57 +1100 Subject: [PATCH 01/16] Rename autoUpload to shouldUpload --- atom/common/api/atom_api_crash_reporter.cc | 2 ++ atom/common/crash_reporter/crash_reporter.cc | 8 ++++++-- atom/common/crash_reporter/crash_reporter.h | 6 ++++-- .../crash_reporter/crash_reporter_linux.cc | 2 +- .../crash_reporter/crash_reporter_linux.h | 2 +- .../crash_reporter/crash_reporter_mac.h | 3 ++- .../crash_reporter/crash_reporter_mac.mm | 14 +++++++++---- .../crash_reporter/crash_reporter_win.cc | 2 +- .../crash_reporter/crash_reporter_win.h | 2 +- docs/api/crash-reporter.md | 8 +++++++- lib/common/api/crash-reporter.js | 20 +++++++++++++++---- 11 files changed, 51 insertions(+), 18 deletions(-) diff --git a/atom/common/api/atom_api_crash_reporter.cc b/atom/common/api/atom_api_crash_reporter.cc index 5dff9e667f..24eb692001 100644 --- a/atom/common/api/atom_api_crash_reporter.cc +++ b/atom/common/api/atom_api_crash_reporter.cc @@ -40,6 +40,8 @@ void Initialize(v8::Local exports, v8::Local unused, base::Bind(&CrashReporter::Start, report)); dict.SetMethod("_getUploadedReports", base::Bind(&CrashReporter::GetUploadedReports, report)); + dict.SetMethod("_setShouldUpload", + base::Bind(&CrashReporter::SetShouldUpload, report)); } } // namespace diff --git a/atom/common/crash_reporter/crash_reporter.cc b/atom/common/crash_reporter/crash_reporter.cc index 1254f3fde6..a672df65ac 100644 --- a/atom/common/crash_reporter/crash_reporter.cc +++ b/atom/common/crash_reporter/crash_reporter.cc @@ -26,13 +26,13 @@ void CrashReporter::Start(const std::string& product_name, const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool auto_submit, + bool should_upload, bool skip_system_crash_handler, const StringMap& extra_parameters) { SetUploadParameters(extra_parameters); InitBreakpad(product_name, ATOM_VERSION_STRING, company_name, submit_url, - crashes_dir, auto_submit, skip_system_crash_handler); + crashes_dir, should_upload, skip_system_crash_handler); } void CrashReporter::SetUploadParameters(const StringMap& parameters) { @@ -43,6 +43,10 @@ void CrashReporter::SetUploadParameters(const StringMap& parameters) { SetUploadParameters(); } +void CrashReporter::SetShouldUpload(const bool should_upload) { + +} + std::vector CrashReporter::GetUploadedReports(const base::FilePath& crashes_dir) { std::string file_content; diff --git a/atom/common/crash_reporter/crash_reporter.h b/atom/common/crash_reporter/crash_reporter.h index ef153523ac..c4fd3003ac 100644 --- a/atom/common/crash_reporter/crash_reporter.h +++ b/atom/common/crash_reporter/crash_reporter.h @@ -26,7 +26,7 @@ class CrashReporter { const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool auto_submit, + bool should_upload, bool skip_system_crash_handler, const StringMap& extra_parameters); @@ -42,10 +42,12 @@ class CrashReporter { const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool auto_submit, + bool should_upload, bool skip_system_crash_handler); virtual void SetUploadParameters(); + virtual void SetShouldUpload(); + StringMap upload_parameters_; bool is_browser_; diff --git a/atom/common/crash_reporter/crash_reporter_linux.cc b/atom/common/crash_reporter/crash_reporter_linux.cc index 2442d7e168..5299997a6f 100644 --- a/atom/common/crash_reporter/crash_reporter_linux.cc +++ b/atom/common/crash_reporter/crash_reporter_linux.cc @@ -60,7 +60,7 @@ void CrashReporterLinux::InitBreakpad(const std::string& product_name, const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool auto_submit, + bool should_upload, bool skip_system_crash_handler) { EnableCrashDumping(crashes_dir); diff --git a/atom/common/crash_reporter/crash_reporter_linux.h b/atom/common/crash_reporter/crash_reporter_linux.h index 36a3699732..8be6e5f67c 100644 --- a/atom/common/crash_reporter/crash_reporter_linux.h +++ b/atom/common/crash_reporter/crash_reporter_linux.h @@ -32,7 +32,7 @@ class CrashReporterLinux : public CrashReporter { const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool auto_submit, + bool should_upload, bool skip_system_crash_handler) override; void SetUploadParameters() override; diff --git a/atom/common/crash_reporter/crash_reporter_mac.h b/atom/common/crash_reporter/crash_reporter_mac.h index ed4888bbaf..ddd037bb8e 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.h +++ b/atom/common/crash_reporter/crash_reporter_mac.h @@ -28,7 +28,7 @@ class CrashReporterMac : public CrashReporter { const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool auto_submit, + bool should_upload, bool skip_system_crash_handler) override; void SetUploadParameters() override; @@ -46,6 +46,7 @@ class CrashReporterMac : public CrashReporter { const base::FilePath& crashes_dir) override; std::unique_ptr simple_string_dictionary_; + std::unique_ptr database_; DISALLOW_COPY_AND_ASSIGN(CrashReporterMac); }; diff --git a/atom/common/crash_reporter/crash_reporter_mac.mm b/atom/common/crash_reporter/crash_reporter_mac.mm index 82e3ea140c..fa01c8d0dc 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.mm +++ b/atom/common/crash_reporter/crash_reporter_mac.mm @@ -31,7 +31,7 @@ void CrashReporterMac::InitBreakpad(const std::string& product_name, const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool auto_submit, + bool should_upload, bool skip_system_crash_handler) { // check whether crashpad has been initialized. // Only need to initialize once. @@ -73,14 +73,20 @@ void CrashReporterMac::InitBreakpad(const std::string& product_name, SetCrashKeyValue(upload_parameter.first, upload_parameter.second); } if (is_browser_) { - std::unique_ptr database = + database_ = crashpad::CrashReportDatabase::Initialize(crashes_dir); - if (database) { - database->GetSettings()->SetUploadsEnabled(auto_submit); + if (database_) { + database_->GetSettings()->SetUploadsEnabled(should_upload); } } } +void CrashReporterMac::SetShouldUpload(const bool should_upload) { + if (database_) { + database_->GetSettings()->SetUploadsEnabled(should_upload); + } +} + void CrashReporterMac::SetUploadParameters() { upload_parameters_["platform"] = "darwin"; } diff --git a/atom/common/crash_reporter/crash_reporter_win.cc b/atom/common/crash_reporter/crash_reporter_win.cc index a465edcb45..d78f35cbb5 100644 --- a/atom/common/crash_reporter/crash_reporter_win.cc +++ b/atom/common/crash_reporter/crash_reporter_win.cc @@ -150,7 +150,7 @@ void CrashReporterWin::InitBreakpad(const std::string& product_name, const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool auto_submit, + bool should_upload, bool skip_system_crash_handler) { skip_system_crash_handler_ = skip_system_crash_handler; diff --git a/atom/common/crash_reporter/crash_reporter_win.h b/atom/common/crash_reporter/crash_reporter_win.h index 20a537d892..6cac00225d 100644 --- a/atom/common/crash_reporter/crash_reporter_win.h +++ b/atom/common/crash_reporter/crash_reporter_win.h @@ -28,7 +28,7 @@ class CrashReporterWin : public CrashReporter { const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool auto_submit, + bool should_upload, bool skip_system_crash_handler) override; void SetUploadParameters() override; diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index cec860ad61..8503059008 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -40,7 +40,7 @@ The `crashReporter` module has the following methods: * `companyName` String (optional) * `submitURL` String - URL that crash reports will be sent to as POST. * `productName` String (optional) - Defaults to `app.getName()`. - * `autoSubmit` Boolean (optional) - Send the crash report without user interaction. + * `shouldUpload` Boolean (optional) _macOS_ - Whether crash reports should be sent to the server Default is `true`. * `ignoreSystemCrashHandler` Boolean (optional) - Default is `false`. * `extra` Object (optional) - An object you can define that will be sent along with the @@ -70,6 +70,12 @@ Returns [`CrashReport[]`](structures/crash-report.md): Returns all uploaded crash reports. Each report contains the date and uploaded ID. +### `crashReporter.setShouldUpload(shouldUpload)` + +* `shouldUpload` Boolean _macOS_ - Whether reports should be submitted to the server + +This would normally be controlled by user preferences. + ## Crash Report Payload The crash reporter will send the following data to the `submitURL` as diff --git a/lib/common/api/crash-reporter.js b/lib/common/api/crash-reporter.js index 7749583f79..e54024b238 100644 --- a/lib/common/api/crash-reporter.js +++ b/lib/common/api/crash-reporter.js @@ -13,10 +13,12 @@ class CrashReporter { options = {} } this.productName = options.productName != null ? options.productName : app.getName() - let {autoSubmit, companyName, extra, ignoreSystemCrashHandler, submitURL} = options + let {autoSubmit, companyName, extra, ignoreSystemCrashHandler, submitURL, shouldUpload} = options - if (autoSubmit == null) { - autoSubmit = true + if (autoSubmit == null && shouldUpload == null) { + shouldUpload = true + } else { + shouldUpload = shouldUpload || autoSubmit } if (ignoreSystemCrashHandler == null) { ignoreSystemCrashHandler = false @@ -56,7 +58,8 @@ class CrashReporter { }) } - binding.start(this.getProductName(), companyName, submitURL, this.getCrashesDirectory(), autoSubmit, ignoreSystemCrashHandler, extra) + this._shouldUpload = shouldUpload + binding.start(this.getProductName(), companyName, submitURL, this.getCrashesDirectory(), shouldUpload, ignoreSystemCrashHandler, extra) } getLastCrashReport () { @@ -95,6 +98,15 @@ class CrashReporter { } return this.tempDirectory } + + getShouldUpload() { + return this._shouldUpload + } + + setShouldUpload(shouldUpload) { + this._shouldUpload = shouldUpload + return bindings._setShouldUpload(shouldUpload) + } } module.exports = new CrashReporter() From 6bbd92368fd5258312c41f14ce681ae870950ad4 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 8 Nov 2016 11:12:46 +1100 Subject: [PATCH 02/16] Add docs for getShouldUpload --- docs/api/crash-reporter.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index 8503059008..7114990d36 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -70,6 +70,11 @@ Returns [`CrashReport[]`](structures/crash-report.md): Returns all uploaded crash reports. Each report contains the date and uploaded ID. +### `crashReporter.getShouldUpload()` + +Returns `Boolean` - Whether reports should be submitted to the server. Set through +the `start` method or `setShouldUpload`. + ### `crashReporter.setShouldUpload(shouldUpload)` * `shouldUpload` Boolean _macOS_ - Whether reports should be submitted to the server From 0b9530efd711f791da54d31a751902c88f6abe28 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 8 Nov 2016 11:14:55 +1100 Subject: [PATCH 03/16] shouldUpload API's are macOS only --- docs/api/crash-reporter.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index 7114990d36..89f7e138e2 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -70,12 +70,12 @@ Returns [`CrashReport[]`](structures/crash-report.md): Returns all uploaded crash reports. Each report contains the date and uploaded ID. -### `crashReporter.getShouldUpload()` +### `crashReporter.getShouldUpload()` _macOS_ Returns `Boolean` - Whether reports should be submitted to the server. Set through the `start` method or `setShouldUpload`. -### `crashReporter.setShouldUpload(shouldUpload)` +### `crashReporter.setShouldUpload(shouldUpload)` _macOS_ * `shouldUpload` Boolean _macOS_ - Whether reports should be submitted to the server From 0d1804b2a07a794a896124877c1f11f21336ed27 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 8 Nov 2016 11:18:49 +1100 Subject: [PATCH 04/16] Fix issues and deprecate the old prop --- atom/common/crash_reporter/crash_reporter.h | 4 ++-- lib/common/api/crash-reporter.js | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/atom/common/crash_reporter/crash_reporter.h b/atom/common/crash_reporter/crash_reporter.h index c4fd3003ac..c7dde01ddb 100644 --- a/atom/common/crash_reporter/crash_reporter.h +++ b/atom/common/crash_reporter/crash_reporter.h @@ -33,6 +33,8 @@ class CrashReporter { virtual std::vector GetUploadedReports( const base::FilePath& crashes_dir); + virtual void SetShouldUpload(); + protected: CrashReporter(); virtual ~CrashReporter(); @@ -46,8 +48,6 @@ class CrashReporter { bool skip_system_crash_handler); virtual void SetUploadParameters(); - virtual void SetShouldUpload(); - StringMap upload_parameters_; bool is_browser_; diff --git a/lib/common/api/crash-reporter.js b/lib/common/api/crash-reporter.js index e54024b238..4f2d3bd764 100644 --- a/lib/common/api/crash-reporter.js +++ b/lib/common/api/crash-reporter.js @@ -18,6 +18,10 @@ class CrashReporter { if (autoSubmit == null && shouldUpload == null) { shouldUpload = true } else { + if (typeof autoSubmit !== 'undefined') { + // TODO: Remove depreceated property in 2.0.0 + console.warn('The "autoSubmit" attribute on electron.crashReporter is depreceated. Please use "shouldUpload" instead.') + } shouldUpload = shouldUpload || autoSubmit } if (ignoreSystemCrashHandler == null) { From 5a1a2616aa790a1d4d70e6b72694ad6e5a71028c Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 8 Nov 2016 13:25:18 +1300 Subject: [PATCH 05/16] Fix build issues --- atom/common/crash_reporter/crash_reporter.h | 2 +- atom/common/crash_reporter/crash_reporter_mac.h | 2 ++ lib/common/api/crash-reporter.js | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/atom/common/crash_reporter/crash_reporter.h b/atom/common/crash_reporter/crash_reporter.h index c7dde01ddb..caf4455628 100644 --- a/atom/common/crash_reporter/crash_reporter.h +++ b/atom/common/crash_reporter/crash_reporter.h @@ -33,7 +33,7 @@ class CrashReporter { virtual std::vector GetUploadedReports( const base::FilePath& crashes_dir); - virtual void SetShouldUpload(); + virtual void SetShouldUpload(bool should_upload); protected: CrashReporter(); diff --git a/atom/common/crash_reporter/crash_reporter_mac.h b/atom/common/crash_reporter/crash_reporter_mac.h index ddd037bb8e..2e3c43639f 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.h +++ b/atom/common/crash_reporter/crash_reporter_mac.h @@ -11,6 +11,7 @@ #include "atom/common/crash_reporter/crash_reporter.h" #include "base/compiler_specific.h" #include "base/strings/string_piece.h" +#include "vendor/crashpad/client/crash_report_database.h" #include "vendor/crashpad/client/simple_string_dictionary.h" namespace base { @@ -31,6 +32,7 @@ class CrashReporterMac : public CrashReporter { bool should_upload, bool skip_system_crash_handler) override; void SetUploadParameters() override; + void SetShouldUpload(bool should_upload) override; private: friend struct base::DefaultSingletonTraits; diff --git a/lib/common/api/crash-reporter.js b/lib/common/api/crash-reporter.js index 4f2d3bd764..e17670321b 100644 --- a/lib/common/api/crash-reporter.js +++ b/lib/common/api/crash-reporter.js @@ -20,7 +20,7 @@ class CrashReporter { } else { if (typeof autoSubmit !== 'undefined') { // TODO: Remove depreceated property in 2.0.0 - console.warn('The "autoSubmit" attribute on electron.crashReporter is depreceated. Please use "shouldUpload" instead.') + console.warn('The "autoSubmit" attribute on electron.crashReporter.start is depreceated. Please use "shouldUpload" instead.') } shouldUpload = shouldUpload || autoSubmit } @@ -109,7 +109,7 @@ class CrashReporter { setShouldUpload(shouldUpload) { this._shouldUpload = shouldUpload - return bindings._setShouldUpload(shouldUpload) + return binding._setShouldUpload(shouldUpload) } } From 285a36f9de706448d29e4f27d3afda49cf478bd0 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 8 Nov 2016 13:39:11 +1300 Subject: [PATCH 06/16] Use GetUploadsEnabled to return GetShouldUpload --- atom/common/api/atom_api_crash_reporter.cc | 2 ++ atom/common/crash_reporter/crash_reporter.cc | 4 ++++ atom/common/crash_reporter/crash_reporter.h | 1 + atom/common/crash_reporter/crash_reporter_mac.h | 1 + atom/common/crash_reporter/crash_reporter_mac.mm | 12 +++++++++--- docs/api/crash-reporter.md | 4 ++++ lib/common/api/crash-reporter.js | 14 ++++++++++---- 7 files changed, 31 insertions(+), 7 deletions(-) diff --git a/atom/common/api/atom_api_crash_reporter.cc b/atom/common/api/atom_api_crash_reporter.cc index 24eb692001..fb9ad3bb54 100644 --- a/atom/common/api/atom_api_crash_reporter.cc +++ b/atom/common/api/atom_api_crash_reporter.cc @@ -42,6 +42,8 @@ void Initialize(v8::Local exports, v8::Local unused, base::Bind(&CrashReporter::GetUploadedReports, report)); dict.SetMethod("_setShouldUpload", base::Bind(&CrashReporter::SetShouldUpload, report)); + dict.SetMethod("_getShouldUpload", + base::Bind(&CrashReporter::GetShouldUpload, report)); } } // namespace diff --git a/atom/common/crash_reporter/crash_reporter.cc b/atom/common/crash_reporter/crash_reporter.cc index a672df65ac..dc76adf6b7 100644 --- a/atom/common/crash_reporter/crash_reporter.cc +++ b/atom/common/crash_reporter/crash_reporter.cc @@ -47,6 +47,10 @@ void CrashReporter::SetShouldUpload(const bool should_upload) { } +bool CrashReporter::GetShouldUpload() { + return true; +} + std::vector CrashReporter::GetUploadedReports(const base::FilePath& crashes_dir) { std::string file_content; diff --git a/atom/common/crash_reporter/crash_reporter.h b/atom/common/crash_reporter/crash_reporter.h index caf4455628..fe7d109460 100644 --- a/atom/common/crash_reporter/crash_reporter.h +++ b/atom/common/crash_reporter/crash_reporter.h @@ -34,6 +34,7 @@ class CrashReporter { const base::FilePath& crashes_dir); virtual void SetShouldUpload(bool should_upload); + virtual bool GetShouldUpload(); protected: CrashReporter(); diff --git a/atom/common/crash_reporter/crash_reporter_mac.h b/atom/common/crash_reporter/crash_reporter_mac.h index 2e3c43639f..d9b0ca7344 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.h +++ b/atom/common/crash_reporter/crash_reporter_mac.h @@ -33,6 +33,7 @@ class CrashReporterMac : public CrashReporter { bool skip_system_crash_handler) override; void SetUploadParameters() override; void SetShouldUpload(bool should_upload) override; + bool GetShouldUpload() override; private: friend struct base::DefaultSingletonTraits; diff --git a/atom/common/crash_reporter/crash_reporter_mac.mm b/atom/common/crash_reporter/crash_reporter_mac.mm index fa01c8d0dc..9c649a21a1 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.mm +++ b/atom/common/crash_reporter/crash_reporter_mac.mm @@ -75,12 +75,18 @@ void CrashReporterMac::InitBreakpad(const std::string& product_name, if (is_browser_) { database_ = crashpad::CrashReportDatabase::Initialize(crashes_dir); - if (database_) { - database_->GetSettings()->SetUploadsEnabled(should_upload); - } + SetShouldUpload(should_upload); } } +bool CrashReporterMac::GetShouldUpload() { + bool enabled = true; + if (database_) { + database_->GetSettings()->GetUploadsEnabled(&enabled); + } + return enabled; +} + void CrashReporterMac::SetShouldUpload(const bool should_upload) { if (database_) { database_->GetSettings()->SetUploadsEnabled(should_upload); diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index 89f7e138e2..216a36a53b 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -75,12 +75,16 @@ ID. Returns `Boolean` - Whether reports should be submitted to the server. Set through the `start` method or `setShouldUpload`. +**NOTE:** This API can only be used from the main process + ### `crashReporter.setShouldUpload(shouldUpload)` _macOS_ * `shouldUpload` Boolean _macOS_ - Whether reports should be submitted to the server This would normally be controlled by user preferences. +**NOTE:** This API can only be used from the main process + ## Crash Report Payload The crash reporter will send the following data to the `submitURL` as diff --git a/lib/common/api/crash-reporter.js b/lib/common/api/crash-reporter.js index e17670321b..7a8648aa44 100644 --- a/lib/common/api/crash-reporter.js +++ b/lib/common/api/crash-reporter.js @@ -62,7 +62,6 @@ class CrashReporter { }) } - this._shouldUpload = shouldUpload binding.start(this.getProductName(), companyName, submitURL, this.getCrashesDirectory(), shouldUpload, ignoreSystemCrashHandler, extra) } @@ -104,12 +103,19 @@ class CrashReporter { } getShouldUpload() { - return this._shouldUpload + if (process.type === 'browser') { + return binding._getShouldUpload() + } else { + throw new Error('getShouldUpload can only be called from the main process') + } } setShouldUpload(shouldUpload) { - this._shouldUpload = shouldUpload - return binding._setShouldUpload(shouldUpload) + if (process.type === 'browser') { + return binding._setShouldUpload(shouldUpload) + } else { + throw new Error('setShouldUpload can only be called from the main process') + } } } From 4b61a4d3deb69fdc50645aaf2b27333a80b0783d Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 8 Nov 2016 15:13:54 +1100 Subject: [PATCH 07/16] Fix linting --- lib/common/api/crash-reporter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/common/api/crash-reporter.js b/lib/common/api/crash-reporter.js index 7a8648aa44..4b49292b91 100644 --- a/lib/common/api/crash-reporter.js +++ b/lib/common/api/crash-reporter.js @@ -102,7 +102,7 @@ class CrashReporter { return this.tempDirectory } - getShouldUpload() { + getShouldUpload () { if (process.type === 'browser') { return binding._getShouldUpload() } else { @@ -110,7 +110,7 @@ class CrashReporter { } } - setShouldUpload(shouldUpload) { + setShouldUpload (shouldUpload) { if (process.type === 'browser') { return binding._setShouldUpload(shouldUpload) } else { From a7dedb3a13c2d495790308c45a3f72f060fc5f40 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 8 Nov 2016 17:06:49 +1100 Subject: [PATCH 08/16] Update crash_reporter.cc --- atom/common/crash_reporter/crash_reporter.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/atom/common/crash_reporter/crash_reporter.cc b/atom/common/crash_reporter/crash_reporter.cc index dc76adf6b7..7f0834941e 100644 --- a/atom/common/crash_reporter/crash_reporter.cc +++ b/atom/common/crash_reporter/crash_reporter.cc @@ -44,7 +44,6 @@ void CrashReporter::SetUploadParameters(const StringMap& parameters) { } void CrashReporter::SetShouldUpload(const bool should_upload) { - } bool CrashReporter::GetShouldUpload() { From 2bf6f281525d4807920df6d84567ac729355f92b Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 22 Nov 2016 19:30:20 +1100 Subject: [PATCH 09/16] shouldUpload --> uploadToServer --- atom/common/api/atom_api_crash_reporter.cc | 8 +++---- atom/common/crash_reporter/crash_reporter.cc | 8 +++---- atom/common/crash_reporter/crash_reporter.h | 8 +++---- .../crash_reporter/crash_reporter_linux.cc | 2 +- .../crash_reporter/crash_reporter_linux.h | 2 +- .../crash_reporter/crash_reporter_mac.h | 6 ++--- .../crash_reporter/crash_reporter_mac.mm | 10 ++++---- .../crash_reporter/crash_reporter_win.cc | 2 +- .../crash_reporter/crash_reporter_win.h | 2 +- docs/api/crash-reporter.md | 10 ++++---- lib/common/api/crash-reporter.js | 24 +++++++++---------- 11 files changed, 41 insertions(+), 41 deletions(-) diff --git a/atom/common/api/atom_api_crash_reporter.cc b/atom/common/api/atom_api_crash_reporter.cc index fb9ad3bb54..aaaf200b32 100644 --- a/atom/common/api/atom_api_crash_reporter.cc +++ b/atom/common/api/atom_api_crash_reporter.cc @@ -40,10 +40,10 @@ void Initialize(v8::Local exports, v8::Local unused, base::Bind(&CrashReporter::Start, report)); dict.SetMethod("_getUploadedReports", base::Bind(&CrashReporter::GetUploadedReports, report)); - dict.SetMethod("_setShouldUpload", - base::Bind(&CrashReporter::SetShouldUpload, report)); - dict.SetMethod("_getShouldUpload", - base::Bind(&CrashReporter::GetShouldUpload, report)); + dict.SetMethod("_setUploadToServer", + base::Bind(&CrashReporter::SetUploadToServer, report)); + dict.SetMethod("_getUploadToServer", + base::Bind(&CrashReporter::GetUploadToServer, report)); } } // namespace diff --git a/atom/common/crash_reporter/crash_reporter.cc b/atom/common/crash_reporter/crash_reporter.cc index 7f0834941e..5e13bd6fe5 100644 --- a/atom/common/crash_reporter/crash_reporter.cc +++ b/atom/common/crash_reporter/crash_reporter.cc @@ -26,13 +26,13 @@ void CrashReporter::Start(const std::string& product_name, const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool should_upload, + bool upload_to_server, bool skip_system_crash_handler, const StringMap& extra_parameters) { SetUploadParameters(extra_parameters); InitBreakpad(product_name, ATOM_VERSION_STRING, company_name, submit_url, - crashes_dir, should_upload, skip_system_crash_handler); + crashes_dir, upload_to_server, skip_system_crash_handler); } void CrashReporter::SetUploadParameters(const StringMap& parameters) { @@ -43,10 +43,10 @@ void CrashReporter::SetUploadParameters(const StringMap& parameters) { SetUploadParameters(); } -void CrashReporter::SetShouldUpload(const bool should_upload) { +void CrashReporter::SetUploadToServer(const bool upload_to_server) { } -bool CrashReporter::GetShouldUpload() { +bool CrashReporter::GetUploadToServer() { return true; } diff --git a/atom/common/crash_reporter/crash_reporter.h b/atom/common/crash_reporter/crash_reporter.h index fe7d109460..6eb43d0a2d 100644 --- a/atom/common/crash_reporter/crash_reporter.h +++ b/atom/common/crash_reporter/crash_reporter.h @@ -26,15 +26,15 @@ class CrashReporter { const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool should_upload, + bool upload_to_server, bool skip_system_crash_handler, const StringMap& extra_parameters); virtual std::vector GetUploadedReports( const base::FilePath& crashes_dir); - virtual void SetShouldUpload(bool should_upload); - virtual bool GetShouldUpload(); + virtual void SetUploadToServer(bool upload_to_server); + virtual bool GetUploadToServer(); protected: CrashReporter(); @@ -45,7 +45,7 @@ class CrashReporter { const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool should_upload, + bool upload_to_server, bool skip_system_crash_handler); virtual void SetUploadParameters(); diff --git a/atom/common/crash_reporter/crash_reporter_linux.cc b/atom/common/crash_reporter/crash_reporter_linux.cc index 5299997a6f..50e1c5ec86 100644 --- a/atom/common/crash_reporter/crash_reporter_linux.cc +++ b/atom/common/crash_reporter/crash_reporter_linux.cc @@ -60,7 +60,7 @@ void CrashReporterLinux::InitBreakpad(const std::string& product_name, const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool should_upload, + bool upload_to_server, bool skip_system_crash_handler) { EnableCrashDumping(crashes_dir); diff --git a/atom/common/crash_reporter/crash_reporter_linux.h b/atom/common/crash_reporter/crash_reporter_linux.h index 8be6e5f67c..846e1f1b0a 100644 --- a/atom/common/crash_reporter/crash_reporter_linux.h +++ b/atom/common/crash_reporter/crash_reporter_linux.h @@ -32,7 +32,7 @@ class CrashReporterLinux : public CrashReporter { const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool should_upload, + bool upload_to_server, bool skip_system_crash_handler) override; void SetUploadParameters() override; diff --git a/atom/common/crash_reporter/crash_reporter_mac.h b/atom/common/crash_reporter/crash_reporter_mac.h index d9b0ca7344..9066f2017b 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.h +++ b/atom/common/crash_reporter/crash_reporter_mac.h @@ -29,11 +29,11 @@ class CrashReporterMac : public CrashReporter { const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool should_upload, + bool upload_to_server, bool skip_system_crash_handler) override; void SetUploadParameters() override; - void SetShouldUpload(bool should_upload) override; - bool GetShouldUpload() override; + void SetUploadToServer(bool upload_to_server) override; + bool GetUploadToServer() override; private: friend struct base::DefaultSingletonTraits; diff --git a/atom/common/crash_reporter/crash_reporter_mac.mm b/atom/common/crash_reporter/crash_reporter_mac.mm index 9c649a21a1..877c68d1c2 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.mm +++ b/atom/common/crash_reporter/crash_reporter_mac.mm @@ -31,7 +31,7 @@ void CrashReporterMac::InitBreakpad(const std::string& product_name, const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool should_upload, + bool upload_to_server, bool skip_system_crash_handler) { // check whether crashpad has been initialized. // Only need to initialize once. @@ -75,11 +75,11 @@ void CrashReporterMac::InitBreakpad(const std::string& product_name, if (is_browser_) { database_ = crashpad::CrashReportDatabase::Initialize(crashes_dir); - SetShouldUpload(should_upload); + SetUploadToServer(upload_to_server); } } -bool CrashReporterMac::GetShouldUpload() { +bool CrashReporterMac::GetUploadToServer() { bool enabled = true; if (database_) { database_->GetSettings()->GetUploadsEnabled(&enabled); @@ -87,9 +87,9 @@ bool CrashReporterMac::GetShouldUpload() { return enabled; } -void CrashReporterMac::SetShouldUpload(const bool should_upload) { +void CrashReporterMac::SetUploadToServer(const bool upload_to_server) { if (database_) { - database_->GetSettings()->SetUploadsEnabled(should_upload); + database_->GetSettings()->SetUploadsEnabled(upload_to_server); } } diff --git a/atom/common/crash_reporter/crash_reporter_win.cc b/atom/common/crash_reporter/crash_reporter_win.cc index d78f35cbb5..25969dc32e 100644 --- a/atom/common/crash_reporter/crash_reporter_win.cc +++ b/atom/common/crash_reporter/crash_reporter_win.cc @@ -150,7 +150,7 @@ void CrashReporterWin::InitBreakpad(const std::string& product_name, const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool should_upload, + bool upload_to_server, bool skip_system_crash_handler) { skip_system_crash_handler_ = skip_system_crash_handler; diff --git a/atom/common/crash_reporter/crash_reporter_win.h b/atom/common/crash_reporter/crash_reporter_win.h index 6cac00225d..3fc139aca1 100644 --- a/atom/common/crash_reporter/crash_reporter_win.h +++ b/atom/common/crash_reporter/crash_reporter_win.h @@ -28,7 +28,7 @@ class CrashReporterWin : public CrashReporter { const std::string& company_name, const std::string& submit_url, const base::FilePath& crashes_dir, - bool should_upload, + bool upload_to_server, bool skip_system_crash_handler) override; void SetUploadParameters() override; diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index 216a36a53b..044e6a8fdc 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -40,7 +40,7 @@ The `crashReporter` module has the following methods: * `companyName` String (optional) * `submitURL` String - URL that crash reports will be sent to as POST. * `productName` String (optional) - Defaults to `app.getName()`. - * `shouldUpload` Boolean (optional) _macOS_ - Whether crash reports should be sent to the server + * `uploadToServer` Boolean (optional) _macOS_ - Whether crash reports should be sent to the server Default is `true`. * `ignoreSystemCrashHandler` Boolean (optional) - Default is `false`. * `extra` Object (optional) - An object you can define that will be sent along with the @@ -70,16 +70,16 @@ Returns [`CrashReport[]`](structures/crash-report.md): Returns all uploaded crash reports. Each report contains the date and uploaded ID. -### `crashReporter.getShouldUpload()` _macOS_ +### `crashReporter.getUploadToServer()` _macOS_ Returns `Boolean` - Whether reports should be submitted to the server. Set through -the `start` method or `setShouldUpload`. +the `start` method or `setUploadToServer`. **NOTE:** This API can only be used from the main process -### `crashReporter.setShouldUpload(shouldUpload)` _macOS_ +### `crashReporter.setUploadToServer(uploadToServer)` _macOS_ -* `shouldUpload` Boolean _macOS_ - Whether reports should be submitted to the server +* `uploadToServer` Boolean _macOS_ - Whether reports should be submitted to the server This would normally be controlled by user preferences. diff --git a/lib/common/api/crash-reporter.js b/lib/common/api/crash-reporter.js index 4b49292b91..a9509e4ed3 100644 --- a/lib/common/api/crash-reporter.js +++ b/lib/common/api/crash-reporter.js @@ -13,16 +13,16 @@ class CrashReporter { options = {} } this.productName = options.productName != null ? options.productName : app.getName() - let {autoSubmit, companyName, extra, ignoreSystemCrashHandler, submitURL, shouldUpload} = options + let {autoSubmit, companyName, extra, ignoreSystemCrashHandler, submitURL, uploadToServer} = options - if (autoSubmit == null && shouldUpload == null) { - shouldUpload = true + if (autoSubmit == null && uploadToServer == null) { + uploadToServer = true } else { if (typeof autoSubmit !== 'undefined') { // TODO: Remove depreceated property in 2.0.0 - console.warn('The "autoSubmit" attribute on electron.crashReporter.start is depreceated. Please use "shouldUpload" instead.') + console.warn('The "autoSubmit" attribute on electron.crashReporter.start is depreceated. Please use "uploadToServer" instead.') } - shouldUpload = shouldUpload || autoSubmit + uploadToServer = uploadToServer || autoSubmit } if (ignoreSystemCrashHandler == null) { ignoreSystemCrashHandler = false @@ -62,7 +62,7 @@ class CrashReporter { }) } - binding.start(this.getProductName(), companyName, submitURL, this.getCrashesDirectory(), shouldUpload, ignoreSystemCrashHandler, extra) + binding.start(this.getProductName(), companyName, submitURL, this.getCrashesDirectory(), uploadToServer, ignoreSystemCrashHandler, extra) } getLastCrashReport () { @@ -102,19 +102,19 @@ class CrashReporter { return this.tempDirectory } - getShouldUpload () { + getUploadToServer () { if (process.type === 'browser') { - return binding._getShouldUpload() + return binding._getUploadToServer() } else { - throw new Error('getShouldUpload can only be called from the main process') + throw new Error('getUploadToServer can only be called from the main process') } } - setShouldUpload (shouldUpload) { + setUploadToServer (uploadToServer) { if (process.type === 'browser') { - return binding._setShouldUpload(shouldUpload) + return binding._setUploadToServer(uploadToServer) } else { - throw new Error('setShouldUpload can only be called from the main process') + throw new Error('setUploadToServer can only be called from the main process') } } } From bb9876bd6e7057a407664ae8ec75f2d381815ced Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 28 Nov 2016 14:51:24 -0800 Subject: [PATCH 10/16] Remove deprecation warning for autoSubmit --- lib/common/api/crash-reporter.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/common/api/crash-reporter.js b/lib/common/api/crash-reporter.js index a9509e4ed3..c5a462cf51 100644 --- a/lib/common/api/crash-reporter.js +++ b/lib/common/api/crash-reporter.js @@ -13,17 +13,17 @@ class CrashReporter { options = {} } this.productName = options.productName != null ? options.productName : app.getName() - let {autoSubmit, companyName, extra, ignoreSystemCrashHandler, submitURL, uploadToServer} = options + let {companyName, extra, ignoreSystemCrashHandler, submitURL, uploadToServer} = options - if (autoSubmit == null && uploadToServer == null) { - uploadToServer = true - } else { - if (typeof autoSubmit !== 'undefined') { - // TODO: Remove depreceated property in 2.0.0 - console.warn('The "autoSubmit" attribute on electron.crashReporter.start is depreceated. Please use "uploadToServer" instead.') - } - uploadToServer = uploadToServer || autoSubmit + if (uploadToServer == null) { + // TODO: Remove deprecated autoSubmit property in 2.0 + uploadToServer = options.autoSubmit } + + if (uploadToServer == null) { + uploadToServer = true + } + if (ignoreSystemCrashHandler == null) { ignoreSystemCrashHandler = false } From f4be3782a265041238bad3b6862e7095b14a8205 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 28 Nov 2016 15:01:38 -0800 Subject: [PATCH 11/16] Add spec for get/setUploadToServer --- spec/api-crash-reporter-spec.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/spec/api-crash-reporter-spec.js b/spec/api-crash-reporter-spec.js index fe8c3f6c62..86509f9f63 100644 --- a/spec/api-crash-reporter-spec.js +++ b/spec/api-crash-reporter-spec.js @@ -117,6 +117,27 @@ describe('crashReporter module', function () { }) }) }) + + describe('.get/setUploadToServer', function () { + it('throws an error when called from the renderer process', function () { + assert.throws(() => require('electron').crashReporter.getUploadToServer()) + }) + + it('can be read/set from the main process', function () { + if (process.platform === 'darwin') { + crashReporter.start({ + companyName: 'Umbrella Corporation', + submitURL: 'http://127.0.0.1/crashes', + autoSubmit: true + }) + assert.equal(crashReporter.getUploadToServer(), true) + crashReporter.setUploadToServer(false) + assert.equal(crashReporter.getUploadToServer(), false) + } else { + assert.equal(crashReporter.getUploadToServer(), false) + } + }) + }) }) const waitForCrashReport = () => { From 02cbd24165cef438ff973ed3c7e7f908670aebe8 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 28 Nov 2016 15:03:10 -0800 Subject: [PATCH 12/16] Mention setUploadToServer only has effect after start --- docs/api/crash-reporter.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index 044e6a8fdc..f9409bdfc3 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -75,15 +75,16 @@ ID. Returns `Boolean` - Whether reports should be submitted to the server. Set through the `start` method or `setUploadToServer`. -**NOTE:** This API can only be used from the main process +**Note:** This API can only be used from the main process. ### `crashReporter.setUploadToServer(uploadToServer)` _macOS_ * `uploadToServer` Boolean _macOS_ - Whether reports should be submitted to the server -This would normally be controlled by user preferences. +This would normally be controlled by user preferences. This has no effect if +called before `start` is called. -**NOTE:** This API can only be used from the main process +**Note:** This API can only be used from the main process. ## Crash Report Payload From 0c73140b070f089389f534a528e37b2da57a1bed Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 28 Nov 2016 15:03:38 -0800 Subject: [PATCH 13/16] used -> called --- docs/api/crash-reporter.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index f9409bdfc3..a8a61025a5 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -75,7 +75,7 @@ ID. Returns `Boolean` - Whether reports should be submitted to the server. Set through the `start` method or `setUploadToServer`. -**Note:** This API can only be used from the main process. +**Note:** This API can only be called from the main process. ### `crashReporter.setUploadToServer(uploadToServer)` _macOS_ @@ -84,7 +84,7 @@ the `start` method or `setUploadToServer`. This would normally be controlled by user preferences. This has no effect if called before `start` is called. -**Note:** This API can only be used from the main process. +**Note:** This API can only be called from the main process. ## Crash Report Payload From 0b9a2f6be68add8345494742c94f8bfa770dd3c0 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 28 Nov 2016 15:06:03 -0800 Subject: [PATCH 14/16] Add autoSubmit crashReporter option --- docs/tutorial/planned-breaking-changes.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/tutorial/planned-breaking-changes.md b/docs/tutorial/planned-breaking-changes.md index ed3606f123..4e604945ef 100644 --- a/docs/tutorial/planned-breaking-changes.md +++ b/docs/tutorial/planned-breaking-changes.md @@ -40,6 +40,23 @@ clipboard.writeHtml() clipboard.writeHTML() ``` +## `crashReporter` + +```js +// Deprecated +crashReporter.start({ + companyName: 'Crashly', + submitURL: 'https://crash.server.com', + autoSubmit: true +}) +// Replace with +crashReporter.start({ + companyName: 'Crashly', + submitURL: 'https://crash.server.com', + uploadToServer: true +}) +``` + ## `nativeImage` ```js From 1c6e166af54ebf32225b3902dc61431067a00c45 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 28 Nov 2016 15:07:26 -0800 Subject: [PATCH 15/16] Value should be true on non-macOS platforms --- spec/api-crash-reporter-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/api-crash-reporter-spec.js b/spec/api-crash-reporter-spec.js index 86509f9f63..df4bbfa8ba 100644 --- a/spec/api-crash-reporter-spec.js +++ b/spec/api-crash-reporter-spec.js @@ -134,7 +134,7 @@ describe('crashReporter module', function () { crashReporter.setUploadToServer(false) assert.equal(crashReporter.getUploadToServer(), false) } else { - assert.equal(crashReporter.getUploadToServer(), false) + assert.equal(crashReporter.getUploadToServer(), true) } }) }) From 07994f50f3150973f8d8a2b5134a4dacffaec108 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 28 Nov 2016 15:11:50 -0800 Subject: [PATCH 16/16] Remove include now included in parent header --- atom/common/crash_reporter/crash_reporter_mac.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/atom/common/crash_reporter/crash_reporter_mac.mm b/atom/common/crash_reporter/crash_reporter_mac.mm index 877c68d1c2..d5107b5edb 100644 --- a/atom/common/crash_reporter/crash_reporter_mac.mm +++ b/atom/common/crash_reporter/crash_reporter_mac.mm @@ -13,7 +13,6 @@ #include "base/strings/string_piece.h" #include "base/strings/stringprintf.h" #include "base/strings/sys_string_conversions.h" -#include "vendor/crashpad/client/crash_report_database.h" #include "vendor/crashpad/client/crashpad_client.h" #include "vendor/crashpad/client/crashpad_info.h" #include "vendor/crashpad/client/settings.h"