-
Notifications
You must be signed in to change notification settings - Fork 888
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
Implement NTP Custom homepage ("super referral") #4866
Conversation
7fe98a6
to
7c6dfeb
Compare
dadc4ef
to
ed7bcf0
Compare
f25f3c5
to
1826181
Compare
Note: This is not yet ready to review. This PR works well for fresh install but not work properly for existing users. Implementation should be slightly modified after fixing brave/brave-browser#8603 |
bd0743a
to
8a5aefb
Compare
Ready to review. |
8022ac4
to
83226b8
Compare
f6c73cd
to
bf9ea53
Compare
|
components/ntp_background_images/browser/ntp_background_images_source_unittest.cc
Show resolved
Hide resolved
It should ignore for in-effective images data when it gets update.
W/o this, we could display SI data for super referral.
To do this, we should check existing install's promo code status properly. at launching time. If referal code is empty and referral service's is not initialized state, we should wait to get promo code. If referal service has already initialized state at launch time, we can set this is not SR install. SR is only effective for fresh install.
NTP displays SR data always. So, caching it and use it instead of using data from component folder. If not, we have to wait to get SR data until component is ready.
So far, current code only can recover fetching mapping table failure. but browser can be crashed while waiting component fetching after mapping table is downloaded. So, we should run recover logic if browser doesn't finish initial component downloding due to crash.
Android needs super referrals code during the campaign to generate QR code. Backend will clear it when campaign ends.
When registering demo, it skips some steps that sr component registration does. It can ruins some preferences. Instead of this, we can demo SR component easily by inserting promoCode file in user data dir.
ViewCounterService will provide SR data even if NTPBGImages pref is false.
Regardless of background image option, NTP will show SR images if user choose SR theme in settings.
Use same manifest filename and determine it's SI or SR by its contents. And delete utils. APIs that in utils should be in service impl file.
caa2905
to
a3eeb48
Compare
Filed f/u issue - brave/brave-browser#9289 |
if/when uplifted, we'll need to grab this one too: |
Implement NTP Custom homepage ("super referral")
Verification passed on
Check fresh install with super referrer
Check fresh install with non-referrer Two scenarios that check this change doesn't affect existing users
Scenario 2 (User has empty code because of 90 days passed)
Verification PASSED on
Ensured that installing Brave without a super referral
Used
Moved the machines date ~90 days ahead and ensured that
|
Resolves brave/brave-browser#8218
NTPBackgroundImagesService
manages background provider(SI or SR).By default, it registers SI component and SR component if it's super referral install.
It's global service because SI or SR component is browser-wide component and it manages their data itself. Which data should be used for webui or android NTP will be determined by
ViewCounterService
.ViewCounterService
will provide proper data (SI or SR) to client (like webui) by checking current profile's preference.NTPBackgroundImagesSource
handles bg data request. It can handle SI and SR data simultaneously.SI data's url starts with
chrome://branded-wallpaper/sponsored-images/...
and SR data is something likechrome://branded-wallpaper/super-referral/logo.png
.I think we can expand this in the future. For example,
chrome://branded-wallpaper/default/wallpaper.jpg
for default background.To make SR data available quickly, SR data is cached. With this caching, client can get SR data
before component registration is ready. It's important because SR's NTP always should display its image.
In this PR,
kNewTabPageSuperReferralThemesOption
pref is added.Client should use it as a switch for turn it on/off SR component in runtime in super referral installs.
If it's turned off, SI or default bg will be displayed. This can be changed in settings page.
Regarding to flags, existing
New Tab Page Branded Wallpapers
flag will act as a top level switch.If it is off,
SI
andSR
are all disabled.If
New Tab Page Branded Wallpapers
feature switch is on,SI
component is enabled by default.New
Enable Brave Custom Homepage
is introduced forSR
component. If this is on,SR
component will be installed when current install comes from super referrer.To test locally, we can pass different local test data by using
ntp-sponsored-images-data-path
andntp-super-referral-data-path
. Also can use demo components. It becomes available when demo flag is on.Settings page with Super referral
Settings page with non Super referral
NTP Pages with Super referral
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
Simply test with demo data => SR component test is not supported by Demo flag.
1. Launch browser with fresh profile2. Turn on
New Tab Page Demo Branded Wallpaper
in brave://flags and relaunch3. Open brave://components and check SI and SR component is registered (
NTP Sponsored Images (DEMO)
andNTP Super Referral (Technikke)
)4. Open NTP and check SR is loaded. If top sites are visible, it means SR component is loaded
Note: With demo data, theme settings are not properly updated. To see the themes option change, please follow below step.
Check fresh install with super referrer
TECHNIK
stringNTP Super Referrer (Technikke)
component is registered in brave://componentsBrave default images
option.Check fresh install with non referrer
Two scenarios that check this change doesn't affect existing users
Scenario 1 (User has default code or non super referrer code)
Scenario 2 (User has empty code because of 90 days passed)
Reviewer Checklist:
After-merge Checklist:
changes has landed on.