From 3c6abb0b62f5dba017c68a3e0e4db354a50e5c12 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Wed, 17 Feb 2021 10:35:27 +0900 Subject: [PATCH] Splited p3a from crash report option from first run dialog p3a prefs would not be affected by first run dialog. crash report checkbox is unchecked by default. fix https://github.com/brave/brave-browser/issues/14183 fix https://github.com/brave/brave-browser/issues/14160 --- app/brave_generated_resources.grd | 9 ++- browser/first_run/BUILD.gn | 16 ++++++ browser/first_run/first_run_unittest.cc | 11 ++++ .../chrome/browser/first_run/first_run.cc | 19 +++++++ .../browser/ui/cocoa/first_run_dialog.mm | 55 ------------------- .../browser/ui/views/first_run_dialog.cc | 18 ------ test/BUILD.gn | 1 + 7 files changed, 51 insertions(+), 78 deletions(-) create mode 100644 browser/first_run/BUILD.gn create mode 100644 browser/first_run/first_run_unittest.cc create mode 100644 chromium_src/chrome/browser/first_run/first_run.cc delete mode 100644 chromium_src/chrome/browser/ui/cocoa/first_run_dialog.mm diff --git a/app/brave_generated_resources.grd b/app/brave_generated_resources.grd index 6a01f3453e58..ee057734871a 100644 --- a/app/brave_generated_resources.grd +++ b/app/brave_generated_resources.grd @@ -1093,16 +1093,15 @@ up to 6x faster on major news sites. Reset Brave Rewards data... - - Help improve Brave by sending crash reports and -completely anonymised, private product analytics. + + Help improve Brave by automatically sending crash reports Set Brave as your default browser - - Help improve $1Brave by sending crash reports and completely anonymised, private product analytics. + + Help improve $1Brave by automatically sending crash reports diff --git a/browser/first_run/BUILD.gn b/browser/first_run/BUILD.gn new file mode 100644 index 000000000000..24e46da74c68 --- /dev/null +++ b/browser/first_run/BUILD.gn @@ -0,0 +1,16 @@ +# 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/. + +source_set("unit_tests") { + testonly = true + if (!is_android) { + sources = [ "first_run_unittest.cc" ] + + deps = [ + "//chrome/browser", + "//testing/gtest", + ] + } +} diff --git a/browser/first_run/first_run_unittest.cc b/browser/first_run/first_run_unittest.cc new file mode 100644 index 000000000000..5b94317e5abc --- /dev/null +++ b/browser/first_run/first_run_unittest.cc @@ -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 "chrome/browser/first_run/first_run.h" +#include "testing/gtest/include/gtest/gtest.h" + +TEST(FirstRunTest, BasicTest) { + EXPECT_TRUE(first_run::IsMetricsReportingOptIn()); +} diff --git a/chromium_src/chrome/browser/first_run/first_run.cc b/chromium_src/chrome/browser/first_run/first_run.cc new file mode 100644 index 000000000000..d21c1f0d8743 --- /dev/null +++ b/chromium_src/chrome/browser/first_run/first_run.cc @@ -0,0 +1,19 @@ +/* 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/. */ + +#define IsMetricsReportingOptIn IsMetricsReportingOptIn_UnUsed +#include "../../../../../chrome/browser/first_run/first_run.cc" +#undef IsMetricsReportingOptIn + +namespace first_run { + +// This is only used for determine whether crash report checkbox in the first +// run dialog is checked or not by default. See the comments of this upstream +// function. Returning true means crash report is unchecked by default. +bool IsMetricsReportingOptIn() { + return true; +} + +} // namespace first_run diff --git a/chromium_src/chrome/browser/ui/cocoa/first_run_dialog.mm b/chromium_src/chrome/browser/ui/cocoa/first_run_dialog.mm deleted file mode 100644 index b058ad641fe5..000000000000 --- a/chromium_src/chrome/browser/ui/cocoa/first_run_dialog.mm +++ /dev/null @@ -1,55 +0,0 @@ -/* 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/components/p3a/buildflags.h" -#include "chrome/browser/browser_process.h" -#include "components/prefs/pref_service.h" - -#if BUILDFLAG(BRAVE_P3A_ENABLED) -#include "brave/components/p3a/pref_names.h" -#endif - -#define ShowFirstRunDialog ShowFirstRunDialog_UnUsed -#include "../../../../../../chrome/browser/ui/cocoa/first_run_dialog.mm" -#undef ShowFirstRunDialog - -namespace { - -// Copied ShowFirstRunModal from upstream and p3a prefs part added. -void ShowFirstRunModalBrave(Profile* profile) { - base::scoped_nsobject dialog( - [[FirstRunDialogController alloc] init]); - - [dialog.get() showWindow:nil]; - - // If the dialog asked the user to opt-in for stats and crash reporting, - // record the decision and enable the crash reporter if appropriate. - bool consent_given = [dialog.get() isStatsReportingEnabled]; - ChangeMetricsReportingState(consent_given); - -#if BUILDFLAG(BRAVE_P3A_ENABLED) - PrefService* local_state = g_browser_process->local_state(); - local_state->SetBoolean(brave::kP3AEnabled, consent_given); -#endif - - // If selected, set as default browser. Skip in automated tests so that an OS - // dialog confirming the default browser choice isn't left on screen. - BOOL make_default_browser = - [dialog.get() isMakeDefaultBrowserEnabled] && - !base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType); - if (make_default_browser) { - bool success = shell_integration::SetAsDefaultBrowser(); - DCHECK(success); - } -} -} // namespace - -namespace first_run { - -void ShowFirstRunDialog(Profile* profile) { - ShowFirstRunModalBrave(profile); -} - -} // namespace first_run diff --git a/chromium_src/chrome/browser/ui/views/first_run_dialog.cc b/chromium_src/chrome/browser/ui/views/first_run_dialog.cc index b0efdd2095c5..3e7c0911aa6c 100644 --- a/chromium_src/chrome/browser/ui/views/first_run_dialog.cc +++ b/chromium_src/chrome/browser/ui/views/first_run_dialog.cc @@ -6,22 +6,6 @@ #include "brave/grit/brave_generated_resources.h" #include "chrome/grit/chromium_strings.h" #include "chrome/grit/generated_resources.h" -#include "ui/views/controls/button/checkbox.h" - -namespace views { -// Override to call SetMultiLine(). -// The label of crash report checkbox should be formatted to 2 lines. -// Otherwise, dialog width is too long. -class MultiLineCheckBox : public views::Checkbox { - public: - explicit MultiLineCheckBox(const base::string16& label) : Checkbox(label) { - SetMultiLine(true); - } - ~MultiLineCheckBox() override = default; - MultiLineCheckBox(const MultiLineCheckBox&) = delete; - MultiLineCheckBox& operator=(const MultiLineCheckBox&) = delete; -}; -} // namespace views // Replaced string here instead of by running 'npm run chromium_rebase_l10n' // because same string is used from other IDS_XXX.. @@ -29,10 +13,8 @@ class MultiLineCheckBox : public views::Checkbox { #undef IDS_FR_CUSTOMIZE_DEFAULT_BROWSER #define IDS_FR_ENABLE_LOGGING IDS_FR_ENABLE_LOGGING_BRAVE #define IDS_FR_CUSTOMIZE_DEFAULT_BROWSER IDS_FR_CUSTOMIZE_DEFAULT_BROWSER_BRAVE -#define Checkbox MultiLineCheckBox #include "../../../../../../chrome/browser/ui/views/first_run_dialog.cc" #undef IDS_FR_ENABLE_LOGGING #undef IDS_FR_CUSTOMIZE_DEFAULT_BROWSER -#undef Checkbox diff --git a/test/BUILD.gn b/test/BUILD.gn index 639734a5ec0f..babf6c440672 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -127,6 +127,7 @@ test("brave_unit_tests") { "//brave/browser/browsing_data", "//brave/browser/content_settings:unit_tests", "//brave/browser/download", + "//brave/browser/first_run:unit_tests", "//brave/browser/metrics/test:brave_metrics_unit_tests", "//brave/browser/net", "//brave/browser/new_tab:unittest",