New issue
Advanced search Search tips

Issue 800020 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Extension reload and extension updater (with install immediately set) unconditionally finish delayed installs

Project Member Reported by tbarzic@chromium.org, Jan 8 2018

Issue description

ExtensionService::ReloadExtensionImpl and ExtensionUpdater::OnExtensionDownloadFailed call into ExtensionService::FinishDelayedInstallation which finishes the extension's delayed installation regardless of whether the condition that delayed the installation still applies.

For example, if the extension update was delayed due to a missing shared module import, reloading the extension will end up installing the update whether the missing shared module got installed or not.

Same applies to kiosk app updates delayed due to incompatible platform version - reloading the app, or running extension update check with install immediately set would (incorrectly) install the delayed update.

 
Labels: -M-55 M-65
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 9 2018

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

commit 667db0d3c2f695c222ffd31d0fd4885f26669a02
Author: Toni Barzic <tbarzic@google.com>
Date: Tue Jan 09 21:12:35 2018

Do not finish delayed installs if they should remain being delayed

ExtensionServiceInterface exposed FinishDelayedInstallation interface,
which unconditionally finishes delayed extension installations. The
current usages do not correctly determine whether the delayed installs
are ready to be installed, e.g. they do not verify that shared module
dependencies specified by the extension are satisfied (it seems that
current calling sites assume the only reason an update can be delayed
is due to the extension being active at the time of the last
installation attempt).

This CL replaces FinishDelayedInstallation with
AttemptDelayedInstallation which runs ShouldDelayExtensionInstall
again before finishing the delayed installation - the behavior is
the same as existing MaybeFinishDelayedInstallation private method,
so the MaybeFinishDelayedInstallation calls are replace with
AttemptDelayedInstallation as well.

Note that this makes check for whether primary app has pending delayed
install before running updater during kiosk app launch unnecessary,
thus the check is removed. Previously, given that the extension update
check was run with install_immediately set, running update checks at
that point would risk installing the app's installation delayed due to
non-compliant platform (which is undesirable behavior) - this cl fixes
the issue this code was working around.

Bug:  800020 

Change-Id: Ic9bba8aa9d5c8a974fba3a9f5331fe24a21e86d1
Reviewed-on: https://chromium-review.googlesource.com/853358
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528110}
[modify] https://crrev.com/667db0d3c2f695c222ffd31d0fd4885f26669a02/chrome/browser/chromeos/app_mode/startup_app_launcher.cc
[modify] https://crrev.com/667db0d3c2f695c222ffd31d0fd4885f26669a02/chrome/browser/extensions/extension_service.cc
[modify] https://crrev.com/667db0d3c2f695c222ffd31d0fd4885f26669a02/chrome/browser/extensions/extension_service.h
[modify] https://crrev.com/667db0d3c2f695c222ffd31d0fd4885f26669a02/chrome/browser/extensions/extension_service_unittest.cc
[modify] https://crrev.com/667db0d3c2f695c222ffd31d0fd4885f26669a02/chrome/browser/extensions/test_extension_service.cc
[modify] https://crrev.com/667db0d3c2f695c222ffd31d0fd4885f26669a02/chrome/browser/extensions/test_extension_service.h
[modify] https://crrev.com/667db0d3c2f695c222ffd31d0fd4885f26669a02/chrome/browser/extensions/updater/extension_updater.cc
[add] https://crrev.com/667db0d3c2f695c222ffd31d0fd4885f26669a02/chrome/test/data/extensions/pending_updates_with_imports/updated_with_imports/1.0.0/manifest.json
[add] https://crrev.com/667db0d3c2f695c222ffd31d0fd4885f26669a02/chrome/test/data/extensions/pending_updates_with_imports/updated_with_imports/2.0.0/manifest.json
[add] https://crrev.com/667db0d3c2f695c222ffd31d0fd4885f26669a02/chrome/test/data/extensions/pending_updates_with_imports/updated_with_imports/update.pem

Status: Fixed (was: Started)

Sign in to add a comment