Skip to content

Commit

Permalink
Implement crash reporting permission ask dialog
Browse files Browse the repository at this point in the history
fix brave/brave-browser#1841

Launch crash reporting permission ask dialog on startup after crash.
Launch tab restore dialog after permission ask dialog is closed.

By overriding, ShowIfNotOffTheRecordProfileBrave() is called instead of
ShowIfNotOffTheRecordProfile().
ShowIfNotOffTheRecordProfileBrave() will launch crash reporting permission
ask dialog and then tab restore dialog is launched after permission ask dialog
is closed.
  • Loading branch information
simonhong committed Aug 2, 2021
1 parent 1bd5574 commit 90e475b
Show file tree
Hide file tree
Showing 21 changed files with 551 additions and 10 deletions.
22 changes: 22 additions & 0 deletions app/brave_generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,28 @@ By installing this extension, you are agreeing to the Google Widevine Terms of U
<message name="IDS_BRAVE_DEFAULT_BROWSER_DIALOG_CANCEL_BUTTON_LABEL" desc="The text for default browser dialog cancel button">
Not now
</message>
<!-- Brave crash report permission ask dialog -->
<message name="IDS_CRASH_REPORT_PERMISSION_ASK_DIALOG_OK_BUTTON_LABEL" desc="The text for ok button">
Allow
</message>
<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_CONTENT_TEXT" desc="The contents for crash report ask dialog">
Send crash reports automatically so Brave can prevent this issue from happening again?
</message>
<message name="IDS_CRASH_REPORT_PERMISSION_ASK_DIALOG_HEADER_TEXT" desc="The header text for crash report ask dialog">
<ph name="BROWSER_NAME">$1<ex>Brave Browser</ex></ph> quit unexpectedly.
</message>
<message name="IDS_CRASH_REPORT_PERMISSION_ASK_DIALOG_HEADER_TEXT_BROWSER_NAME_PART" desc="This is used for replacing BROWSER_NAME from above HEADER_TEXT.">
Brave Browser
</message>
<message name="IDS_CRASH_REPORT_PERMISSION_ASK_DIALOG_FOOTNOTE_TEXT" desc="The footnote text for crash report ask dialog.">
You can change <ph name="SETTING_TEXT">$1<ex>this setting</ex></ph> again at any time.
</message>
<message name="IDS_CRASH_REPORT_PERMISSION_ASK_DIALOG_FOOTNOTE_TEXT_SETTING_PART" desc="This is used for replacing SETTING_TEXT from above FOOTNOTE_TEXT.">
this setting
</message>
</if>

<!-- Brave Ads -->
Expand Down
1 change: 1 addition & 0 deletions app/vector_icons/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ aggregate_vector_icons("brave_vector_icons") {
"brave_ads_close_button.icon",
"brave_ads_dark_mode_info_button.icon",
"brave_ads_light_mode_info_button.icon",
"brave_sad.icon",
"download_unlock.icon",
"open_in_ipfs.icon",
"open_in_tor.icon",
Expand Down
50 changes: 50 additions & 0 deletions app/vector_icons/brave_sad.icon
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright (c) 2021 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at http://mozilla.org/MPL/2.0/.

CANVAS_DIMENSIONS, 24,
MOVE_TO, 22, 23,
H_LINE_TO, 2,
R_CUBIC_TO, -1.1f, 0, -2, -0.9f, -2, -2,
V_LINE_TO, 3,
R_CUBIC_TO, 0, -1.1f, 0.9f, -2, 2, -2,
R_H_LINE_TO, 20,
R_CUBIC_TO, 1.1f, 0, 2, 0.9f, 2, 2,
R_V_LINE_TO, 18,
R_CUBIC_TO, 0, 1.1f, -0.9f, 2, -2, 2,
CLOSE,
R_MOVE_TO, 0, -2,
R_LINE_TO, 0, -14,
H_LINE_TO, 2,
R_V_LINE_TO, 14,
R_H_LINE_TO, 20,
CLOSE,
MOVE_TO, 2, 3,
R_V_LINE_TO, 2,
R_H_LINE_TO, 20,
V_LINE_TO, 3,
H_LINE_TO, 2,
CLOSE,
R_MOVE_TO, 6.64f, 14.66f,
R_ARC_TO, 0.99f, 0.99f, 0, 0, 1, -1.41f, -0.13f,
R_ARC_TO, 1.03f, 1.03f, 0, 0, 1, 0.13f, -1.43f,
R_CUBIC_TO, 1.5f, -1.27f, 3.05f, -1.92f, 4.64f, -1.92f,
R_CUBIC_TO, 1.59f, 0, 3.14f, 0.66f, 4.64f, 1.93f,
R_CUBIC_TO, 0.42f, 0.36f, 0.48f, 1, 0.13f, 1.43f,
R_ARC_TO, 0.99f, 0.99f, 0, 0, 1, -1.41f, 0.13f,
CUBIC_TO, 14.2f, 16.67f, 13.08f, 16.2f, 12, 16.2f,
R_CUBIC_TO, -1.08f, 0, -2.2f, 0.47f, -3.36f, 1.46f,
CLOSE,
MOVE_TO, 9, 12.14f,
R_CUBIC_TO, 0.55f, 0, 1, -0.45f, 1, -1.01f,
R_CUBIC_TO, 0, -0.56f, -0.45f, -1.01f, -1, -1.01f,
R_CUBIC_TO, -0.55f, 0, -1, 0.45f, -1, 1.01f,
R_CUBIC_TO, 0, 0.56f, 0.45f, 1.02f, 1, 1.02f,
CLOSE,
R_MOVE_TO, 6, 0,
R_CUBIC_TO, 0.55f, 0, 1, -0.45f, 1, -1.01f,
R_CUBIC_TO, 0, -0.56f, -0.45f, -1.01f, -1, -1.01f,
R_CUBIC_TO, -0.55f, 0, -1, 0.45f, -1, 1.01f,
R_CUBIC_TO, 0, 0.56f, 0.45f, 1.02f, 1, 1.02f,
CLOSE
22 changes: 13 additions & 9 deletions browser/metrics/test/BUILD.gn → browser/metrics/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,23 @@ import("//testing/test.gni")

source_set("brave_metrics_unit_tests") {
testonly = true

sources = [ "metrics_reporting_util_unittest.cc" ]
deps = [
"//base/test:test_support",
"//brave/browser",
"//chrome/browser",
"//chrome/test:test_support",
"//content/test:test_support",
]

if (is_linux) {
sources =
[ "//brave/browser/metrics/metrics_reporting_util_unittest_linux.cc" ]
sources += [ "metrics_reporting_util_unittest_linux.cc" ]
configs += [ "//brave/build/linux:linux_channel_names" ]

deps = [
"//base/test:test_support",
"//brave/browser",
"//chrome/browser:browser",
deps += [
"//chrome/common:channel_info",
"//components/version_info:channel",
"//content/test:test_support",
]

configs += [ "//brave/build/linux:linux_channel_names" ]
}
}
11 changes: 11 additions & 0 deletions browser/metrics/brave_metrics_service_accessor.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* Copyright (c) 2021 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/browser/metrics/brave_metrics_service_accessor.h"
#include "chrome/browser/browser_process.h"

bool BraveMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled() {
return IsMetricsReportingEnabled(g_browser_process->local_state());
}
21 changes: 21 additions & 0 deletions browser/metrics/brave_metrics_service_accessor.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/* Copyright (c) 2021 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_BROWSER_METRICS_BRAVE_METRICS_SERVICE_ACCESSOR_H_
#define BRAVE_BROWSER_METRICS_BRAVE_METRICS_SERVICE_ACCESSOR_H_

#include "components/metrics/metrics_service_accessor.h"

// This class limits and documents access to metrics service helper methods.
// Since these methods are private, each user has to be explicitly declared
// as a 'friend' below.
class BraveMetricsServiceAccessor : public metrics::MetricsServiceAccessor {
private:
friend bool ShouldShowCrashReportPermissionAskDialog();

static bool IsMetricsAndCrashReportingEnabled();
};

#endif // BRAVE_BROWSER_METRICS_BRAVE_METRICS_SERVICE_ACCESSOR_H_
12 changes: 12 additions & 0 deletions browser/metrics/metrics_reporting_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#include "base/logging.h"
#include "base/notreached.h"
#include "brave/browser/metrics/brave_metrics_service_accessor.h"
#include "chrome/browser/metrics/metrics_reporting_state.h"
#include "chrome/common/channel_info.h"
#include "components/version_info/channel.h"

Expand All @@ -25,3 +27,13 @@ bool GetDefaultPrefValueForMetricsReporting() {
return false;
}
}

bool ShouldShowCrashReportPermissionAskDialog() {
if (IsMetricsReportingPolicyManaged())
return false;

if (BraveMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled())
return false;

return true;
}
1 change: 1 addition & 0 deletions browser/metrics/metrics_reporting_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@
#define BRAVE_BROWSER_METRICS_METRICS_REPORTING_UTIL_H_

bool GetDefaultPrefValueForMetricsReporting();
bool ShouldShowCrashReportPermissionAskDialog();

#endif // BRAVE_BROWSER_METRICS_METRICS_REPORTING_UTIL_H_
27 changes: 27 additions & 0 deletions browser/metrics/metrics_reporting_util_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/* Copyright (c) 2021 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include <memory>

#include "brave/browser/metrics/metrics_reporting_util.h"
#include "chrome/test/base/scoped_testing_local_state.h"
#include "chrome/test/base/testing_browser_process.h"
#include "components/metrics/metrics_pref_names.h"
#include "components/prefs/pref_service.h"
#include "testing/gtest/include/gtest/gtest.h"

TEST(MetricsUtilTest, CrashReportPermissionAskDialogTest) {
std::unique_ptr<ScopedTestingLocalState> local_state =
std::make_unique<ScopedTestingLocalState>(
TestingBrowserProcess::GetGlobal());

g_browser_process->local_state()->SetBoolean(
metrics::prefs::kMetricsReportingEnabled, false);
EXPECT_TRUE(ShouldShowCrashReportPermissionAskDialog());

g_browser_process->local_state()->SetBoolean(
metrics::prefs::kMetricsReportingEnabled, true);
EXPECT_FALSE(ShouldShowCrashReportPermissionAskDialog());
}
2 changes: 2 additions & 0 deletions browser/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ brave_chrome_browser_sources = [
"//brave/browser/browser_context_keyed_service_factories.h",
"//brave/browser/geolocation/brave_geolocation_permission_context_delegate.cc",
"//brave/browser/geolocation/brave_geolocation_permission_context_delegate.h",
"//brave/browser/metrics/brave_metrics_service_accessor.cc",
"//brave/browser/metrics/brave_metrics_service_accessor.h",
"//brave/browser/metrics/metrics_reporting_util.cc",
"//brave/browser/metrics/metrics_reporting_util.h",
"//brave/browser/update_util.cc",
Expand Down
4 changes: 4 additions & 0 deletions browser/themes/theme_properties.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ absl::optional<SkColor> MaybeGetDefaultColorForBraveLightUi(int id) {
return kLightToolbarIcon;
case ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON_INACTIVE:
return color_utils::AlphaBlend(kLightToolbarIcon, kLightToolbar, 0.3f);
case BraveThemeProperties::COLOR_ICON_BASE:
return SkColorSetRGB(0x49, 0x50, 0x57);
#if BUILDFLAG(ENABLE_SIDEBAR)
case BraveThemeProperties::COLOR_SIDEBAR_BACKGROUND:
return SkColorSetRGB(0xF3, 0xF3, 0xF5);
Expand Down Expand Up @@ -128,6 +130,8 @@ absl::optional<SkColor> MaybeGetDefaultColorForBraveDarkUi(int id) {
return kDarkToolbarIcon;
case ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON_INACTIVE:
return color_utils::AlphaBlend(kDarkToolbarIcon, kDarkToolbar, 0.3f);
case BraveThemeProperties::COLOR_ICON_BASE:
return SkColorSetRGB(0xC2, 0xC4, 0xCF);
#if BUILDFLAG(ENABLE_SIDEBAR)
case BraveThemeProperties::COLOR_SIDEBAR_BACKGROUND:
return kDarkToolbar;
Expand Down
1 change: 1 addition & 0 deletions browser/themes/theme_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ enum TestProperty {
enum ThemeProperties {
BRAVE_THEME_PROPERTIES_START = 10000,
COLOR_BOOKMARK_BAR_INSTRUCTIONS_TEXT = BRAVE_THEME_PROPERTIES_START,
COLOR_ICON_BASE,
#if BUILDFLAG(ENABLE_SIDEBAR)
COLOR_SIDEBAR_ADD_BUTTON_DISABLED,
COLOR_SIDEBAR_BACKGROUND,
Expand Down
4 changes: 4 additions & 0 deletions browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ source_set("ui") {
"content_settings/brave_content_setting_image_models.h",
"omnibox/brave_omnibox_client_impl.cc",
"omnibox/brave_omnibox_client_impl.h",
"session_crashed_bubble_brave.cc",
"startup/default_brave_browser_prompt.cc",
"startup/default_brave_browser_prompt.h",
"toolbar/brave_app_menu_model.cc",
Expand Down Expand Up @@ -132,6 +133,8 @@ source_set("ui") {
"views/brave_default_browser_dialog_view.h",
"views/brave_layout_provider.cc",
"views/brave_layout_provider.h",
"views/crash_report_permission_ask_dialog_view.cc",
"views/crash_report_permission_ask_dialog_view.h",
"views/download/brave_download_item_view.cc",
"views/download/brave_download_item_view.h",
"views/frame/brave_browser_view.cc",
Expand Down Expand Up @@ -542,6 +545,7 @@ source_set("ui") {

if (toolkit_views) {
deps += [
"//components/constrained_window",
"//ui/events",
"//ui/views",
]
Expand Down
2 changes: 2 additions & 0 deletions browser/ui/browser_dialogs.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ class Browser;
namespace brave {

void ShowDefaultBrowserDialog(Browser* browser);
// Tab restore dialog will be launched after ask dialog is closed.
void ShowCrashReportPermissionAskDialog(Browser* browser);

} // namespace brave

Expand Down
20 changes: 20 additions & 0 deletions browser/ui/session_crashed_bubble_brave.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* Copyright (c) 2021 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/browser/metrics/metrics_reporting_util.h"
#include "brave/browser/ui/browser_dialogs.h"
#include "chrome/browser/ui/session_crashed_bubble.h"

// static
void SessionCrashedBubble::ShowIfNotOffTheRecordProfileBrave(Browser* browser) {
// If crash report permission ask dialog is launched, tab restore bubble will
// be shown after closing aks dialog.
if (ShouldShowCrashReportPermissionAskDialog()) {
brave::ShowCrashReportPermissionAskDialog(browser);
return;
}

ShowIfNotOffTheRecordProfile(browser);
}
36 changes: 36 additions & 0 deletions browser/ui/views/crash_report_permission_ask_dialog_browsertest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/* Copyright (c) 2021 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/browser/metrics/metrics_reporting_util.h"
#include "brave/browser/ui/brave_browser.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/ui/session_crashed_bubble.h"
#include "chrome/browser/ui/test/test_browser_dialog.h"
#include "components/metrics/metrics_pref_names.h"
#include "components/prefs/pref_service.h"
#include "content/public/test/browser_test.h"

class CrashReportPermissionAskDialogTest : public DialogBrowserTest {
public:
CrashReportPermissionAskDialogTest() = default;
~CrashReportPermissionAskDialogTest() override = default;

// TestBrowserUi:
void ShowUi(const std::string& name) override {
SessionCrashedBubble::ShowIfNotOffTheRecordProfileBrave(browser());
}
};

IN_PROC_BROWSER_TEST_F(CrashReportPermissionAskDialogTest, InvokeUi_Dialog) {
// When reporting is disabled, crash report permission ask dialog is launched.
g_browser_process->local_state()->SetBoolean(
metrics::prefs::kMetricsReportingEnabled, false);
ShowAndVerifyUi();

// When reporting is enabled, tab restore dialog is launched.
g_browser_process->local_state()->SetBoolean(
metrics::prefs::kMetricsReportingEnabled, true);
ShowAndVerifyUi();
}
Loading

0 comments on commit 90e475b

Please sign in to comment.