Skip to content

Commit

Permalink
Merge pull request #10093 from brave/add_dont_ask_checkbox_to_crash_r…
Browse files Browse the repository at this point in the history
…eport_permission_ask_dialog

Added don't ask checkbox to crash report permission ask dialog
  • Loading branch information
bsclifton authored Sep 15, 2021
2 parents 791f320 + 25f4916 commit 2fbc667
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 9 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
5 changes: 5 additions & 0 deletions browser/brave_local_state_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "base/values.h"
#include "brave/browser/brave_stats/brave_stats_updater.h"
#include "brave/browser/metrics/buildflags/buildflags.h"
#include "brave/browser/metrics/metrics_reporting_util.h"
#include "brave/browser/themes/brave_dark_mode_utils.h"
#include "brave/common/pref_names.h"
Expand Down Expand Up @@ -100,6 +101,10 @@ void RegisterLocalStatePrefs(PrefRegistrySimple* registry) {
registry->RegisterBooleanPref(kDefaultBrowserPromptEnabled, true);
#endif

#if BUILDFLAG(ENABLE_CRASH_DIALOG)
registry->RegisterBooleanPref(kDontAskForCrashReporting, false);
#endif

#if BUILDFLAG(ENABLE_WIDEVINE)
RegisterWidevineLocalstatePrefs(registry);
#endif
Expand Down
10 changes: 8 additions & 2 deletions browser/brave_prefs_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/browser/ethereum_remote_client/buildflags/buildflags.h"
#include "brave/browser/metrics/buildflags/buildflags.h"
#include "brave/common/pref_names.h"
#include "brave/components/brave_rewards/common/pref_names.h"
#include "brave/components/brave_shields/common/pref_names.h"
Expand Down Expand Up @@ -158,9 +159,14 @@ IN_PROC_BROWSER_TEST_F(BraveProfilePrefsBrowserTest,
prefs::kHideWebStoreIcon));
}

#if !defined(OS_ANDROID)
IN_PROC_BROWSER_TEST_F(BraveLocalStatePrefsBrowserTest, DefaultLocalStateTest) {
#if !defined(OS_ANDROID)
EXPECT_TRUE(g_browser_process->local_state()->GetBoolean(
kDefaultBrowserPromptEnabled));
}
#endif

#if BUILDFLAG(ENABLE_CRASH_DIALOG)
EXPECT_FALSE(
g_browser_process->local_state()->GetBoolean(kDontAskForCrashReporting));
#endif
}
2 changes: 1 addition & 1 deletion browser/metrics/buildflags/buildflags.gni
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
# You can obtain one at http://mozilla.org/MPL/2.0/.

declare_args() {
enable_crash_dialog = is_official_build
enable_crash_dialog = !is_android && is_official_build
}
13 changes: 10 additions & 3 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 @@ -30,9 +33,10 @@ bool GetDefaultPrefValueForMetricsReporting() {
}

bool ShouldShowCrashReportPermissionAskDialog() {
#if !BUILDFLAG(ENABLE_CRASH_DIALOG)
return false;
#endif
#if BUILDFLAG(ENABLE_CRASH_DIALOG)
PrefService* local_prefs = g_browser_process->local_state();
if (local_prefs->GetBoolean(kDontAskForCrashReporting))
return false;

if (IsMetricsReportingPolicyManaged())
return false;
Expand All @@ -41,4 +45,7 @@ bool ShouldShowCrashReportPermissionAskDialog() {
return false;

return true;
#else
return false;
#endif
}
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(
kDontAskForCrashReporting, 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 kDontAskForCrashReporting[] = "brave.dont_ask_for_crash_reporting";

#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 kDontAskForCrashReporting[];

#if BUILDFLAG(ENABLE_BRAVE_VPN)
extern const char kBraveVPNShowButton[];
Expand Down
2 changes: 2 additions & 0 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,7 @@ if (!is_android) {
"//brave/browser/brave_wallet:tab_helper",
"//brave/browser/browsing_data:browser_tests",
"//brave/browser/farbling:browser_tests",
"//brave/browser/metrics/buildflags",
"//brave/browser/permissions:browser_tests",
"//brave/browser/tor:browser_tests",
"//brave/browser/ui:browser_tests",
Expand Down Expand Up @@ -971,6 +972,7 @@ if (!is_android) {
"//brave/app/theme:brave_unscaled_resources_grit",
"//brave/browser/brave_ads",
"//brave/browser/ethereum_remote_client/buildflags",
"//brave/browser/metrics/buildflags",
"//brave/browser/net",
"//brave/browser/themes",
"//brave/common:pref_names",
Expand Down

0 comments on commit 2fbc667

Please sign in to comment.