cros: Tighten profile loading

cros: Tighten profile loading

- Make it FATAL if GetLastUsedProfile() is called before user profile
  is properly loaded;
- Fix a few places that attempt to access an user profile before it is
  loaded;

BUG=718734
TEST=ProfileManagerTest.UserProfileLoading

Review-Url: https://codereview.chromium.org/2942043002
Cr-Commit-Position: refs/heads/master@{#480575}
Committed: https://chromium.googlesource.com/chromium/src/+/8c6a41160e0750686a0fb4cf05563a8ac48ccaa9

[xiyuan, 2017-06-15 19:45:20]

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

[commit-bot: I haz the power, 2017-06-15 19:45:45]

Dry run: CQ is trying da patch.

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

[xiyuan, 2017-06-15 20:40:44]

Description was changed from

==========
cros: Tighten profile loading

- Make it FATAL if GetLastUsedProfile() is called before user profile
  is properly loaded;
- Fix a few places that attempt to access an user profile before it is
  loaded;

BUG=718734
==========

to

==========
cros: Tighten profile loading

- Make it FATAL if GetLastUsedProfile() is called before user profile
  is properly loaded;
- Fix a few places that attempt to access an user profile before it is
  loaded;

BUG=718734
TEST=ProfileManagerTest.UserProfileLoading
==========

[xiyuan, 2017-06-15 20:44:07]

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

[commit-bot: I haz the power, 2017-06-15 20:45:06]

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-06-15 21:51:06]

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

[commit-bot: I haz the power, 2017-06-15 21:51:07]

Dry run: This issue passed the CQ dry run.

[xiyuan, 2017-06-15 22:49:08]

xiyuan@chromium.org changed reviewers:
+ emaxx@chromium.org, msarda@chromium.org

[xiyuan, 2017-06-15 22:49:08]

Follow up on previous CL to make LOG(WARNING) -> LOG(FATAL) in
GetLastUserProfile() and fix call sites.

[msarda, 2017-06-16 13:50:42]

LGTM for chrome/browser/profiles/*

[emaxx, 2017-06-19 11:29:11]

https://codereview.chromium.org/2942043002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/accessibility/accessibility_manager.cc (right):

https://codereview.chromium.org/2942043002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/accessibility/accessibility_manager.cc:1275: if
(active_user && active_user->is_profile_created())
Doesn't this change mean that |AccessibilityManager| has now to observe the
profile creation additionally (e.g. as the NOTIFICATION_PROFILE_CREATED event)?

The same question is for similar cases in other files in this CL.

https://codereview.chromium.org/2942043002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/system/system_clock.cc (right):

https://codereview.chromium.org/2942043002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/system/system_clock.cc:125: void
SystemClock::ActiveUserChanged(const user_manager::User* active_user) {
nit: Please change the declaration at
chrome/browser/chromeos/system/system_clock.h accordingly.

[xiyuan, 2017-06-19 15:28:00]

https://codereview.chromium.org/2942043002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/accessibility/accessibility_manager.cc (right):

https://codereview.chromium.org/2942043002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/accessibility/accessibility_manager.cc:1275: if
(active_user && active_user->is_profile_created())
On 2017/06/19 11:29:11, emaxx wrote:
> Doesn't this change mean that |AccessibilityManager| has now to observe the
> profile creation additionally (e.g. as the NOTIFICATION_PROFILE_CREATED
event)?
> 
> The same question is for similar cases in other files in this CL.

Not necessarily. |AccessibilityManager|, |MagnificationManagerImpl| and
|SystemClock| observe NOTIFICATION_SESSION_STARTED as well, which is sent after
the user profile has been loaded. The skipped work here would be resumed by
then.

https://codereview.chromium.org/2942043002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/system/system_clock.cc (right):

https://codereview.chromium.org/2942043002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/system/system_clock.cc:125: void
SystemClock::ActiveUserChanged(const user_manager::User* active_user) {
On 2017/06/19 11:29:11, emaxx wrote:
> nit: Please change the declaration at
> chrome/browser/chromeos/system/system_clock.h accordingly.

Done.

[xiyuan, 2017-06-19 15:28:05]

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

[commit-bot: I haz the power, 2017-06-19 15:28:23]

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-06-19 16:40:51]

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

[commit-bot: I haz the power, 2017-06-19 16:40:52]

Dry run: This issue passed the CQ dry run.

[emaxx, 2017-06-19 18:19:19]

LGTM.


On 2017/06/19 15:28:00, xiyuan wrote:
> Not necessarily. |AccessibilityManager|, |MagnificationManagerImpl| and
> |SystemClock| observe NOTIFICATION_SESSION_STARTED as well, which is sent
after
> the user profile has been loaded. The skipped work here would be resumed by
> then.

Thanks for the clarification!

[xiyuan, 2017-06-19 20:59:59]

The CQ bit was checked by xiyuan@chromium.org

[xiyuan, 2017-06-19 21:00:00]

The patchset sent to the CQ was uploaded after l-g-t-m from msarda@chromium.org
Link to the patchset: https://codereview.chromium.org/2942043002/#ps40001
(title: "update SystemClock header")

[commit-bot: I haz the power, 2017-06-19 21:00:22]

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-06-19 21:03:52]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1497905999556780,
"parent_rev": "abece3848bf76cef2dd29caa25ca842a23986e50", "commit_rev":
"e1ca112777a67ee9fb28828b13ad8ca54ab94723"}

[commit-bot: I haz the power, 2017-06-19 21:04:09]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1497905999556780,
"parent_rev": "116f35aec4d67fefec509503b17e8645b41ce3f4", "commit_rev":
"8c6a41160e0750686a0fb4cf05563a8ac48ccaa9"}

[commit-bot: I haz the power, 2017-06-19 21:04:21]

Description was changed from

==========
cros: Tighten profile loading

- Make it FATAL if GetLastUsedProfile() is called before user profile
  is properly loaded;
- Fix a few places that attempt to access an user profile before it is
  loaded;

BUG=718734
TEST=ProfileManagerTest.UserProfileLoading
==========

to

==========
cros: Tighten profile loading

- Make it FATAL if GetLastUsedProfile() is called before user profile
  is properly loaded;
- Fix a few places that attempt to access an user profile before it is
  loaded;

BUG=718734
TEST=ProfileManagerTest.UserProfileLoading

Review-Url: https://codereview.chromium.org/2942043002
Cr-Commit-Position: refs/heads/master@{#480575}
Committed:
https://chromium.googlesource.com/chromium/src/+/8c6a41160e0750686a0fb4cf0556...
==========

[commit-bot: I haz the power, 2017-06-19 21:04:26]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/8c6a41160e0750686a0fb4cf0556...