From 2d94955a901083b6e2c5a809800af892cdef7bf4 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Wed, 15 Sep 2021 09:52:44 +0900 Subject: [PATCH] Added don't ask checkbox to crash report permission ask dialog fix https://github.com/brave/brave-browser/issues/18103 User will not see this dialog anymore after checking this checkbox. --- app/brave_generated_resources.grd | 3 +++ browser/brave_local_state_prefs.cc | 1 + browser/brave_prefs_browsertest.cc | 2 ++ browser/metrics/metrics_reporting_util.cc | 7 +++++++ .../crash_report_permission_ask_dialog_view.cc | 15 ++++++++++++--- .../crash_report_permission_ask_dialog_view.h | 6 ++++++ common/pref_names.cc | 1 + common/pref_names.h | 1 + 8 files changed, 33 insertions(+), 3 deletions(-) diff --git a/app/brave_generated_resources.grd b/app/brave_generated_resources.grd index d6eafc4d7853..c62105673306 100644 --- a/app/brave_generated_resources.grd +++ b/app/brave_generated_resources.grd @@ -1167,6 +1167,9 @@ By installing this extension, you are agreeing to the Google Widevine Terms of U Block + + Don't ask again + Send crash reports automatically so Brave can prevent this issue from happening again? diff --git a/browser/brave_local_state_prefs.cc b/browser/brave_local_state_prefs.cc index 7762c39726ec..97f90f833619 100644 --- a/browser/brave_local_state_prefs.cc +++ b/browser/brave_local_state_prefs.cc @@ -98,6 +98,7 @@ void RegisterLocalStatePrefs(PrefRegistrySimple* registry) { dark_mode::RegisterBraveDarkModeLocalStatePrefs(registry); registry->RegisterBooleanPref(kDefaultBrowserPromptEnabled, true); + registry->RegisterBooleanPref(kAskCrashReportPermission, true); #endif #if BUILDFLAG(ENABLE_WIDEVINE) diff --git a/browser/brave_prefs_browsertest.cc b/browser/brave_prefs_browsertest.cc index 7b3d281fd028..07db3c57bc44 100644 --- a/browser/brave_prefs_browsertest.cc +++ b/browser/brave_prefs_browsertest.cc @@ -162,5 +162,7 @@ IN_PROC_BROWSER_TEST_F(BraveProfilePrefsBrowserTest, IN_PROC_BROWSER_TEST_F(BraveLocalStatePrefsBrowserTest, DefaultLocalStateTest) { EXPECT_TRUE(g_browser_process->local_state()->GetBoolean( kDefaultBrowserPromptEnabled)); + EXPECT_TRUE( + g_browser_process->local_state()->GetBoolean(kAskCrashReportPermission)); } #endif diff --git a/browser/metrics/metrics_reporting_util.cc b/browser/metrics/metrics_reporting_util.cc index d5bec5743923..b73de98e8e7c 100644 --- a/browser/metrics/metrics_reporting_util.cc +++ b/browser/metrics/metrics_reporting_util.cc @@ -9,8 +9,11 @@ #include "base/notreached.h" #include "brave/browser/metrics/brave_metrics_service_accessor.h" #include "brave/browser/metrics/buildflags/buildflags.h" +#include "brave/common/pref_names.h" +#include "chrome/browser/browser_process.h" #include "chrome/browser/metrics/metrics_reporting_state.h" #include "chrome/common/channel_info.h" +#include "components/prefs/pref_service.h" #include "components/version_info/channel.h" bool GetDefaultPrefValueForMetricsReporting() { @@ -34,6 +37,10 @@ bool ShouldShowCrashReportPermissionAskDialog() { return false; #endif + PrefService* local_prefs = g_browser_process->local_state(); + if (!local_prefs->GetBoolean(kAskCrashReportPermission)) + return false; + if (IsMetricsReportingPolicyManaged()) return false; diff --git a/browser/ui/views/crash_report_permission_ask_dialog_view.cc b/browser/ui/views/crash_report_permission_ask_dialog_view.cc index 99536f998f4b..507d714e2ea5 100644 --- a/browser/ui/views/crash_report_permission_ask_dialog_view.cc +++ b/browser/ui/views/crash_report_permission_ask_dialog_view.cc @@ -11,6 +11,7 @@ #include "base/threading/sequenced_task_runner_handle.h" #include "brave/app/vector_icons/vector_icons.h" #include "brave/browser/themes/theme_properties.h" +#include "brave/common/pref_names.h" #include "brave/grit/brave_generated_resources.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/metrics/metrics_reporting_state.h" @@ -22,10 +23,12 @@ #include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/common/webui_url_constants.h" #include "components/constrained_window/constrained_window_views.h" +#include "components/prefs/pref_service.h" #include "ui/base/l10n/l10n_util.h" #include "ui/base/models/image_model.h" #include "ui/base/theme_provider.h" #include "ui/views/background.h" +#include "ui/views/controls/button/checkbox.h" #include "ui/views/controls/image_view.h" #include "ui/views/controls/label.h" #include "ui/views/controls/styled_label.h" @@ -150,11 +153,11 @@ void CrashReportPermissionAskDialogView::CreateChildViews( if (offset != 0) header_label->AddStyleRange(gfx::Range(0, offset), default_style); - // Construct contents text area + // Construct contents area that includes main text and checkbox. auto* contents = AddChildView(std::make_unique()); contents->SetLayoutManager(std::make_unique( - views::BoxLayout::Orientation::kHorizontal, - gfx::Insets{0, kPadding + kChildSpacing, 0, 0})); + views::BoxLayout::Orientation::kVertical, + gfx::Insets{0, kPadding + kChildSpacing, 0, 0}, 5)); constexpr int kContentsTextFontSize = 13; auto* contents_label = contents->AddChildView(std::make_unique( l10n_util::GetStringUTF16( @@ -165,6 +168,9 @@ void CrashReportPermissionAskDialogView::CreateChildViews( contents_label->SetMultiLine(true); constexpr int kContentsLabelMaxWidth = 350; contents_label->SetMaximumWidth(kContentsLabelMaxWidth); + dont_ask_again_checkbox_ = contents->AddChildView( + std::make_unique(l10n_util::GetStringUTF16( + IDS_CRASH_REPORT_PERMISSION_ASK_DIALOG_DONT_ASK_TEXT))); // Construct footnote text area constexpr int kFootnoteVerticalPadding = 16; @@ -231,6 +237,9 @@ void CrashReportPermissionAskDialogView::OnAcceptButtonClicked() { } void CrashReportPermissionAskDialogView::OnWindowClosing() { + g_browser_process->local_state()->SetBoolean( + kAskCrashReportPermission, !dont_ask_again_checkbox_->GetChecked()); + // On macOS, this dialog is not destroyed properly when session crashed bubble // is launched directly. base::SequencedTaskRunnerHandle::Get()->PostTask( diff --git a/browser/ui/views/crash_report_permission_ask_dialog_view.h b/browser/ui/views/crash_report_permission_ask_dialog_view.h index 7ea6ddbf50b3..8ce35959d135 100644 --- a/browser/ui/views/crash_report_permission_ask_dialog_view.h +++ b/browser/ui/views/crash_report_permission_ask_dialog_view.h @@ -12,6 +12,10 @@ class Browser; +namespace views { +class Checkbox; +} // namespace views + class CrashReportPermissionAskDialogView : public views::DialogDelegateView { public: static void Show(Browser* browser); @@ -34,6 +38,8 @@ class CrashReportPermissionAskDialogView : public views::DialogDelegateView { void OnAcceptButtonClicked(); void OnWindowClosing(); void CreateChildViews(views::Widget* parent); + + views::Checkbox* dont_ask_again_checkbox_ = nullptr; }; #endif // BRAVE_BROWSER_UI_VIEWS_CRASH_REPORT_PERMISSION_ASK_DIALOG_VIEW_H_ diff --git a/common/pref_names.cc b/common/pref_names.cc index 7affec2f5c06..f6210f159688 100644 --- a/common/pref_names.cc +++ b/common/pref_names.cc @@ -94,6 +94,7 @@ const char kImportDialogExtensions[] = "import_dialog_extensions"; const char kImportDialogPayments[] = "import_dialog_payments"; const char kMRUCyclingEnabled[] = "brave.mru_cycling_enabled"; const char kTabsSearchShow[] = "brave.tabs_search_show"; +const char kAskCrashReportPermission[] = "brave.ask_crash_report_permission"; #if BUILDFLAG(ENABLE_BRAVE_VPN) const char kBraveVPNShowButton[] = "brave.brave_vpn.show_button"; diff --git a/common/pref_names.h b/common/pref_names.h index 85bd52037328..720552766a94 100644 --- a/common/pref_names.h +++ b/common/pref_names.h @@ -91,6 +91,7 @@ extern const char kSafetynetStatus[]; extern const char kDefaultBrowserLaunchingCount[]; extern const char kTabsSearchShow[]; +extern const char kAskCrashReportPermission[]; #if BUILDFLAG(ENABLE_BRAVE_VPN) extern const char kBraveVPNShowButton[];