Cryptohome: Notify about error in async calls if cryptohome is not ready yet.

Cryptohome: Notify about error in async calls if cryptohome is not ready yet.

BUG=451148
Committed: https://crrev.com/7df6f9fc7fcd1c02babf89548ce967bf8605b7f3
Cr-Commit-Position: refs/heads/master@{#313687}

[Denis Kuznetsov (DE-MUC), 2015-01-28 17:52:25]

antrim@chromium.org changed reviewers:
+ derat@chromium.org

[Denis Kuznetsov (DE-MUC), 2015-01-28 17:52:25]

Daniel, please take a look.
If user is really quick he can perform some UI actions before  cryptohomed is
started in chromeos, and DBus calls would fail.
We did not propagate failure up the call stack, so UI (blocked at the start of
signin attempt) was not unblocked upon failure.
I've added explicit handling of guest signin failure handling, as current error
messages are aligned with user pod.

[Daniel Erat, 2015-01-28 18:01:45]

derat@chromium.org changed reviewers:
+ nkostylev@chromium.org

[Daniel Erat, 2015-01-28 18:01:46]

chromeos seems fine to me after some comments are addressed. adding nikita for
c/b/chromeos/login since i'm not familiar with the code that's being changed
there.

https://chromiumcodereview.appspot.com/880303003/diff/1/chromeos/cryptohome/a...
File chromeos/cryptohome/async_method_caller.cc (right):

https://chromiumcodereview.appspot.com/880303003/diff/1/chromeos/cryptohome/a...
chromeos/cryptohome/async_method_caller.cc:339: std::string()));
nit: is there a reason for not using MOUNT_ERROR_FATAL here? is there some other
reason that could be attached rather than an empty string?

https://chromiumcodereview.appspot.com/880303003/diff/1/chromeos/dbus/cryptoh...
File chromeos/dbus/cryptohome_client.h (right):

https://chromiumcodereview.appspot.com/880303003/diff/1/chromeos/dbus/cryptoh...
chromeos/dbus/cryptohome_client.h:39: extern const int
kCryptohomeNotReadyAsyncId;
can you make this be a public static member of CryptohomeClient (with a shorter
name) instead of putting it directly into the cryptohome namespace? that makes
it easier to see where it's defined when reading code that uses it.

e.g.

class ... CryptohomeClient ... {
 public:
  static const int kNotReadyAsyncId;

[Denis Kuznetsov (DE-MUC), 2015-01-28 19:45:26]

https://chromiumcodereview.appspot.com/880303003/diff/1/chromeos/cryptohome/a...
File chromeos/cryptohome/async_method_caller.cc (right):

https://chromiumcodereview.appspot.com/880303003/diff/1/chromeos/cryptohome/a...
chromeos/cryptohome/async_method_caller.cc:339: std::string()));
On 2015/01/28 18:01:46, Daniel Erat wrote:
> nit: is there a reason for not using MOUNT_ERROR_FATAL here? is there some
other
> reason that could be attached rather than an empty string?

This callback expects std::string() instead of enum, it is used to get part of
mountpoint (actually, hash of username).

https://chromiumcodereview.appspot.com/880303003/diff/1/chromeos/dbus/cryptoh...
File chromeos/dbus/cryptohome_client.h (right):

https://chromiumcodereview.appspot.com/880303003/diff/1/chromeos/dbus/cryptoh...
chromeos/dbus/cryptohome_client.h:39: extern const int
kCryptohomeNotReadyAsyncId;
On 2015/01/28 18:01:46, Daniel Erat wrote:
> can you make this be a public static member of CryptohomeClient (with a
shorter
> name) instead of putting it directly into the cryptohome namespace? that makes
> it easier to see where it's defined when reading code that uses it.
> 
> e.g.
> 
> class ... CryptohomeClient ... {
>  public:
>   static const int kNotReadyAsyncId;

Done.

[Daniel Erat, 2015-01-28 20:07:28]

thanks, lgtm for chromeos/

[Nikita (slow), 2015-01-29 08:51:48]

The CQ bit was checked by nkostylev@chromium.org

[Nikita (slow), 2015-01-29 08:51:49]

lgtm

[commit-bot: I haz the power, 2015-01-29 08:51:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/880303003/20001

[commit-bot: I haz the power, 2015-01-29 09:46:07]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-29 09:47:10]

Patchset 2 (id:??) landed as
https://crrev.com/7df6f9fc7fcd1c02babf89548ce967bf8605b7f3
Cr-Commit-Position: refs/heads/master@{#313687}