cros: Move wallpaper after login screen is gone

cros: Move wallpaper after login screen is gone

- Make LoginDisplayHost::Finalize to take a callback to
  be invoked just before login screen is dismissed;
- Set ACTIVE session state via the callback so that
  wallpaper is moved to unlock container after login
  screen is gone;
- Add LOGGED_IN_NOT_ACTIVE as another user session blocked
  exception so that initial browser window created in
  this state can be activated;

BUG=718011
Review-Url: https://codereview.chromium.org/2891223002
Cr-Commit-Position: refs/heads/master@{#474654}
Committed: https://chromium.googlesource.com/chromium/src/+/e6a229fa45aa9c57619d7e8f7bae4abe8351600c

[xiyuan, 2017-05-18 23:10:42]

The CQ bit was checked by xiyuan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-18 23:11:36]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-18 23:40:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-18 23:40:42]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[xiyuan, 2017-05-19 20:18:14]

The CQ bit was checked by xiyuan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-19 20:18:54]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-19 21:41:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-19 21:41:33]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[xiyuan, 2017-05-19 23:22:24]

The CQ bit was checked by xiyuan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-19 23:23:05]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-20 00:00:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-20 00:00:05]

Dry run: This issue passed the CQ dry run.

[xiyuan, 2017-05-22 19:41:26]

Patchset #3 (id:40001) has been deleted

[xiyuan, 2017-05-22 21:00:08]

The CQ bit was checked by xiyuan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-22 21:00:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-22 21:34:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-22 21:34:21]

Dry run: This issue passed the CQ dry run.

[xiyuan, 2017-05-24 19:32:52]

xiyuan@chromium.org changed reviewers:
+ jamescook@chromium.org

[James Cook, 2017-05-24 20:08:14]

LGTM

https://codereview.chromium.org/2891223002/diff/80001/ash/session/session_con...
File ash/session/session_controller.cc (right):

https://codereview.chromium.org/2891223002/diff/80001/ash/session/session_con...
ash/session/session_controller.cc:89: //   activated before session state
changes to ACTIVE.
Aside: Didn't we have some trouble about 6 months ago with browser windows being
activated in tests? I seem to remember adding and removing a FlushForTesting
call in the session restore code. I wonder if that can be cleaned up (not for
this CL).

https://codereview.chromium.org/2891223002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/ui/login_display_host.h (right):

https://codereview.chromium.org/2891223002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/login/ui/login_display_host.h:56: virtual void
Finalize(base::OnceClosure callback) = 0;
nit: |done_callback| or |completion_callback|

[xiyuan, 2017-05-24 20:37:22]

https://codereview.chromium.org/2891223002/diff/80001/ash/session/session_con...
File ash/session/session_controller.cc (right):

https://codereview.chromium.org/2891223002/diff/80001/ash/session/session_con...
ash/session/session_controller.cc:89: //   activated before session state
changes to ACTIVE.
On 2017/05/24 20:08:13, James Cook wrote:
> Aside: Didn't we have some trouble about 6 months ago with browser windows
being
> activated in tests? I seem to remember adding and removing a FlushForTesting
> call in the session restore code. I wonder if that can be cleaned up (not for
> this CL).

I believe this is the relevant discussion in the CL:

https://codereview.chromium.org/2734933004/diff/100001/chrome/browser/chromeo...

After discussion, FlushForTesting call is not added in the session restore code.
Instead SessionController is changed to have a different default session state
based on whether we have --login-user switch to resolve the issue.

That code could only be removed if we want to make UNKNOWN session state another
exception here. I am not sure whether we want to do that.

https://codereview.chromium.org/2891223002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/ui/login_display_host.h (right):

https://codereview.chromium.org/2891223002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/login/ui/login_display_host.h:56: virtual void
Finalize(base::OnceClosure callback) = 0;
On 2017/05/24 20:08:13, James Cook wrote:
> nit: |done_callback| or |completion_callback|

Done.

[xiyuan, 2017-05-24 20:37:28]

The CQ bit was checked by xiyuan@chromium.org to run a CQ dry run

[xiyuan, 2017-05-24 20:38:20]

xiyuan@chromium.org changed reviewers:
+ drcrash@chromium.org

[xiyuan, 2017-05-24 20:38:21]

drcrash@, could I get an owner stamp from you for

  enterprise_platform_keys_apitest_nss.cc

?

Thanks.

[commit-bot: I haz the power, 2017-05-24 20:38:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2017-05-24 20:52:36]

https://codereview.chromium.org/2891223002/diff/80001/ash/session/session_con...
File ash/session/session_controller.cc (right):

https://codereview.chromium.org/2891223002/diff/80001/ash/session/session_con...
ash/session/session_controller.cc:89: //   activated before session state
changes to ACTIVE.
On 2017/05/24 20:37:22, xiyuan wrote:
> On 2017/05/24 20:08:13, James Cook wrote:
> > Aside: Didn't we have some trouble about 6 months ago with browser windows
> being
> > activated in tests? I seem to remember adding and removing a FlushForTesting
> > call in the session restore code. I wonder if that can be cleaned up (not
for
> > this CL).
> 
> I believe this is the relevant discussion in the CL:
> 
>
https://codereview.chromium.org/2734933004/diff/100001/chrome/browser/chromeo...
> 
> After discussion, FlushForTesting call is not added in the session restore
code.
> Instead SessionController is changed to have a different default session state
> based on whether we have --login-user switch to resolve the issue.
> 
> That code could only be removed if we want to make UNKNOWN session state
another
> exception here. I am not sure whether we want to do that.

Ah, OK. Nothing to do, then.

[commit-bot: I haz the power, 2017-05-24 21:13:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-24 21:13:22]

Dry run: This issue passed the CQ dry run.

[The one and only Dr. Crash, 2017-05-24 23:11:57]

LGTM with nits on a comment on the first file (then I skipped straight to the
_nss.cc test).

[The one and only Dr. Crash, 2017-05-24 23:12:09]

https://codereview.chromium.org/2891223002/diff/100001/ash/session/session_co...
File ash/session/session_controller.cc (right):

https://codereview.chromium.org/2891223002/diff/100001/ash/session/session_co...
ash/session/session_controller.cc:88: //   created by session restore (or a
default new browser window) is properly
Plural ("are") since you're talking about multiple windows.

https://codereview.chromium.org/2891223002/diff/100001/ash/session/session_co...
ash/session/session_controller.cc:91: //   the unlocking animation hides lock
container at the end. During the
the lock

https://codereview.chromium.org/2891223002/diff/100001/ash/session/session_co...
ash/session/session_controller.cc:93: //   user windows are deemed activatable
and ash correctly restore the active
restores

[xiyuan, 2017-05-25 14:31:48]

https://codereview.chromium.org/2891223002/diff/100001/ash/session/session_co...
File ash/session/session_controller.cc (right):

https://codereview.chromium.org/2891223002/diff/100001/ash/session/session_co...
ash/session/session_controller.cc:88: //   created by session restore (or a
default new browser window) is properly
On 2017/05/24 23:12:09, The one and only Dr. Crash wrote:
> Plural ("are") since you're talking about multiple windows.

Done.

https://codereview.chromium.org/2891223002/diff/100001/ash/session/session_co...
ash/session/session_controller.cc:91: //   the unlocking animation hides lock
container at the end. During the
On 2017/05/24 23:12:08, The one and only Dr. Crash wrote:
> the lock

Done.

https://codereview.chromium.org/2891223002/diff/100001/ash/session/session_co...
ash/session/session_controller.cc:93: //   user windows are deemed activatable
and ash correctly restore the active
On 2017/05/24 23:12:08, The one and only Dr. Crash wrote:
> restores

Done.

[xiyuan, 2017-05-25 14:31:56]

The CQ bit was checked by xiyuan@chromium.org

[xiyuan, 2017-05-25 14:31:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org, drcrash@chromium.org
Link to the patchset: https://codereview.chromium.org/2891223002/#ps120001
(title: "fix nits")

[commit-bot: I haz the power, 2017-05-25 14:32:15]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-25 15:05:04]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1495722716805840,
"parent_rev": "a94c5212e856eec73332d8c761dfa905199bd064", "commit_rev":
"e6a229fa45aa9c57619d7e8f7bae4abe8351600c"}

[commit-bot: I haz the power, 2017-05-25 15:05:23]

Description was changed from

==========
cros: Move wallpaper after login screen is gone

- Make LoginDisplayHost::Finalize to take a callback to
  be invoked just before login screen is dismissed;
- Set ACTIVE session state via the callback so that
  wallpaper is moved to unlock container after login
  screen is gone;
- Add LOGGED_IN_NOT_ACTIVE as another user session blocked
  exception so that initial browser window created in
  this state can be activated;

BUG=718011
==========

to

==========
cros: Move wallpaper after login screen is gone

- Make LoginDisplayHost::Finalize to take a callback to
  be invoked just before login screen is dismissed;
- Set ACTIVE session state via the callback so that
  wallpaper is moved to unlock container after login
  screen is gone;
- Add LOGGED_IN_NOT_ACTIVE as another user session blocked
  exception so that initial browser window created in
  this state can be activated;

BUG=718011

Review-Url: https://codereview.chromium.org/2891223002
Cr-Commit-Position: refs/heads/master@{#474654}
Committed:
https://chromium.googlesource.com/chromium/src/+/e6a229fa45aa9c57619d7e8f7bae...
==========

[commit-bot: I haz the power, 2017-05-25 15:05:24]

Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/e6a229fa45aa9c57619d7e8f7bae...