New issue
Advanced search Search tips

Issue 872871 link

Starred by 1 user
Status: Fixed
Owner:
tbarzic@chromium.org
Closed: Aug 14
Cc:
r...@chromium.org
waff...@chromium.org
michae...@chromium.org
xiaochu@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Early calls to CrOSComponentManager::Load could force foreground update check

Project Member Reported by tbarzic@chromium.org, Aug 9 
If CrOSComponentManager::Load is called early on in the browser process lifetime (i.e. before all Register tasks started by InstallRegistered() are done), it might cause foreground component update check even if the target component is already registered.

The method initiates update check if there are no compatible paths registered at the time it's called. Though, compatible paths are registered asynchronously by call to InstallRegistered, and there is no guarantee that this operation has finished by the time a call to CrOSComponentManager is made.


(we plan to attempt to load demo-mode-resources component (not yet added to cros_component_installer) fairly early, so we might hit this issue; for demo-mode-resources we might want to avoid update all-together, and load a preinstalled version if no installed component is found, but even if update step is skipped in that case, Load might incorrectly report that the component was not installed).
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8504f674e97cfead82712c7b887b0d665cdd1d0a

commit 8504f674e97cfead82712c7b887b0d665cdd1d0a
Author: Toni Barzic <tbarzic@google.com>
Date: Fri Aug 10 17:37:12 2018

Skip update if a component is found during registration

Update CrOSComponentManager to handle the case when an installed
compatible component is found during component registration started by
CrOSComponentManager::Load.

Previously, CrOSComponentManager would start OnDemandUpdate whenever no
compatible component path was registered when Load() was called - this
did not account for cases where Load was called early on, before
registration of installed components finished. This means that
CrOSComponentManager could end up starting foreground on demand update
even if a compatible version of component was already installed.

This CL moves decision whether an update should be requested later on -
after the component registration was done. Update will be run if it's
forced, or if there is no compatible component version after component
registration finishes (which should check for any installed components).

Note - this also moves ComponentInstaller::Register callback after
OnComponentReady is called, to ensure that any potential compatible
component path is registered before CrOSComponentManager::StartInstall
is called. (This is more in line with order OnComponentReady and the
callback are invoked during Install request, and no other callers seem
to depend on this ordering).

BUG= 872871 

Change-Id: I33ff01b5d127d17cd9016b012fc400562c03d9c1
Reviewed-on: https://chromium-review.googlesource.com/1170172
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582230}
[modify] https://crrev.com/8504f674e97cfead82712c7b887b0d665cdd1d0a/chrome/browser/component_updater/cros_component_installer_chromeos.cc
[modify] https://crrev.com/8504f674e97cfead82712c7b887b0d665cdd1d0a/chrome/browser/component_updater/cros_component_installer_chromeos.h
[modify] https://crrev.com/8504f674e97cfead82712c7b887b0d665cdd1d0a/components/component_updater/component_installer.cc

Comment 2 by xiaochu@chromium.org, Aug 10 

(Sorry for the late response.) Thanks for fixing this!

Comment 3 by tbarzic@chromium.org, Aug 14

Status: Fixed (was: Started)
►

Sign in to add a comment