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

Splited p3a from crash report option from first run dialog #7978

Merged
merged 1 commit into from
Feb 18, 2021

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Feb 17, 2021

p3a prefs would not be affected by first run dialog.
crash report checkbox is unchecked by default.

fix brave/brave-browser#14183
fix brave/brave-browser#14160

image
image

Resolves

Submitter Checklist:

  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)
  • Requested a security/privacy review as needed

Reviewer Checklist:

  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

  1. Launch with clean profile
  2. Check crash report checkbox is un-checked by default
  3. Check settings reflects properly for crash report checkbox state
  4. Check crash report checkbox state doesn't affect p3a prefs state

@simonhong simonhong requested a review from a team as a code owner February 17, 2021 01:39
@simonhong simonhong self-assigned this Feb 17, 2021
p3a prefs would not be affected by first run dialog.
crash report checkbox is unchecked by default.

fix brave/brave-browser#14183
fix brave/brave-browser#14160
@iefremov
Copy link
Contributor

I recall we were thinking about different wording for crash reports, like "if things go wrong" or something :)

@karenkliu
Copy link

@iefremov I remember that too! No one proposed the official alternate string in the spec though so I left it as is.

Copy link

@karenkliu karenkliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great Simon! 👍

Copy link
Collaborator

@mkarolin mkarolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chromium_src changes LGTM.

@simonhong simonhong merged commit 0cad0ca into master Feb 18, 2021
@simonhong simonhong deleted the split_p3a_crash_report_firstrun branch February 18, 2021 00:36
@simonhong simonhong added this to the 1.22.x - Nightly milestone Feb 18, 2021
brave-builds pushed a commit that referenced this pull request Feb 18, 2021
@kjozwiak
Copy link
Member

kjozwiak commented Feb 23, 2021

Verification PASSED on macOS 11.1 x64 using the following build:

Brave | 1.22.39 Chromium: 89.0.4389.58 (Official Build) nightly (x86_64)
--- | ---
Revision | 1a139f28ecc27719439e37c6b1533cee999cb802-refs/branch-heads/4389@{#1134}
OS | macOS Version 11.1 (Build 20C69)

Test Case #1 (leaving crash report unchecked)

  • launched 1.22.39 CR: 89.0.4389.58 and ensured that Help improve Brave by automatically sending crash reports was not being selected by default
  • ensured that Automatically send completely private product analytics to Brave was enabled via brave://settings/privacy
  • ensured that Help improve Brave's features and performance was disabled
  • crashed Brave via brave://crash and ensured that the crash report appeared under brave://crashes but wasn't submitted
  • ensured that restarting Brave didn't submit the crash report via brave://crashes
  • ensured that clicking on Send now under brave://crashes sends the crash report to BackTrace
  • ensured that P3A is working/being updated via brave://local-state
Example Example Example
Screen Shot 2021-02-22 at 10 16 38 PM Screen Shot 2021-02-22 at 10 23 04 PM Screen Shot 2021-02-22 at 10 27 53 PM

Test Case #2 (leaving crash report selected)

  • launched 1.22.39 CR: 89.0.4389.58 and ensured that Help improve Brave by automatically sending crash reports was not being selected by default
  • selected Help improve Brave by automatically sending crash reports and clicked on Start Brave
  • ensured that Automatically send completely private product analytics to Brave was enabled via brave://settings/privacy
  • ensured that Help improve Brave's features and performance was enabled via brave://settings/privacy
  • crashed Brave via brave://crash and ensured that the crash report appeared under brave://crashes and was automatically submitted
  • ensured that P3A is working/being updated via brave://local-state
Example Example Example
Screen Shot 2021-02-22 at 10 49 45 PM Screen Shot 2021-02-22 at 10 50 00 PM Screen Shot 2021-02-22 at 10 50 17 PM

Test Case #3 (Onboarding)

Screen Shot 2021-02-22 at 10 31 26 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants