New issue
Advanced search Search tips

Issue 765029 link

Starred by 2 users
Status: Fixed
Owner:
tbarzic@chromium.org
Closed: Oct 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

[lock screen apps]: Make lock screen profile creation lazy

Project Member Reported by tbarzic@chromium.org, Sep 14 2017 
Refactor lock screen apps profile creation out of lock_screen_apps::StateController, and improve lock screen apps profile creation logic.
In particular, instead of setting up lock screen profile after primary user's profile initialization, create lock screen profile only when it's really needed - i.e. when the user has installed and enabled a note taking app available on the lock screen.
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 5 2017 

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

commit a20785d5c280afeaa21aead507fa6c16f905eba8
Author: Toni Barzic <tbarzic@google.com>
Date: Thu Oct 05 23:19:59 2017

Do not start lock screen apps state controller if stylus is missing

This delayes final step in lock screen apps initialization (observing
session state, thus preventing the state controller from reacting on
session being locked) until stylus input is detected (for one settings
UI for controlling lock screen apps is only shown for stylus enabled
devices).
Remove somewhat hacky solution where lock screen apps were disabled by
simply not reporting any app lock screen enabled.

BUG= 765029 

Change-Id: I214dbe09e95babdd4779268fa6abfa5c1615e80b
Reviewed-on: https://chromium-review.googlesource.com/693961
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506904}
[modify] https://crrev.com/a20785d5c280afeaa21aead507fa6c16f905eba8/ash/system/palette/palette_tray.cc
[modify] https://crrev.com/a20785d5c280afeaa21aead507fa6c16f905eba8/chrome/browser/chromeos/lock_screen_apps/state_controller.cc
[modify] https://crrev.com/a20785d5c280afeaa21aead507fa6c16f905eba8/chrome/browser/chromeos/lock_screen_apps/state_controller.h
[modify] https://crrev.com/a20785d5c280afeaa21aead507fa6c16f905eba8/chrome/browser/chromeos/lock_screen_apps/state_controller_unittest.cc
[modify] https://crrev.com/a20785d5c280afeaa21aead507fa6c16f905eba8/chrome/browser/chromeos/note_taking_helper.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 6 2017 

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

commit 2bf8265ca4df937a47e4188c4aba90a30df1417b
Author: Toni Barzic <tbarzic@google.com>
Date: Fri Oct 06 04:29:43 2017

Create lock screen profile lazily

Extract lock screen profile creation out of
lock_screen_apps::StateController into a separate,
LockScreenProfileCreator class.

Primary motivation is to delay lock screen profile creation until
lock screen note taking is enabled (i.e. to avoid profile creation
if the user does not have lock screen note taking apps).

AppManager requires lock screen apps profile (as it installs the
note taking app to the profile). Instead of providing the profile
to the AppManager directly, the interface is changed to accept
LockScreenProfileCreator, which can be used to get the lock screen
profile.

LockScreenProfileCreator will observe lock screen note taking
availability, and if it detects lock screen enabled note taking app,
it will create the lock screen apps profile.
If the profile is not created by the time AppManager needs it (i.e.
the screen is locked and the user has a lock screen note taking app
set), the AppManager will be able to use AddProfileCreatedCallback
API added to LockScreenProfileCreator to observe profile state.
When the profile is created, and initialized, the AppManager will
be able to proceed to set up the note taking app.

CL also adds FakeLockScreenProfileCreator that can be used in
unittests to create lock screen (testing) profile and simulate lock
screen profile creator state changes, and adds unittests covering
lock screen apps profile creation.

BUG= 765029 

Change-Id: I33fdf3a91ea4817a3576b66c950570fc76b3c9c9
Reviewed-on: https://chromium-review.googlesource.com/662403
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506966}
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/app_manager.h
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/app_manager_impl.cc
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/app_manager_impl.h
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/app_manager_impl_unittest.cc
[add] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/fake_lock_screen_profile_creator.cc
[add] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/fake_lock_screen_profile_creator.h
[add] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator.cc
[add] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator.h
[add] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator_impl.cc
[add] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator_impl.h
[add] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator_impl_unittest.cc
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/note_taking_browsertest.cc
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/state_controller.cc
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/state_controller.h
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/chromeos/lock_screen_apps/state_controller_unittest.cc
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/2bf8265ca4df937a47e4188c4aba90a30df1417b/tools/metrics/histograms/histograms.xml

Comment 3 by tbarzic@chromium.org, Oct 6 2017

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 6 2017 

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

commit aa5306329a841818b63279811e798eeb7423bff3
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Oct 06 17:49:24 2017

Revert "Create lock screen profile lazily"

This reverts commit 2bf8265ca4df937a47e4188c4aba90a30df1417b.

Reason for revert: Speculative revert. Sorry for the noise if this doesn't do the trick. Flaky failures in mus_browser_tests started here: https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%281%29/builds/45441

Original change's description:
> Create lock screen profile lazily
> 
> Extract lock screen profile creation out of
> lock_screen_apps::StateController into a separate,
> LockScreenProfileCreator class.
> 
> Primary motivation is to delay lock screen profile creation until
> lock screen note taking is enabled (i.e. to avoid profile creation
> if the user does not have lock screen note taking apps).
> 
> AppManager requires lock screen apps profile (as it installs the
> note taking app to the profile). Instead of providing the profile
> to the AppManager directly, the interface is changed to accept
> LockScreenProfileCreator, which can be used to get the lock screen
> profile.
> 
> LockScreenProfileCreator will observe lock screen note taking
> availability, and if it detects lock screen enabled note taking app,
> it will create the lock screen apps profile.
> If the profile is not created by the time AppManager needs it (i.e.
> the screen is locked and the user has a lock screen note taking app
> set), the AppManager will be able to use AddProfileCreatedCallback
> API added to LockScreenProfileCreator to observe profile state.
> When the profile is created, and initialized, the AppManager will
> be able to proceed to set up the note taking app.
> 
> CL also adds FakeLockScreenProfileCreator that can be used in
> unittests to create lock screen (testing) profile and simulate lock
> screen profile creator state changes, and adds unittests covering
> lock screen apps profile creation.
> 
> BUG= 765029 
> 
> Change-Id: I33fdf3a91ea4817a3576b66c950570fc76b3c9c9
> Reviewed-on: https://chromium-review.googlesource.com/662403
> Commit-Queue: Toni Barzic <tbarzic@chromium.org>
> Reviewed-by: Ilya Sherman <isherman@chromium.org>
> Reviewed-by: Sky Malice <skym@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#506966}

TBR=xiyuan@chromium.org,tbarzic@chromium.org,isherman@chromium.org,skym@chromium.org

Change-Id: I301e9b44d439424837237f1bfc65b54887ade91c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  765029 
Reviewed-on: https://chromium-review.googlesource.com/705398
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507116}
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/chrome/browser/chromeos/lock_screen_apps/app_manager.h
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/chrome/browser/chromeos/lock_screen_apps/app_manager_impl.cc
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/chrome/browser/chromeos/lock_screen_apps/app_manager_impl.h
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/chrome/browser/chromeos/lock_screen_apps/app_manager_impl_unittest.cc
[delete] https://crrev.com/b2294af33d807a684cc1ba48d7fafa4f431fca71/chrome/browser/chromeos/lock_screen_apps/fake_lock_screen_profile_creator.cc
[delete] https://crrev.com/b2294af33d807a684cc1ba48d7fafa4f431fca71/chrome/browser/chromeos/lock_screen_apps/fake_lock_screen_profile_creator.h
[delete] https://crrev.com/b2294af33d807a684cc1ba48d7fafa4f431fca71/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator.cc
[delete] https://crrev.com/b2294af33d807a684cc1ba48d7fafa4f431fca71/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator.h
[delete] https://crrev.com/b2294af33d807a684cc1ba48d7fafa4f431fca71/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator_impl.cc
[delete] https://crrev.com/b2294af33d807a684cc1ba48d7fafa4f431fca71/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator_impl.h
[delete] https://crrev.com/b2294af33d807a684cc1ba48d7fafa4f431fca71/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator_impl_unittest.cc
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/chrome/browser/chromeos/lock_screen_apps/note_taking_browsertest.cc
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/chrome/browser/chromeos/lock_screen_apps/state_controller.cc
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/chrome/browser/chromeos/lock_screen_apps/state_controller.h
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/chrome/browser/chromeos/lock_screen_apps/state_controller_unittest.cc
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/aa5306329a841818b63279811e798eeb7423bff3/tools/metrics/histograms/histograms.xml
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 26 2017 

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

commit 941c360e19357db2acef99203cca4db90ebb9d2d
Author: Toni Barzic <tbarzic@google.com>
Date: Thu Oct 26 01:49:59 2017

Reland: Create lock screen profile lazily

Extract lock screen profile creation out of
lock_screen_apps::StateController into a separate,
LockScreenProfileCreator class.

Primary motivation is to delay lock screen profile creation until
lock screen note taking is enabled (i.e. to avoid profile creation
if the user does not have lock screen note taking apps).

AppManager requires lock screen apps profile (as it installs the
note taking app to the profile). Instead of providing the profile
to the AppManager directly, the interface is changed to accept
LockScreenProfileCreator, which can be used to get the lock screen
profile.

LockScreenProfileCreator will observe lock screen note taking
availability, and if it detects lock screen enabled note taking app,
it will create the lock screen apps profile.
If the profile is not created by the time AppManager needs it (i.e.
the screen is locked and the user has a lock screen note taking app
set), the AppManager will be able to use AddProfileCreatedCallback
API added to LockScreenProfileCreator to observe profile state.
When the profile is created, and initialized, the AppManager will
be able to proceed to set up the note taking app.

CL also adds FakeLockScreenProfileCreator that can be used in
unittests to create lock screen (testing) profile and simulate lock
screen profile creator state changes, and adds unittests covering
lock screen apps profile creation.

TBR=isherman@chromium.org,skym@chromium.org
(inheriting lgtm from https://chromium-review.googlesource.com/662403)

BUG= 765029 

Change-Id: I148e0aac8e228d991c01b50792dc8ca01e872f7e
Reviewed-on: https://chromium-review.googlesource.com/732724
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511689}
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/app_manager.h
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/app_manager_impl.cc
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/app_manager_impl.h
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/app_manager_impl_unittest.cc
[add] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/fake_lock_screen_profile_creator.cc
[add] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/fake_lock_screen_profile_creator.h
[add] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator.cc
[add] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator.h
[add] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator_impl.cc
[add] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator_impl.h
[add] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator_impl_unittest.cc
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/note_taking_browsertest.cc
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/state_controller.cc
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/state_controller.h
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/chromeos/lock_screen_apps/state_controller_unittest.cc
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/941c360e19357db2acef99203cca4db90ebb9d2d/tools/metrics/histograms/histograms.xml

Comment 6 by tbarzic@chromium.org, Oct 27 2017

Labels: -M-63 M-64
the fix for this actually (re)landed in M-64
►

Sign in to add a comment