Convert chromeos::CryptohomeClient::TpmCanAttemptOwnership to asynchronous

Convert chromeos::CryptohomeClient::TpmCanAttemptOwnership to asynchronous

BUG=126674
TEST=git try -b linux_chromeos


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

[stevenjb, 2012-07-26 17:19:52]

http://codereview.chromium.org/10830033/diff/1/chrome/browser/chromeos/cros/c...
File chrome/browser/chromeos/cros/cryptohome_library.cc (right):

http://codereview.chromium.org/10830033/diff/1/chrome/browser/chromeos/cros/c...
chrome/browser/chromeos/cros/cryptohome_library.cc:23: void
DoNothing(DBusMethodCallStatus call_status) {}
Should we consider adding this where DBusMethodCallStatus is defined? Haven't we
defined this same function elsewhere?

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

http://codereview.chromium.org/10830033/diff/1/chromeos/dbus/cryptohome_clien...
chromeos/dbus/cryptohome_client.h:35: DBusMethodCallStatus call_status)>
VoidMethodCallback;
This and the two callbacks below seem generic enough to be defined with
DBusMethodCallStatus?

http://codereview.chromium.org/10830033/diff/1/chromeos/dbus/cryptohome_clien...
chromeos/dbus/cryptohome_client.h:129: virtual void TpmCanAttemptOwnership(const
VoidMethodCallback& callback) = 0;
I'm confused about what this method does. It looks like it should return whether
or not we can attempt ownership, but in that case shouldn't the callback pass a
bool? Or does it tell cryptohome that it is OK to attempt ownership, in which
case we should rename this to something like SetTpmCanAttemptOwnership.

[hashimoto, 2012-07-27 04:37:18]

http://codereview.chromium.org/10830033/diff/1/chrome/browser/chromeos/cros/c...
File chrome/browser/chromeos/cros/cryptohome_library.cc (right):

http://codereview.chromium.org/10830033/diff/1/chrome/browser/chromeos/cros/c...
chrome/browser/chromeos/cros/cryptohome_library.cc:23: void
DoNothing(DBusMethodCallStatus call_status) {}
On 2012/07/26 17:19:52, stevenjb (chromium) wrote:
> Should we consider adding this where DBusMethodCallStatus is defined? Haven't
we
> defined this same function elsewhere?
We're having the exactly same function in cros_network_functions.cc.
Will do in a separate patch.

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

http://codereview.chromium.org/10830033/diff/1/chromeos/dbus/cryptohome_clien...
chromeos/dbus/cryptohome_client.h:35: DBusMethodCallStatus call_status)>
VoidMethodCallback;
On 2012/07/26 17:19:52, stevenjb (chromium) wrote:
> This and the two callbacks below seem generic enough to be defined with
> DBusMethodCallStatus?

Sounds good, will do in a separate patch.

http://codereview.chromium.org/10830033/diff/1/chromeos/dbus/cryptohome_clien...
chromeos/dbus/cryptohome_client.h:129: virtual void TpmCanAttemptOwnership(const
VoidMethodCallback& callback) = 0;
On 2012/07/26 17:19:52, stevenjb (chromium) wrote:
> I'm confused about what this method does. It looks like it should return
whether
> or not we can attempt ownership, but in that case shouldn't the callback pass
a
> bool? Or does it tell cryptohome that it is OK to attempt ownership, in which
> case we should rename this to something like SetTpmCanAttemptOwnership.
Looks like your latter guess is correct.
(http://git.chromium.org/gitweb/?p=chromiumos/platform/cryptohome.git;a=blob;f...)
This method is named badly from the beginning, I think it should be renamed to
something like MyabeStartInitializeTpm().
Anyway, I'm using the name "TpmCanAttemptOwnership" because the Cryptohome
service is providing the method with this name.
If we are going to rename this method, we should also rename Cryptohome
service's method name too.

[stevenjb, 2012-07-30 22:24:38]

LGTM

http://codereview.chromium.org/10830033/diff/1/chrome/browser/chromeos/cros/c...
File chrome/browser/chromeos/cros/cryptohome_library.cc (right):

http://codereview.chromium.org/10830033/diff/1/chrome/browser/chromeos/cros/c...
chrome/browser/chromeos/cros/cryptohome_library.cc:23: void
DoNothing(DBusMethodCallStatus call_status) {}
Sounds good.

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

http://codereview.chromium.org/10830033/diff/1/chromeos/dbus/cryptohome_clien...
chromeos/dbus/cryptohome_client.h:129: virtual void TpmCanAttemptOwnership(const
VoidMethodCallback& callback) = 0;
OK, I see. That is definitely badly named. Can you at least comment what it does
and/or include the link?

[hashimoto, 2012-07-31 02:27:34]

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

http://codereview.chromium.org/10830033/diff/1/chromeos/dbus/cryptohome_clien...
chromeos/dbus/cryptohome_client.h:129: virtual void TpmCanAttemptOwnership(const
VoidMethodCallback& callback) = 0;
On 2012/07/30 22:24:38, stevenjb (chromium) wrote:
> OK, I see. That is definitely badly named. Can you at least comment what it
does
> and/or include the link?

Added a comment.

[commit-bot: I haz the power, 2012-07-31 02:29:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hashimoto@chromium.org/10830033/9001

[commit-bot: I haz the power, 2012-07-31 04:13:39]

Change committed as 149136