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

Follow up #5142 SpeedReader code review #9652

Closed
4 tasks
AndriusA opened this issue May 6, 2020 · 1 comment
Closed
4 tasks

Follow up #5142 SpeedReader code review #9652

AndriusA opened this issue May 6, 2020 · 1 comment

Comments

@AndriusA
Copy link

AndriusA commented May 6, 2020

Description

Follow up for brave/brave-core#5142

Documents comments left to address.

  • We're using the component installer (SpeedreaderWhitelist) where we should be using the SpeedreaderService. speedreader::Speedreader should be a base::Singleton and can be accessed inside the SpeedreaderService using Speedreader::GetInstance
  • SpeedreaderWhitelist should be SpeedreaderWhitelistInstaller`
  • kSpeedreaderEnabled doesn't appear to actually disable speedreader, it just removes the button (wasting cpu cycles and memory when it's off). It sort of turns it off with SpeedreaderTabHelper::UpdateActiveState, but that should use a pref listener like the button
  • explore ways to use chromium's css/html parsers instead of relying on third-party library that may be treating things differently
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants