New issue
Advanced search Search tips

Issue 686901 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Add unittests for chromeos::StartupAppLauncher

Project Member Reported by tbarzic@chromium.org, Jan 30 2017

Issue description

The class behavior is covered by browser tests, but IMHO, adding unit_tests would provide more focused and thorough tests.

 

Comment 1 by st...@chromium.org, Mar 3 2017

Cc: r...@chromium.org

Comment 2 by st...@chromium.org, Mar 3 2017

Cc: -st...@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 3 2018

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

commit 1256163f43be8a359d4f5eb2d80e2ad3267ad01c
Author: Toni Barzic <tbarzic@google.com>
Date: Wed Jan 03 02:52:00 2018

Introduce test external cache

Makes chromeos::ExternalCache an interface, and introduces a test
external cache implementation. The test implementation is currently
not used, but the plan is to eventually inject it into KioskAppManager
in unittests where needed (immediate plan is to use it in unittests
for chromeos::StartupAppLauncher).

BUG=686901

Change-Id: I711221e57358c535f3bc34750a5c59521b805be6
Reviewed-on: https://chromium-review.googlesource.com/845059
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526602}
[modify] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc
[modify] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/app_mode/kiosk_app_manager.h
[modify] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc
[modify] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/app_mode/kiosk_external_updater.cc
[modify] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/extensions/device_local_account_external_policy_loader.cc
[modify] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/extensions/device_local_account_external_policy_loader.h
[modify] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/extensions/external_cache.cc
[modify] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/extensions/external_cache.h
[add] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/extensions/external_cache_delegate.h
[add] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/extensions/external_cache_impl.cc
[add] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/extensions/external_cache_impl.h
[rename] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/extensions/external_cache_impl_unittest.cc
[add] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/extensions/test_external_cache.cc
[add] https://crrev.com/1256163f43be8a359d4f5eb2d80e2ad3267ad01c/chrome/browser/chromeos/extensions/test_external_cache.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 4 2018

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

commit 536a80f82b9276dbcf22f73d281ea63a16654127
Author: Toni Barzic <tbarzic@google.com>
Date: Thu Jan 04 23:27:49 2018

Extract checking for extension updates from startup_app_launcher

This should simplify code in startup_app_launcher a little, make
extension updater check more modular, and make it easier  to later
fake extension updater behavior in tests that need it (this cl does
not add a fake update checker implementation, though).

BUG=686901

Change-Id: I771eb62952c657078d46648e800c8c05dfb28b95
Reviewed-on: https://chromium-review.googlesource.com/849424
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527143}
[modify] https://crrev.com/536a80f82b9276dbcf22f73d281ea63a16654127/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/536a80f82b9276dbcf22f73d281ea63a16654127/chrome/browser/chromeos/app_mode/startup_app_launcher.cc
[modify] https://crrev.com/536a80f82b9276dbcf22f73d281ea63a16654127/chrome/browser/chromeos/app_mode/startup_app_launcher.h
[add] https://crrev.com/536a80f82b9276dbcf22f73d281ea63a16654127/chrome/browser/chromeos/app_mode/startup_app_launcher_update_checker.cc
[add] https://crrev.com/536a80f82b9276dbcf22f73d281ea63a16654127/chrome/browser/chromeos/app_mode/startup_app_launcher_update_checker.h

Project Member

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

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

commit 8702668ef1f8c068f9b3e298467c5f39257d8ebd
Author: Toni Barzic <tbarzic@google.com>
Date: Tue Jan 09 18:48:44 2018

Refactor kiosk app external loader creation

Prior to this change KioskAppManager exposed methods for creating
kiosk app external loaders - when creating them, KioskAppManager would
remember their weak ptrs in order to later notify them when the kiosk
apps are ready to be installed.

After this cl, kiosk app external loaders will be created directly by
ExternalProviderImpl. When initialized, the external loaders will
register callback with KioskAppManager that will be run when kiosk
apps become ready to be installed. When the callback is run, the
KioskAppExternalLoader will forward the kiosk apps prefs dictionary
to it's owner (using extensions::ExternalLoader inerface).

Additionally, when kiosk apps are updated after the initial app
propoerties have been set, use ExternalLoader::OnUpdated rather than
ExternalLoader::LoadFinished - to avoid CHECK crash, add a option to
explicitly allow external extension set updates to
ExternalProviderImpl (instead of relying on CRX and update url
sources assigned to associated external loader).

BUG=686901

Change-Id: Ie9ee272ee055e39ccde747e13652fbc0fb6ccfd0
Reviewed-on: https://chromium-review.googlesource.com/845264
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528058}
[modify] https://crrev.com/8702668ef1f8c068f9b3e298467c5f39257d8ebd/chrome/browser/chromeos/app_mode/kiosk_app_external_loader.cc
[modify] https://crrev.com/8702668ef1f8c068f9b3e298467c5f39257d8ebd/chrome/browser/chromeos/app_mode/kiosk_app_external_loader.h
[modify] https://crrev.com/8702668ef1f8c068f9b3e298467c5f39257d8ebd/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc
[modify] https://crrev.com/8702668ef1f8c068f9b3e298467c5f39257d8ebd/chrome/browser/chromeos/app_mode/kiosk_app_manager.h
[modify] https://crrev.com/8702668ef1f8c068f9b3e298467c5f39257d8ebd/chrome/browser/chromeos/app_mode/startup_app_launcher.cc
[modify] https://crrev.com/8702668ef1f8c068f9b3e298467c5f39257d8ebd/chrome/browser/extensions/extension_service_unittest.cc
[modify] https://crrev.com/8702668ef1f8c068f9b3e298467c5f39257d8ebd/chrome/browser/extensions/external_provider_impl.cc
[modify] https://crrev.com/8702668ef1f8c068f9b3e298467c5f39257d8ebd/chrome/browser/extensions/external_provider_impl.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 10 2018

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

commit debecfb1c0cefeff24fa0fbdf47df2f2ab8244fa
Author: Toni Barzic <tbarzic@google.com>
Date: Wed Jan 10 21:49:11 2018

Adds some unit tests for startup_app_launcher

Along the way fixes a couple of detected issues:
 * Prevent app relaunch after the launcher reported it's ready
   (e.g. when kiosk app data is updated during offline kiosk launch)
 * Fail kiosk app launch if a primary app CRX installation fails
   (the launch used to hang waiting for CRX installation in this case)

Bug:686901

Change-Id: I889924239573869ca43de88eaf14d7a136ad8e93
Reviewed-on: https://chromium-review.googlesource.com/843754
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528436}
[modify] https://crrev.com/debecfb1c0cefeff24fa0fbdf47df2f2ab8244fa/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/debecfb1c0cefeff24fa0fbdf47df2f2ab8244fa/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc
[modify] https://crrev.com/debecfb1c0cefeff24fa0fbdf47df2f2ab8244fa/chrome/browser/chromeos/app_mode/kiosk_app_manager.h
[modify] https://crrev.com/debecfb1c0cefeff24fa0fbdf47df2f2ab8244fa/chrome/browser/chromeos/app_mode/startup_app_launcher.cc
[add] https://crrev.com/debecfb1c0cefeff24fa0fbdf47df2f2ab8244fa/chrome/browser/chromeos/app_mode/startup_app_launcher_unittest.cc
[add] https://crrev.com/debecfb1c0cefeff24fa0fbdf47df2f2ab8244fa/chrome/browser/chromeos/app_mode/test_kiosk_extension_builder.cc
[add] https://crrev.com/debecfb1c0cefeff24fa0fbdf47df2f2ab8244fa/chrome/browser/chromeos/app_mode/test_kiosk_extension_builder.h

Sign in to add a comment