Determine whether user is ephemeral when coming back from a crash

When Chrome is restarted after a crash, it needs to determine whether the user is logged in as ephemeral. This can be done by checking whether the user is found on the persistent list in local store.

BUG=127230
TEST=Log in as ephemeral, turn off ephemeral users, induce a crash:
TEST=Chrome now correctly determines that the user is still ephemeral

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=137111

[bartfab (slow), 2012-05-09 17:31:30]

Hi Satoru, Nikita,

Could you please review the parts of this CL that affect code you own? Thanks.

[satorux1, 2012-05-10 07:18:20]

hashimoto@ knows more about the cryptohome d-bus stuff than me.

[hashimoto, 2012-05-10 07:41:55]

http://codereview.chromium.org/10382086/diff/1/chromeos/dbus/cryptohome_client.h
File chromeos/dbus/cryptohome_client.h (right):

http://codereview.chromium.org/10382086/diff/1/chromeos/dbus/cryptohome_clien...
chromeos/dbus/cryptohome_client.h:64: // Calls IsMountedForUser and returns true
when the call succeeds.
Please make this method asynchronous, it should be something like:
typedef base::Callback<void(DBusMethodCallStatus call_status, bool is_mounted,
bool is_ephemeral> IsMountedForUserCallback;
virtual void IsMountedForUser(const std::string& username, const
IsMountedForUserCallback& callback) = 0;

I know synchronous API is easier to use, but we are making all D-Bus method
calls asynchronous to reduce UI jank. (crbug.com/127186)

[Nikita (slow), 2012-05-10 09:10:12]

http://codereview.chromium.org/10382086/diff/1/chrome/browser/chromeos/login/...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

http://codereview.chromium.org/10382086/diff/1/chrome/browser/chromeos/login/...
chrome/browser/chromeos/login/user_manager_impl.cc:896: if
(DBusThreadManager::Get()->GetCryptohomeClient()->
That doesn't seem like a correct solution i.e. always call a blocking call here.

You should call that method async early on and then use cached value.

[Nikita (slow), 2012-05-10 09:19:56]

http://codereview.chromium.org/10382086/diff/1/chrome/browser/chromeos/login/...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

http://codereview.chromium.org/10382086/diff/1/chrome/browser/chromeos/login/...
chrome/browser/chromeos/login/user_manager_impl.cc:896: if
(DBusThreadManager::Get()->GetCryptohomeClient()->
On 2012/05/10 09:10:12, Nikita Kostylev wrote:
> That doesn't seem like a correct solution i.e. always call a blocking call
here.
> 
> You should call that method async early on and then use cached value.

When Chrome is restarted after crash, UserManager::UserLoggedIn() is called,
which calls IsEphemeralUser(). First ever call should initialize
is_current_user_ephemeral_ in async way so subsequent calls use that cached
value.

We do distinguish case when UserLoggedIn is called after crash so you should
pass some bool variable to UserLoggedIn in that case so that async Cryptohome
call is done. That async call should not happen during normal sign in otherwise
it would delay that.

Here's the code path for reload after crash:
http://code.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/chromeos/chr...

[bartfab (slow), 2012-05-10 09:21:44]

http://codereview.chromium.org/10382086/diff/1/chrome/browser/chromeos/login/...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

http://codereview.chromium.org/10382086/diff/1/chrome/browser/chromeos/login/...
chrome/browser/chromeos/login/user_manager_impl.cc:896: if
(DBusThreadManager::Get()->GetCryptohomeClient()->
Thanks. I was looking through the code just now to find out how to better handle
the restart-after-crash case. I will dive right into it now that I know where to
look.

[bartfab (slow), 2012-05-15 09:34:44]

Hi Nikita, could you please review? I found a way to reduce this CL to an almost
trivial code change.

[Nikita (slow), 2012-05-15 10:20:37]

lgtm

[commit-bot: I haz the power, 2012-05-15 10:33:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bartfab@chromium.org/10382086/2002

[commit-bot: I haz the power, 2012-05-15 11:39:57]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is android, revision is 137103, job name
was 10382086-2002 (previous was lost) (previous was lost) (previous was lost)
(previous was lost).

[commit-bot: I haz the power, 2012-05-15 11:40:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bartfab@chromium.org/10382086/2002

[commit-bot: I haz the power, 2012-05-15 13:26:02]

Change committed as 137111