Skip to content

Commit

Permalink
Added don't ask checkbox to crash report permission ask dialog
Browse files Browse the repository at this point in the history
fix brave/brave-browser#18103

User will not see this dialog anymore after checking this checkbox.
  • Loading branch information
simonhong committed Sep 15, 2021
1 parent 5d29af1 commit 2d94955
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 3 deletions.
3 changes: 3 additions & 0 deletions app/brave_generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -1167,6 +1167,9 @@ By installing this extension, you are agreeing to the Google Widevine Terms of U
<message name="IDS_CRASH_REPORT_PERMISSION_ASK_DIALOG_CANCEL_BUTTON_LABEL" desc="The text for cancel button">
Block
</message>
<message name="IDS_CRASH_REPORT_PERMISSION_ASK_DIALOG_DONT_ASK_TEXT" desc="The text for disabling dialog checkbox">
Don't ask again
</message>
<message name="IDS_CRASH_REPORT_PERMISSION_ASK_DIALOG_CONTENT_TEXT" desc="The contents for crash report ask dialog">
Send crash reports automatically so Brave can prevent this issue from happening again?
</message>
Expand Down
1 change: 1 addition & 0 deletions browser/brave_local_state_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions browser/brave_prefs_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions browser/metrics/metrics_reporting_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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;

Expand Down
15 changes: 12 additions & 3 deletions browser/ui/views/crash_report_permission_ask_dialog_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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<views::View>());
contents->SetLayoutManager(std::make_unique<views::BoxLayout>(
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<views::Label>(
l10n_util::GetStringUTF16(
Expand All @@ -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<views::Checkbox>(l10n_util::GetStringUTF16(
IDS_CRASH_REPORT_PERMISSION_ASK_DIALOG_DONT_ASK_TEXT)));

// Construct footnote text area
constexpr int kFootnoteVerticalPadding = 16;
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 6 additions & 0 deletions browser/ui/views/crash_report_permission_ask_dialog_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@

class Browser;

namespace views {
class Checkbox;
} // namespace views

class CrashReportPermissionAskDialogView : public views::DialogDelegateView {
public:
static void Show(Browser* browser);
Expand All @@ -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_
1 change: 1 addition & 0 deletions common/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
1 change: 1 addition & 0 deletions common/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand Down

0 comments on commit 2d94955

Please sign in to comment.