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

Intermittent crash if Confirmations library is called before being instantiated #4370

Merged
merged 2 commits into from
Jan 15, 2020

Conversation

tmancey
Copy link
Collaborator

@tmancey tmancey commented Jan 14, 2020

Fixes brave/brave-browser#7728

Submitter Checklist:

Test Plan:

  • Confirm no crashes occur for Confirmations (see the console log)
  • Confirm ads can be viewed and users are rewarded for a fresh install
  • Confirm ads can be viewed and users are rewarded for an existing user profile
  • Confirm ads can be viewed and users are rewarded for a recovered wallet
  • Confirm ads can be viewed and users are rewarded after ads are switched off and then on

QA will also run through the following which also includes manual passes for ads:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

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

rewards code looks good, didn't check other things

@tmancey tmancey merged commit 484851e into master Jan 15, 2020
@tmancey tmancey deleted the issues/7728 branch January 15, 2020 22:19
@tmancey
Copy link
Collaborator Author

tmancey commented Jan 15, 2020

Resolves Android crashes 1, 3, 5 and 6 for https://github.com/brave/browser-android-tabs/issues/2435 (which can also affecteds desktop users)

@LaurenWags
Copy link
Member

Verified passed with macOS 10.14.6 x64 using

Brave 1.5.40 Chromium: 79.0.3945.117 (Official Build) nightly (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS macOS Version 10.14.6 (Build 18G103)

Clean profile:

  • Confirmed on the Ads panel on brave://rewards the Estimated pending rewards and Ad notifications received this month are incrementing as expected for a fresh profile. (Staging)
  • Confirmed on the NTP widget that Estimated earnings so far this month is incrementing as expected for a fresh profile. (Staging)

Recovered wallet:

  • Confirmed on the Ads panel on brave://rewards the Estimated pending rewards incremented as expected for a recovered wallet. (Staging)
  • Confirmed on the NTP widget that Estimated earnings so far this month incremented as expected for a recovered wallet. (Staging)

Toggling Ads off/on:

  • Confirmed able to view ads after Ads are switched off and then back on. (Staging and production)
  • Confirmed on the Ads panel on brave://rewards the Estimated pending rewards and Ad notifications received this month are incrementing as expected after toggling Ads off and back on. (Staging and production)
  • Confirmed on the NTP widget that Estimated earnings so far this month is incrementing as expected after toggling Ads off and back on. (Staging and production)

Existing/Upgraded profile:

  • Confirmed on the Ads panel on brave://rewards the Estimated pending rewards and Ad notifications received this month are incrementing as expected for an upgraded/existing profile. (Production)
  • Confirmed on the NTP widget that Estimated earnings so far this month is incrementing as expected for an upgraded/existing profile. (Production)

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

Successfully merging this pull request may close these issues.

Intermittent crash if Confirmations library is called before being instantiated
4 participants