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

[ads] Optimize ads tab helper innerText/HTML performance #34919

Open
tmancey opened this issue Dec 15, 2023 · 9 comments
Open

[ads] Optimize ads tab helper innerText/HTML performance #34919

tmancey opened this issue Dec 15, 2023 · 9 comments

Comments

@tmancey
Copy link
Contributor

tmancey commented Dec 15, 2023

No description provided.

@atuchin-m
Copy link
Contributor

atuchin-m commented Dec 15, 2023

From DMs:

AdsTabHelper seems to:

It also:

As for Chromium it looks they prefer to:

Also we can take a look at Chromium/Brave page distillers (used in Speedreader) as an alternative.

@tmancey
Copy link
Contributor Author

tmancey commented Dec 15, 2023

regarding do it always even ads/rewards are disabled;, we need HTML for both users who have/have not joined Rewards. innerText is not required if a user has not joined Brave News ads or Brave Rewards.

@aseren
Copy link

aseren commented Dec 15, 2023

Regarding do capturing multiply times for one page: after DidFinishNavigation + after DocumentOnLoadCompletedInPrimaryMainFrame (onload). concern.
We are doing JS requests in DidFinishNavigation only in case of same-page navigation. So we will run our JS requests twice if the web page loads and then does the same navigation for some reason. This was done to support Single-Page-Applications, in which the web page content can be changed with the same-page navigation.
We need to find better ways of handling this page content change for Single-Page-Applications rather than listening for each same-page navigations.

@tmancey tmancey self-assigned this Jan 9, 2024
@tmancey tmancey added the closed/duplicate Issue has already been reported label Jan 9, 2024
@tmancey
Copy link
Contributor Author

tmancey commented Jan 9, 2024

Closing issue as a duplicate of #35205

@bridiver
Copy link
Contributor

bridiver commented Jun 9, 2024

I am reopening this issue because it's not clear what #35205 is supposed to be since it does not contain any details at all and the PR that closed it does not address all of the issues here. In particular we're still calling XMLSerializer on every page load with or without rewards and that needs to be changed.

@bridiver bridiver reopened this Jun 9, 2024
@bridiver
Copy link
Contributor

bridiver commented Jun 9, 2024

also do we need a similar ticket on ios? Are we doing something similar there @brave/ios and if so, can we consolidate this logic for what should get injected and under what conditions?

@tmancey
Copy link
Contributor Author

tmancey commented Jun 10, 2024

@bridiver see description for links to issues for each of the bullets.

@atuchin-m
Copy link
Contributor

any progress here?

@tmancey
Copy link
Contributor Author

tmancey commented Sep 3, 2024

@atuchin-m These calls were refactored to occur only for Rewards users. We've had a few other priorities, but we plan to return to them soon. They're part of this quarter's goals.

@tmancey tmancey removed the closed/duplicate Issue has already been reported label Sep 3, 2024
@tmancey tmancey removed their assignment Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Dev Concern ❤️
Development

No branches or pull requests

4 participants