From 36418bc29b15edf309ab378d8395b6aecb86505a Mon Sep 17 00:00:00 2001 From: Aubrey Keus Date: Wed, 5 Oct 2016 20:50:13 -0400 Subject: [PATCH 1/8] Add toggle to enable / disable crash reporting * Add toggle in advanced preferences to enable or disable crash reporting * Enable crash reporting on startup if preferences selected Auditors: @bbondy Test Plan: * Navigate to Preferences > Advanced * Toggle crash reporting to off (restart browser) * Load browser and select crash from debug menu -> Confirm crash report NOT sent to stats.brave.com * Load browser and navigate to Preferences > Advanced * Toggle crash reporting to on (restart browser) * Load browser and select crash from debug menu -> Confirm crash report SENT to stats.brave.com --- app/extensions/brave/locales/en-US/preferences.properties | 1 + app/index.js | 7 +++++-- js/about/preferences.js | 4 +++- js/constants/appConfig.js | 1 + js/constants/settings.js | 1 + 5 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/extensions/brave/locales/en-US/preferences.properties b/app/extensions/brave/locales/en-US/preferences.properties index e51a9b1116b..dca67ced4d1 100644 --- a/app/extensions/brave/locales/en-US/preferences.properties +++ b/app/extensions/brave/locales/en-US/preferences.properties @@ -222,3 +222,4 @@ enableAutofill=Enable Autofill importBrowserData=Import Browser Data importNow=Import now… clearAll=Clear all +sendCrashReports=Send anonymous crash reports to Brave (requires browser restart) \ No newline at end of file diff --git a/app/index.js b/app/index.js index a19164b5a5c..4894875b958 100644 --- a/app/index.js +++ b/app/index.js @@ -9,7 +9,6 @@ let ready = false // Setup the crash handling const CrashHerald = require('./crash-herald') -CrashHerald.init() const handleUncaughtError = (error) => { var message, ref, stack @@ -244,10 +243,14 @@ let flashInitialized = false // Some settings must be set right away on startup, those settings should be handled here. loadAppStatePromise.then((initialState) => { - const {HARDWARE_ACCELERATION_ENABLED, SMOOTH_SCROLL_ENABLED} = require('../js/constants/settings') + const {HARDWARE_ACCELERATION_ENABLED, SMOOTH_SCROLL_ENABLED, SEND_CRASH_REPORTS} = require('../js/constants/settings') if (initialState.settings[HARDWARE_ACCELERATION_ENABLED] === false) { app.disableHardwareAcceleration() } + if (initialState.settings[SEND_CRASH_REPORTS]) { + console.log('Crash reporting enabled') + CrashHerald.init() + } if (initialState.settings[SMOOTH_SCROLL_ENABLED] === false) { app.commandLine.appendSwitch('disable-smooth-scrolling') } diff --git a/js/about/preferences.js b/js/about/preferences.js index 40643ca6124..acf05146937 100644 --- a/js/about/preferences.js +++ b/js/about/preferences.js @@ -1281,6 +1281,7 @@ class AdvancedTab extends ImmutableComponent { + } @@ -1455,7 +1456,8 @@ class AboutPreferences extends React.Component { }) aboutActions.changeSetting(key, value) if (key === settings.DO_NOT_TRACK || key === settings.HARDWARE_ACCELERATION_ENABLED || - key === settings.PDFJS_ENABLED || key === settings.SMOOTH_SCROLL_ENABLED) { + key === settings.PDFJS_ENABLED || key === settings.SMOOTH_SCROLL_ENABLED || + key === settings.SEND_CRASH_REPORTS) { ipc.send(messages.PREFS_RESTART, key, value) } if (key === settings.PAYMENTS_ENABLED) { diff --git a/js/constants/appConfig.js b/js/constants/appConfig.js index d0c36fbc314..133ba8e835d 100644 --- a/js/constants/appConfig.js +++ b/js/constants/appConfig.js @@ -123,6 +123,7 @@ module.exports = { 'advanced.default-zoom-level': null, 'advanced.pdfjs-enabled': true, 'advanced.smooth-scroll-enabled': false, + 'advanced.send-crash-reports': true, 'shutdown.clear-history': false, 'shutdown.clear-downloads': false, 'shutdown.clear-cache': false, diff --git a/js/constants/settings.js b/js/constants/settings.js index 06d9dd3e8aa..7d2e5dd71a9 100644 --- a/js/constants/settings.js +++ b/js/constants/settings.js @@ -57,6 +57,7 @@ const settings = { PDFJS_ENABLED: 'advanced.pdfjs-enabled', DEFAULT_ZOOM_LEVEL: 'advanced.default-zoom-level', SMOOTH_SCROLL_ENABLED: 'advanced.smooth-scroll-enabled', + SEND_CRASH_REPORTS: 'advanced.send-crash-reports', ADBLOCK_CUSTOM_RULES: 'adblock.customRules' } From da530f726a80303e1e50d8a5789fe832a3e133b7 Mon Sep 17 00:00:00 2001 From: Aubrey Keus Date: Thu, 6 Oct 2016 13:25:09 -0400 Subject: [PATCH 2/8] Crash reporting initialization * Enable crash reporting on undefined or true value in setting * Update state documentation Auditors: @bbondy Test Plan: In previous commit --- app/index.js | 5 ++++- docs/state.md | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/index.js b/app/index.js index 4894875b958..bb042242e6c 100644 --- a/app/index.js +++ b/app/index.js @@ -244,12 +244,15 @@ let flashInitialized = false // Some settings must be set right away on startup, those settings should be handled here. loadAppStatePromise.then((initialState) => { const {HARDWARE_ACCELERATION_ENABLED, SMOOTH_SCROLL_ENABLED, SEND_CRASH_REPORTS} = require('../js/constants/settings') + console.log(initialState) if (initialState.settings[HARDWARE_ACCELERATION_ENABLED] === false) { app.disableHardwareAcceleration() } - if (initialState.settings[SEND_CRASH_REPORTS]) { + if (initialState.settings[SEND_CRASH_REPORTS] !== false) { console.log('Crash reporting enabled') CrashHerald.init() + } else { + console.log('Crash reporting disabled') } if (initialState.settings[SMOOTH_SCROLL_ENABLED] === false) { app.commandLine.appendSwitch('disable-smooth-scrolling') diff --git a/docs/state.md b/docs/state.md index 360c930578a..efb8f82469a 100644 --- a/docs/state.md +++ b/docs/state.md @@ -176,6 +176,7 @@ AppStore 'advanced.default-zoom-level': number, // the default zoom level for sites that have no specific setting 'advanced.pdfjs-enabled': boolean, // Whether or not to render PDF documents in the browser 'advanced.smooth-scroll-enabled': boolean, // false if smooth scrolling should be explicitly disabled + 'advanced.send-crash-reports': boolean, // true or undefined if crash reports should be sent 'shutdown.clear-history': boolean, // true to clear history on shutdown 'shutdown.clear-downloads': boolean, // true to clear downloads on shutdown 'shutdown.clear-cache': boolean, // true to clear cache on shutdown From d275aa8320c34af7368493b2f6662b53c4362355 Mon Sep 17 00:00:00 2001 From: Aubrey Keus Date: Thu, 6 Oct 2016 13:31:05 -0400 Subject: [PATCH 3/8] Removed spurious console.log --- app/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/index.js b/app/index.js index bb042242e6c..524db8e4efe 100644 --- a/app/index.js +++ b/app/index.js @@ -244,7 +244,6 @@ let flashInitialized = false // Some settings must be set right away on startup, those settings should be handled here. loadAppStatePromise.then((initialState) => { const {HARDWARE_ACCELERATION_ENABLED, SMOOTH_SCROLL_ENABLED, SEND_CRASH_REPORTS} = require('../js/constants/settings') - console.log(initialState) if (initialState.settings[HARDWARE_ACCELERATION_ENABLED] === false) { app.disableHardwareAcceleration() } From fc922e1d6ce95480f0cea1e8411e1881eed32e9c Mon Sep 17 00:00:00 2001 From: Aubrey Keus Date: Wed, 5 Oct 2016 20:50:13 -0400 Subject: [PATCH 4/8] Add toggle to enable / disable crash reporting * Add toggle in advanced preferences to enable or disable crash reporting * Enable crash reporting on startup if preferences selected Auditors: @bbondy Test Plan: * Navigate to Preferences > Advanced * Toggle crash reporting to off (restart browser) * Load browser and select crash from debug menu -> Confirm crash report NOT sent to stats.brave.com * Load browser and navigate to Preferences > Advanced * Toggle crash reporting to on (restart browser) * Load browser and select crash from debug menu -> Confirm crash report SENT to stats.brave.com --- app/extensions/brave/locales/en-US/preferences.properties | 1 + app/index.js | 7 +++++-- js/about/preferences.js | 4 +++- js/constants/appConfig.js | 1 + js/constants/settings.js | 1 + 5 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/extensions/brave/locales/en-US/preferences.properties b/app/extensions/brave/locales/en-US/preferences.properties index e51a9b1116b..dca67ced4d1 100644 --- a/app/extensions/brave/locales/en-US/preferences.properties +++ b/app/extensions/brave/locales/en-US/preferences.properties @@ -222,3 +222,4 @@ enableAutofill=Enable Autofill importBrowserData=Import Browser Data importNow=Import now… clearAll=Clear all +sendCrashReports=Send anonymous crash reports to Brave (requires browser restart) \ No newline at end of file diff --git a/app/index.js b/app/index.js index a19164b5a5c..4894875b958 100644 --- a/app/index.js +++ b/app/index.js @@ -9,7 +9,6 @@ let ready = false // Setup the crash handling const CrashHerald = require('./crash-herald') -CrashHerald.init() const handleUncaughtError = (error) => { var message, ref, stack @@ -244,10 +243,14 @@ let flashInitialized = false // Some settings must be set right away on startup, those settings should be handled here. loadAppStatePromise.then((initialState) => { - const {HARDWARE_ACCELERATION_ENABLED, SMOOTH_SCROLL_ENABLED} = require('../js/constants/settings') + const {HARDWARE_ACCELERATION_ENABLED, SMOOTH_SCROLL_ENABLED, SEND_CRASH_REPORTS} = require('../js/constants/settings') if (initialState.settings[HARDWARE_ACCELERATION_ENABLED] === false) { app.disableHardwareAcceleration() } + if (initialState.settings[SEND_CRASH_REPORTS]) { + console.log('Crash reporting enabled') + CrashHerald.init() + } if (initialState.settings[SMOOTH_SCROLL_ENABLED] === false) { app.commandLine.appendSwitch('disable-smooth-scrolling') } diff --git a/js/about/preferences.js b/js/about/preferences.js index 40643ca6124..acf05146937 100644 --- a/js/about/preferences.js +++ b/js/about/preferences.js @@ -1281,6 +1281,7 @@ class AdvancedTab extends ImmutableComponent { + } @@ -1455,7 +1456,8 @@ class AboutPreferences extends React.Component { }) aboutActions.changeSetting(key, value) if (key === settings.DO_NOT_TRACK || key === settings.HARDWARE_ACCELERATION_ENABLED || - key === settings.PDFJS_ENABLED || key === settings.SMOOTH_SCROLL_ENABLED) { + key === settings.PDFJS_ENABLED || key === settings.SMOOTH_SCROLL_ENABLED || + key === settings.SEND_CRASH_REPORTS) { ipc.send(messages.PREFS_RESTART, key, value) } if (key === settings.PAYMENTS_ENABLED) { diff --git a/js/constants/appConfig.js b/js/constants/appConfig.js index 88a7b20db2c..46ff517e6a2 100644 --- a/js/constants/appConfig.js +++ b/js/constants/appConfig.js @@ -124,6 +124,7 @@ module.exports = { 'advanced.default-zoom-level': null, 'advanced.pdfjs-enabled': true, 'advanced.smooth-scroll-enabled': false, + 'advanced.send-crash-reports': true, 'shutdown.clear-history': false, 'shutdown.clear-downloads': false, 'shutdown.clear-cache': false, diff --git a/js/constants/settings.js b/js/constants/settings.js index 06d9dd3e8aa..7d2e5dd71a9 100644 --- a/js/constants/settings.js +++ b/js/constants/settings.js @@ -57,6 +57,7 @@ const settings = { PDFJS_ENABLED: 'advanced.pdfjs-enabled', DEFAULT_ZOOM_LEVEL: 'advanced.default-zoom-level', SMOOTH_SCROLL_ENABLED: 'advanced.smooth-scroll-enabled', + SEND_CRASH_REPORTS: 'advanced.send-crash-reports', ADBLOCK_CUSTOM_RULES: 'adblock.customRules' } From d5042d8efddcb3335d13eb725e8660f059ece050 Mon Sep 17 00:00:00 2001 From: Aubrey Keus Date: Thu, 6 Oct 2016 13:25:09 -0400 Subject: [PATCH 5/8] Crash reporting initialization * Enable crash reporting on undefined or true value in setting * Update state documentation Auditors: @bbondy Test Plan: In previous commit --- app/index.js | 5 ++++- docs/state.md | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/index.js b/app/index.js index 4894875b958..bb042242e6c 100644 --- a/app/index.js +++ b/app/index.js @@ -244,12 +244,15 @@ let flashInitialized = false // Some settings must be set right away on startup, those settings should be handled here. loadAppStatePromise.then((initialState) => { const {HARDWARE_ACCELERATION_ENABLED, SMOOTH_SCROLL_ENABLED, SEND_CRASH_REPORTS} = require('../js/constants/settings') + console.log(initialState) if (initialState.settings[HARDWARE_ACCELERATION_ENABLED] === false) { app.disableHardwareAcceleration() } - if (initialState.settings[SEND_CRASH_REPORTS]) { + if (initialState.settings[SEND_CRASH_REPORTS] !== false) { console.log('Crash reporting enabled') CrashHerald.init() + } else { + console.log('Crash reporting disabled') } if (initialState.settings[SMOOTH_SCROLL_ENABLED] === false) { app.commandLine.appendSwitch('disable-smooth-scrolling') diff --git a/docs/state.md b/docs/state.md index 360c930578a..efb8f82469a 100644 --- a/docs/state.md +++ b/docs/state.md @@ -176,6 +176,7 @@ AppStore 'advanced.default-zoom-level': number, // the default zoom level for sites that have no specific setting 'advanced.pdfjs-enabled': boolean, // Whether or not to render PDF documents in the browser 'advanced.smooth-scroll-enabled': boolean, // false if smooth scrolling should be explicitly disabled + 'advanced.send-crash-reports': boolean, // true or undefined if crash reports should be sent 'shutdown.clear-history': boolean, // true to clear history on shutdown 'shutdown.clear-downloads': boolean, // true to clear downloads on shutdown 'shutdown.clear-cache': boolean, // true to clear cache on shutdown From 637a8869a2124ca9fce7ecb8d321c6a43fa841f6 Mon Sep 17 00:00:00 2001 From: Aubrey Keus Date: Thu, 6 Oct 2016 13:31:05 -0400 Subject: [PATCH 6/8] Removed spurious console.log --- app/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/index.js b/app/index.js index bb042242e6c..524db8e4efe 100644 --- a/app/index.js +++ b/app/index.js @@ -244,7 +244,6 @@ let flashInitialized = false // Some settings must be set right away on startup, those settings should be handled here. loadAppStatePromise.then((initialState) => { const {HARDWARE_ACCELERATION_ENABLED, SMOOTH_SCROLL_ENABLED, SEND_CRASH_REPORTS} = require('../js/constants/settings') - console.log(initialState) if (initialState.settings[HARDWARE_ACCELERATION_ENABLED] === false) { app.disableHardwareAcceleration() } From 86e6830169ec00ad8902efef793d075e3f4778ed Mon Sep 17 00:00:00 2001 From: Aubrey Keus Date: Fri, 7 Oct 2016 09:04:54 -0400 Subject: [PATCH 7/8] Handle renderer crash initialization using crash reporting preferences * Turn on renderer crash handling if selected in preferences and the platform is darwin Auditors: @bbondy, @bridiver Test Plan: In previous commit --- js/entry.js | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/js/entry.js b/js/entry.js index d3d0f43e2cf..eb88fb00a6f 100644 --- a/js/entry.js +++ b/js/entry.js @@ -21,11 +21,21 @@ require('../less/notificationBar.less') require('../less/addEditBookmark.less') require('../node_modules/font-awesome/css/font-awesome.css') -if (process.platform === 'darwin') { - // Setup the crash handling for mac renderer processes - // https://github.com/electron/electron/blob/master/docs/api/crash-reporter.md#crashreporterstartoptions - const CrashHerald = require('../app/crash-herald') - CrashHerald.init() +// Enable or disable crash reporting based on platform +const setupCrashReporting = () => { + if (process.platform === 'darwin') { + // Setup the crash handling for mac renderer processes + // https://github.com/electron/electron/blob/master/docs/api/crash-reporter.md#crashreporterstartoptions + console.log('Renderer crash reporting initialized') + require('../app/crash-herald').init() + } else { + console.log(`Unsupported crash reporting platform ${process.platform} for renderer crashes`) + } +} + +// Notify that renderer crash reporting is disabled +const disableCrashReporting = () => { + console.log('Disabling renderer crash reporting') } const React = require('react') @@ -77,6 +87,12 @@ window.addEventListener('beforeunload', function () { // get appStore from url ipc.on(messages.INITIALIZE_WINDOW, (e, disposition, appState, frames, initWindowState) => { + // Configure renderer crash reporting + if (appState.settings[require('./constants/settings').SEND_CRASH_REPORTS] !== false) { + setupCrashReporting() + } else { + disableCrashReporting() + } appStoreRenderer.state = Immutable.fromJS(appState) ReactDOM.render( , From 55783c3025b93fa05782819d46315764159018b8 Mon Sep 17 00:00:00 2001 From: Aubrey Keus Date: Sat, 8 Oct 2016 11:44:04 -0400 Subject: [PATCH 8/8] Removed inaccurate warning per @bridiver comment Auditor: @bbondy --- js/entry.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/js/entry.js b/js/entry.js index eb88fb00a6f..ff741d545c1 100644 --- a/js/entry.js +++ b/js/entry.js @@ -26,10 +26,8 @@ const setupCrashReporting = () => { if (process.platform === 'darwin') { // Setup the crash handling for mac renderer processes // https://github.com/electron/electron/blob/master/docs/api/crash-reporter.md#crashreporterstartoptions - console.log('Renderer crash reporting initialized') + console.log('macOS renderer crash reporting initialized') require('../app/crash-herald').init() - } else { - console.log(`Unsupported crash reporting platform ${process.platform} for renderer crashes`) } }