Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added don't ask checkbox to crash report permission ask dialog #10093

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -708,6 +708,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 @@ -970,6 +971,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