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

Remove dependency on the Rewards extension #13719

Merged
merged 1 commit into from
Jul 8, 2022

Conversation

zenparsing
Copy link
Collaborator

@zenparsing zenparsing commented Jun 9, 2022

Resolves brave/brave-browser#22423

High-level overview:

The goal of this change is to ensure that all Rewards features can be run without loading the Rewards built-in extension. Currently, the Rewards extension is used to:

  • Display the Rewards action button with a badge containing a notification count or creator "checkmark".
  • Display the Rewards panel.
  • Display the Brave Talk opt-in panel.
  • Record and store information about the creator associated with a given tab.

In this PR:

  • RewardsTabHelper is modified to take responsibility for recording tab/creator activity.
  • A desktop-only browser helper RewardsPanelCoordinator is added to route "panel requests" to the appropriate handler. Code that previously loaded the Rewards extension and called BraveActionAPI::ShowActionUI have been refactored to use this coordinator.
  • A new native button class BraveRewardsActionView is added to display the Rewards icon button.
  • A MojoBubbleWebUIController is added to display the Rewards panel (rewards_panel_ui.h).
  • The Rewards panel front-end is modified to work in both extension and WebUI contexts.
  • The Rewards panel front-end is modified to allow displaying the Brave Talk opt-in.
  • A feature flag is introduced (named "Use WebUI Rewards Panel") that, when enabled, will cause the browser to display the Rewards panel using WebUI and the Rewards button using BraveRewardsActionView. When enabled, the Rewards extension should not need to be loaded.

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • 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)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • 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:

Regression Testing

The following cases should be tested for regressions with the feature flag off:

  • All strings on the Rewards panel are displayed properly.
  • Publisher info is displayed properly in the panel for a variety of publishers.
  • After enabling Rewards from the NTP, the panel is opened with the Rewards tour displayed.
  • Adaptive captcha is displayed correctly when the user clicks on the Adaptive captcha tooltip.
  • On the adaptive captcha UI, the support URL is opened correctly.
  • Grant captchas can be opened from the NTP and from the Rewards page.
  • Button "checkmark" state and publisher info is correct for Greaselion publishers.
  • The Rewards panel can be opened from the settings page when the user has not enabled Rewards.

With the feature flag on, the following cases should be tested in addition to the cases above:

  • For a variety of publishers, the Rewards icon shows the correct "checkmark" badge.
  • The Rewards icon shows the correct notification count badge when there are Rewards notifications.
  • When the Rewards icon is hidden in settings, the Rewards icon does not appear.
  • When the Rewards icon is hidden in settings, the panel can still be opened from the NTP and Rewards page as indicated above.
  • When in a "private" context (i.e. a private window or tor window), the Rewards icon does not appear.

@zenparsing zenparsing requested review from a team as code owners June 9, 2022 19:38
@github-actions github-actions bot added CI/storybook-url Deploy storybook and provide a unique URL for each build potential-layer-violation-fixes This PR touches a BUILD.gn file with check_includes=false labels Jun 9, 2022
@zenparsing zenparsing force-pushed the ksmith-rewards-panel-webui branch from ed91fbf to 3461375 Compare June 9, 2022 19:55
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@zenparsing zenparsing force-pushed the ksmith-rewards-panel-webui branch from 3461375 to 5268471 Compare June 9, 2022 23:51
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@zenparsing zenparsing force-pushed the ksmith-rewards-panel-webui branch from 5268471 to 1c2592b Compare June 10, 2022 14:20
@zenparsing zenparsing requested a review from a team as a code owner June 10, 2022 14:20
@zenparsing zenparsing removed the CI/storybook-url Deploy storybook and provide a unique URL for each build label Jun 10, 2022
@zenparsing zenparsing force-pushed the ksmith-rewards-panel-webui branch 2 times, most recently from ca047fb to d45a8ff Compare June 13, 2022 14:56
@zenparsing zenparsing requested review from petemill and bridiver June 14, 2022 14:00
@zenparsing zenparsing force-pushed the ksmith-rewards-panel-webui branch 2 times, most recently from af35cc1 to 1c54006 Compare June 14, 2022 17:46
@zenparsing zenparsing changed the title [WIP] Remove dependency on the Rewards extension Remove dependency on the Rewards extension Jun 14, 2022
@zenparsing zenparsing force-pushed the ksmith-rewards-panel-webui branch from 1c54006 to b543123 Compare June 14, 2022 19:21
@zenparsing zenparsing force-pushed the ksmith-rewards-panel-webui branch 3 times, most recently from e711a51 to 19bfbe4 Compare June 18, 2022 13:57
test/BUILD.gn Outdated Show resolved Hide resolved
@zenparsing zenparsing force-pushed the ksmith-rewards-panel-webui branch 2 times, most recently from f84fabf to 649af3a Compare June 30, 2022 15:40
@zenparsing zenparsing force-pushed the ksmith-rewards-panel-webui branch 8 times, most recently from aab1709 to b07c19b Compare July 5, 2022 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
potential-layer-violation-fixes This PR touches a BUILD.gn file with check_includes=false
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate away from Rewards Extension to WebUI: Make background page of Reward Extension non-persistent
8 participants