brave-core icon indicating copy to clipboard operation
brave-core copied to clipboard

Implement OnDemandInstall and use it instead of OnDemandUpdate at startup

Open goodov opened this issue 9 months ago • 1 comments

This PR replaces forced component update calls on each startup with forced install calls (except for AdBlock components).

This is to lower the amount of requests on each browser start and let the standard components update mechanism do its job one minute after a startup.

Resolves https://github.com/brave/brave-browser/issues/38119

Submitter Checklist:

  • [x] I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • [x] There is a ticket for my issue
  • [x] 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 wiki
    • npm run presubmit wiki, 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:

  • [ ] The associated issue milestone is set to the smallest version that the changes has landed on
  • [ ] All relevant documentation has been updated, for instance:
    • [ ] https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove)
    • [ ] https://github.com/brave/brave-browser/wiki/Proxy-redirected-URLs
    • [ ] https://github.com/brave/brave-browser/wiki/Fingerprinting-Protections
    • [ ] https://github.com/brave/brave-browser/wiki/Brave%E2%80%99s-Use-of-Referral-Codes
    • [ ] https://github.com/brave/brave-browser/wiki/Web-Compatibility-Exceptions-in-Brave
    • [ ] https://github.com/brave/brave-browser/wiki/QA-Guide
    • [ ] https://github.com/brave/brave-browser/wiki/P3A

Test Plan:

goodov avatar May 07 '24 11:05 goodov

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

brave-builds avatar Jul 12 '24 08:07 brave-builds

@brave/crypto-wallets-core @brave/chromium-src-reviewers ptal.

goodov avatar Jul 12 '24 11:07 goodov

@brave/crypto-wallets-core ptal

goodov avatar Jul 24 '24 04:07 goodov

[puLL-Merge] - brave/brave-core@23453

Description

This PR modifies the component updater system in Brave, changing the OnDemandUpdate method to EnsureInstalled. The main motivation appears to be to ensure that components are installed when needed, rather than unnecessarily updating them if they're already present.

Changes

Changes

  1. browser/extensions/brave_component_loader.cc:

    • Changed OnDemandUpdate to EnsureInstalled
  2. browser/net/brave_ad_block_tp_network_delegate_helper_unittest.cc:

    • Updated test mock to use EnsureInstalled instead of OnDemandUpdate
  3. browser/widevine/widevine_utils.cc:

    • Updated to use EnsureInstalled instead of OnDemandUpdate
  4. chromium_src/chrome/browser/component_updater/component_updater_utils.cc and component_updater_utils.h:

    • Removed these files, likely because the functionality is now handled differently
  5. chromium_src/chrome/browser/component_updater/crl_set_component_installer.cc:

    • Updated to use EnsureInstalled instead of OnDemandUpdate
  6. chromium_src/components/component_updater/component_updater_service.cc:

    • Added new EnsureInstalled method to CrxUpdateService
    • This method checks if the component is already installed before updating
  7. components/brave_component_updater/browser/brave_component.cc:

    • Updated to use EnsureInstalled instead of OnDemandUpdate
  8. components/brave_component_updater/browser/brave_on_demand_updater.cc:

    • Added new EnsureInstalled method
    • Modified OnDemandUpdate to use EnsureInstalled
  9. Various other files:

    • Updated to use EnsureInstalled instead of OnDemandUpdate
  10. iOS-specific changes:

    • Removed iOS-specific component updater utils, likely because they're no longer needed with the new approach

Possible Issues

  1. The removal of iOS-specific component updater utils might affect iOS functionality if not properly accounted for elsewhere.
  2. Changing from OnDemandUpdate to EnsureInstalled might lead to components not being updated as frequently, which could potentially leave older versions installed for longer periods.

Security Hotspots

None identified. The changes appear to be refactoring existing functionality rather than introducing new security-sensitive code.

github-actions[bot] avatar Jul 25 '24 09:07 github-actions[bot]