-
Notifications
You must be signed in to change notification settings - Fork 965
Try trigger NTPSI ad on each NTP refresh. #14225
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
Conversation
67195c8
to
b8da1af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM++
components/ntp_background_images/browser/view_counter_service.cc
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ with one comment for f/u
aba4707
to
6f50cc1
Compare
Verification passed on
ads enabledVerified attempt to serve the ad is made every NTP load 1st NTP - ad is served (prefetched):
2nd NTP - ad is viewed (shown):
3rd NTP - attempt to serve the ad is made but blocked due frequency capping
4th NTP - attempt to serve the ad is made but blocked due frequency capping
5th NTP - attempt to serve the ad is made but blocked due frequency capping
6th NTP - ad is served
7th, 8th, 9th - ad is already served so we do nothing 10th and is shown
ads disabledVerified ads are shown but user is not paid 1st ad not shown |
We should trigger NTPSI ad on each refresh except cases:
Resolves brave/brave-browser#24107
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
,npm run lint
,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
Need to check that if there is no prefetched NTP ad, then each NTP refresh/open triggers NTP ad serving attempt.