Skip to content

Commit

Permalink
Revert "Migrate Bool(ean)?DBusMethodCallback to DBusMethodCallback<bo…
Browse files Browse the repository at this point in the history
…ol>."

This reverts commit bbebe80.

Reason for revert: Suspecting in breaking the compilation for ChromeOS build

Original change's description:
> Migrate Bool(ean)?DBusMethodCallback to DBusMethodCallback<bool>.
> 
> Part of OnceCallback migration in chromeos/dbus.
> 
> BUG=739622
> TEST=Ran trybot.
> 
> Change-Id: Ic932aa7c018297a064889c4f05c8578bca712474
> Reviewed-on: https://chromium-review.googlesource.com/666944
> Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
> Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
> Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
> Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
> Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#502831}

TBR=hashimoto@chromium.org,stevenjb@chromium.org,skuhne@chromium.org,bauerb@chromium.org,hidehiko@chromium.org,waffles@chromium.org,emaxx@chromium.org

Change-Id: I4dcf82e6eb3f745582c48c9b035c92b0c72b69ac
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 739622
Reviewed-on: https://chromium-review.googlesource.com/672505
Reviewed-by: Tatiana Gornak <melandory@chromium.org>
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502834}
  • Loading branch information
Tatiana Gornak authored and Commit Bot committed Sep 19, 2017
1 parent 7756d36 commit 34ff18b
Show file tree
Hide file tree
Showing 36 changed files with 328 additions and 255 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
chromeos::attestation::KEY_USER,
cryptohome::Identification(user->GetAccountId()),
chromeos::attestation::kContentProtectionKeyPrefix,
base::BindOnce(
base::Bind(
&ChromeBrowsingDataRemoverDelegate::OnClearPlatformKeys,
weak_ptr_factory_.GetWeakPtr()));
}
Expand Down Expand Up @@ -1179,8 +1179,9 @@ void ChromeBrowsingDataRemoverDelegate::OnClearedCookies() {

#if defined(OS_CHROMEOS)
void ChromeBrowsingDataRemoverDelegate::OnClearPlatformKeys(
base::Optional<bool> result) {
LOG_IF(ERROR, !result.has_value() || !result.value())
chromeos::DBusMethodCallStatus call_status,
bool result) {
LOG_IF(ERROR, call_status != chromeos::DBUS_METHOD_CALL_SUCCESS || !result)
<< "Failed to clear platform keys.";
clear_platform_keys_.GetCompletionCallback().Run();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "base/callback_forward.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/synchronization/waitable_event_watcher.h"
#include "base/task/cancelable_task_tracker.h"
#include "build/build_config.h"
Expand All @@ -30,6 +29,10 @@
#include "chrome/browser/pepper_flash_settings_manager.h"
#endif

#if defined(OS_CHROMEOS)
#include "chromeos/dbus/dbus_method_call_status.h"
#endif

class BrowsingDataFlashLSOHelper;
class Profile;
class WebappRegistry;
Expand Down Expand Up @@ -213,7 +216,8 @@ class ChromeBrowsingDataRemoverDelegate
void OnKeywordsLoaded(base::Callback<bool(const GURL&)> url_filter);

#if defined (OS_CHROMEOS)
void OnClearPlatformKeys(base::Optional<bool> result);
void OnClearPlatformKeys(chromeos::DBusMethodCallStatus call_status,
bool result);
#endif

// Callback for when cookies have been deleted. Invokes NotifyIfDone.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,10 @@ class FakeCryptohomeClient : public chromeos::FakeCryptohomeClient {
chromeos::attestation::AttestationKeyType key_type,
const cryptohome::Identification& cryptohome_id,
const std::string& key_prefix,
chromeos::DBusMethodCallback<bool> callback) override {
const chromeos::BoolDBusMethodCallback& callback) override {
++delete_keys_call_count_;
chromeos::FakeCryptohomeClient::TpmAttestationDeleteKeys(
key_type, cryptohome_id, key_prefix, std::move(callback));
key_type, cryptohome_id, key_prefix, callback);
}

int delete_keys_call_count() const { return delete_keys_call_count_; }
Expand Down
16 changes: 8 additions & 8 deletions chrome/browser/chromeos/app_mode/kiosk_profile_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "base/logging.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/string_util.h"
#include "base/sys_info.h"
Expand Down Expand Up @@ -57,9 +56,9 @@ KioskAppLaunchError::Error LoginFailureToKioskAppLaunchError(
////////////////////////////////////////////////////////////////////////////////
// KioskProfileLoader::CryptohomedChecker ensures cryptohome daemon is up
// and running by issuing an IsMounted call. If the call does not go through
// and base::nullopt is not returned, it will retry after some time out and at
// the maximum five times before it gives up. Upon success, it resumes the
// launch by logging in as a kiosk mode account.
// and chromeos::DBUS_METHOD_CALL_SUCCESS is not returned, it will retry after
// some time out and at the maximum five times before it gives up. Upon
// success, it resumes the launch by logging in as a kiosk mode account.

class KioskProfileLoader::CryptohomedChecker
: public base::SupportsWeakPtr<CryptohomedChecker> {
Expand Down Expand Up @@ -103,17 +102,18 @@ class KioskProfileLoader::CryptohomedChecker
base::Bind(&CryptohomedChecker::OnCryptohomeIsMounted, AsWeakPtr()));
}

void OnCryptohomeIsMounted(base::Optional<bool> is_mounted) {
if (!is_mounted.has_value()) {
void OnCryptohomeIsMounted(chromeos::DBusMethodCallStatus call_status,
bool is_mounted) {
if (call_status != chromeos::DBUS_METHOD_CALL_SUCCESS) {
Retry();
return;
}

if (!is_mounted.value())
if (is_mounted)
SYSLOG(ERROR) << "Cryptohome is mounted before launching kiosk app.";

// Proceed only when cryptohome is not mounded or running on dev box.
if (!is_mounted.value() || !base::SysInfo::IsRunningOnChromeOS())
if (!is_mounted || !base::SysInfo::IsRunningOnChromeOS())
ReportCheckResult(KioskAppLaunchError::NONE);
else
ReportCheckResult(KioskAppLaunchError::ALREADY_MOUNTED);
Expand Down
25 changes: 13 additions & 12 deletions chrome/browser/chromeos/attestation/attestation_policy_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "base/bind.h"
#include "base/callback.h"
#include "base/location.h"
#include "base/optional.h"
#include "base/time/time.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/chromeos/attestation/attestation_ca_client.h"
Expand Down Expand Up @@ -48,19 +47,21 @@ const int kRetryLimit = 100;
// on_true - Called when status=success and value=true.
// on_false - Called when status=success and value=false.
// status - The dbus operation status.
// result - The value returned by the dbus operation.
// value - The value returned by the dbus operation.
void DBusBoolRedirectCallback(const base::Closure& on_true,
const base::Closure& on_false,
const base::Closure& on_failure,
const base::Location& from_here,
base::Optional<bool> result) {
if (!result.has_value()) {
LOG(ERROR) << "Cryptohome DBus method failed: " << from_here.ToString();
chromeos::DBusMethodCallStatus status,
bool value) {
if (status != chromeos::DBUS_METHOD_CALL_SUCCESS) {
LOG(ERROR) << "Cryptohome DBus method failed: " << from_here.ToString()
<< " - " << status;
if (!on_failure.is_null())
on_failure.Run();
return;
}
const base::Closure& task = result.value() ? on_true : on_false;
const base::Closure& task = value ? on_true : on_false;
if (!task.is_null())
task.Run();
}
Expand Down Expand Up @@ -176,10 +177,10 @@ void AttestationPolicyObserver::Start() {
KEY_DEVICE,
cryptohome::Identification(), // Not used.
kEnterpriseMachineKey,
base::BindOnce(DBusBoolRedirectCallback, on_does_exist, on_does_not_exist,
base::Bind(&AttestationPolicyObserver::Reschedule,
weak_factory_.GetWeakPtr()),
FROM_HERE));
base::Bind(DBusBoolRedirectCallback, on_does_exist, on_does_not_exist,
base::Bind(&AttestationPolicyObserver::Reschedule,
weak_factory_.GetWeakPtr()),
FROM_HERE));
}

void AttestationPolicyObserver::GetNewCertificate() {
Expand Down Expand Up @@ -302,8 +303,8 @@ void AttestationPolicyObserver::MarkAsUploaded(const std::string& key_payload) {
KEY_DEVICE,
cryptohome::Identification(), // Not used.
kEnterpriseMachineKey, new_payload,
base::BindOnce(DBusBoolRedirectCallback, base::Closure(), base::Closure(),
base::Closure(), FROM_HERE));
base::Bind(DBusBoolRedirectCallback, base::Closure(), base::Closure(),
base::Closure(), FROM_HERE));
}

void AttestationPolicyObserver::Reschedule() {
Expand Down
19 changes: 10 additions & 9 deletions chrome/browser/chromeos/attestation/platform_verification_flow.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,10 @@ const int kOpportunisticRenewalThresholdInDays = 30;
// A callback method to handle DBus errors.
void DBusCallback(const base::Callback<void(bool)>& on_success,
const base::Closure& on_failure,
base::Optional<bool> result) {
if (result.has_value()) {
on_success.Run(result.value());
chromeos::DBusMethodCallStatus call_status,
bool result) {
if (call_status == chromeos::DBUS_METHOD_CALL_SUCCESS) {
on_success.Run(result);
} else {
LOG(ERROR) << "PlatformVerificationFlow: DBus call failed!";
on_failure.Run();
Expand Down Expand Up @@ -228,13 +229,13 @@ void PlatformVerificationFlow::ChallengePlatformKey(
}

ChallengeContext context(web_contents, service_id, challenge, callback);

// Check if the device has been prepared to use attestation.
cryptohome_client_->TpmAttestationIsPrepared(base::BindOnce(
&DBusCallback,
base::Bind(&PlatformVerificationFlow::OnAttestationPrepared, this,
context),
base::Bind(&ReportError, callback, INTERNAL_ERROR)));
BoolDBusMethodCallback dbus_callback =
base::Bind(&DBusCallback,
base::Bind(&PlatformVerificationFlow::OnAttestationPrepared,
this, context),
base::Bind(&ReportError, callback, INTERNAL_ERROR));
cryptohome_client_->TpmAttestationIsPrepared(dbus_callback);
}

void PlatformVerificationFlow::OnAttestationPrepared(
Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/chromeos/chrome_browser_main_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/path_service.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h"
Expand Down Expand Up @@ -512,8 +511,8 @@ class SystemTokenCertDBInitializer {
// This is a callback for the cryptohome TpmIsReady query. Note that this is
// not a listener which would be called once TPM becomes ready if it was not
// ready on startup (e.g. after device enrollment), see crbug.com/725500.
void OnGotTpmIsReady(base::Optional<bool> tpm_is_ready) {
if (!tpm_is_ready.has_value() || !tpm_is_ready.value()) {
void OnGotTpmIsReady(DBusMethodCallStatus call_status, bool tpm_is_ready) {
if (!tpm_is_ready) {
VLOG(1) << "SystemTokenCertDBInitializer: TPM is not ready - not loading "
"system token.";
return;
Expand Down
17 changes: 9 additions & 8 deletions chrome/browser/chromeos/login/screens/user_selection_screen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "base/location.h"
#include "base/logging.h"
#include "base/memory/ptr_util.h"
#include "base/optional.h"
#include "base/strings/utf_string_conversions.h"
#include "base/values.h"
#include "chrome/browser/browser_process.h"
Expand All @@ -36,6 +35,7 @@
#include "chromeos/chromeos_switches.h"
#include "chromeos/cryptohome/cryptohome_parameters.h"
#include "chromeos/dbus/cryptohome_client.h"
#include "chromeos/dbus/dbus_method_call_status.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "components/arc/arc_util.h"
#include "components/prefs/pref_service.h"
Expand Down Expand Up @@ -270,25 +270,26 @@ class UserSelectionScreen::DircryptoMigrationChecker {
const cryptohome::Identification cryptohome_id(account_id);
DBusThreadManager::Get()->GetCryptohomeClient()->NeedsDircryptoMigration(
cryptohome_id,
base::BindOnce(&DircryptoMigrationChecker::
OnCryptohomeNeedsDircryptoMigrationCallback,
weak_ptr_factory_.GetWeakPtr(), account_id));
base::Bind(&DircryptoMigrationChecker::
OnCryptohomeNeedsDircryptoMigrationCallback,
weak_ptr_factory_.GetWeakPtr(), account_id));
}

// Callback invoked when NeedsDircryptoMigration call is finished.
void OnCryptohomeNeedsDircryptoMigrationCallback(
const AccountId& account_id,
base::Optional<bool> needs_migration) {
if (!needs_migration.has_value()) {
DBusMethodCallStatus call_status,
bool needs_migration) {
if (call_status != DBUS_METHOD_CALL_SUCCESS) {
LOG(ERROR) << "Failed to call cryptohome NeedsDircryptoMigration.";
// Hide the banner to avoid confusion in http://crbug.com/721948.
// Cache is not updated so that cryptohome call will still be attempted.
UpdateUI(account_id, false);
return;
}

needs_dircrypto_migration_cache_[account_id] = needs_migration.value();
UpdateUI(account_id, needs_migration.value());
needs_dircrypto_migration_cache_[account_id] = needs_migration;
UpdateUI(account_id, needs_migration);
}

// Update UI for the given user when the check result is available.
Expand Down
8 changes: 5 additions & 3 deletions chrome/browser/chromeos/policy/pre_signin_policy_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,18 @@ void PreSigninPolicyFetcher::OnCachedPolicyRetrieved(
void PreSigninPolicyFetcher::OnPolicyKeyLoaded(
const std::string& policy_blob,
RetrievePolicyResponseType retrieve_policy_response) {
cryptohome_client_->Unmount(base::BindOnce(
cryptohome_client_->Unmount(base::Bind(
&PreSigninPolicyFetcher::OnUnmountTemporaryUserHome,
weak_ptr_factory_.GetWeakPtr(), policy_blob, retrieve_policy_response));
}

void PreSigninPolicyFetcher::OnUnmountTemporaryUserHome(
const std::string& policy_blob,
RetrievePolicyResponseType retrieve_policy_response,
base::Optional<bool> unmount_success) {
if (!unmount_success.has_value() || !unmount_success.value()) {
chromeos::DBusMethodCallStatus unmount_call_status,
bool unmount_success) {
if (unmount_call_status != chromeos::DBUS_METHOD_CALL_SUCCESS ||
!unmount_success) {
// The temporary userhome mount could not be unmounted. Log an error and
// continue, and hope that the unmount will be successful on the next mount
// (temporary user homes are automatically unmounted by cryptohomed on every
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/chromeos/policy/pre_signin_policy_fetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/timer/timer.h"
#include "chrome/browser/chromeos/policy/cached_policy_key_loader_chromeos.h"
#include "chromeos/cryptohome/cryptohome_parameters.h"
#include "chromeos/dbus/dbus_method_call_status.h"
#include "chromeos/dbus/session_manager_client.h"
#include "components/policy/core/common/cloud/cloud_policy_client.h"
#include "components/policy/core/common/cloud/cloud_policy_validator.h"
Expand Down Expand Up @@ -102,7 +102,8 @@ class PreSigninPolicyFetcher : public CloudPolicyClient::Observer {
void OnUnmountTemporaryUserHome(
const std::string& policy_blob,
RetrievePolicyResponseType retrieve_policy_response,
base::Optional<bool> unmount_success);
chromeos::DBusMethodCallStatus unmount_call_status,
bool unmount_success);

void OnCachedPolicyValidated(UserCloudPolicyValidator* validator);

Expand Down
32 changes: 20 additions & 12 deletions chrome/browser/chromeos/settings/install_attributes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,9 @@ void InstallAttributes::ReadImmutableAttributes(const base::Closure& callback) {
}

void InstallAttributes::ReadAttributesIfReady(const base::Closure& callback,
base::Optional<bool> is_ready) {
if (is_ready.value_or(false)) {
DBusMethodCallStatus call_status,
bool result) {
if (call_status == DBUS_METHOD_CALL_SUCCESS && result) {
registration_mode_ = policy::DEVICE_MODE_NOT_SET;
if (!cu::InstallAttributesIsInvalid() &&
!cu::InstallAttributesIsFirstInstall()) {
Expand Down Expand Up @@ -257,9 +258,13 @@ void InstallAttributes::LockDevice(policy::DeviceMode device_mode,

device_lock_running_ = true;
cryptohome_client_->InstallAttributesIsReady(
base::BindOnce(&InstallAttributes::LockDeviceIfAttributesIsReady,
weak_ptr_factory_.GetWeakPtr(), device_mode, domain, realm,
device_id, callback));
base::Bind(&InstallAttributes::LockDeviceIfAttributesIsReady,
weak_ptr_factory_.GetWeakPtr(),
device_mode,
domain,
realm,
device_id,
callback));
}

void InstallAttributes::LockDeviceIfAttributesIsReady(
Expand All @@ -268,8 +273,9 @@ void InstallAttributes::LockDeviceIfAttributesIsReady(
const std::string& realm,
const std::string& device_id,
const LockResultCallback& callback,
base::Optional<bool> is_ready) {
if (!is_ready.has_value() || !is_ready.value()) {
DBusMethodCallStatus call_status,
bool result) {
if (call_status != DBUS_METHOD_CALL_SUCCESS || !result) {
device_lock_running_ = false;
callback.Run(LOCK_NOT_READY);
return;
Expand Down Expand Up @@ -387,9 +393,11 @@ void InstallAttributes::TriggerConsistencyCheck(int dbus_retries) {
dbus_retries));
}

void InstallAttributes::OnTpmOwnerCheckCompleted(int dbus_retries_remaining,
base::Optional<bool> result) {
if (!result.has_value() && dbus_retries_remaining) {
void InstallAttributes::OnTpmOwnerCheckCompleted(
int dbus_retries_remaining,
DBusMethodCallStatus call_status,
bool result) {
if (call_status != DBUS_METHOD_CALL_SUCCESS && dbus_retries_remaining) {
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::Bind(&InstallAttributes::TriggerConsistencyCheck,
Expand All @@ -400,8 +408,8 @@ void InstallAttributes::OnTpmOwnerCheckCompleted(int dbus_retries_remaining,

base::HistogramBase::Sample state = device_locked_;
state |= 0x2 * (registration_mode_ == policy::DEVICE_MODE_ENTERPRISE);
if (!result.has_value())
state |= 0x4 * result.value();
if (call_status == DBUS_METHOD_CALL_SUCCESS)
state |= 0x4 * result;
else
state = 0x8; // This case is not a bit mask.
UMA_HISTOGRAM_ENUMERATION("Enterprise.AttributesTPMConsistency", state, 9);
Expand Down
Loading

0 comments on commit 34ff18b

Please sign in to comment.