Commit 479399dd authored by Yusuke Sato's avatar Yusuke Sato Committed by Commit Bot

arc: Add cryptohome::Identification param to SetUserInfo()

This will allow ArcVmClientAdapter to start arc-create-data job
to create /home/root/<hash>/android-data directory.

BUG=b:151683475
TEST=try

Change-Id: I0769ca10a5b48dec0f6cb2c4891503a5915ab058
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135308Reviewed-by: default avatarHidehiko Abe <hidehiko@chromium.org>
Reviewed-by: default avatarLong Cheng <lgcheng@google.com>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756904}
parent a89c23a4
......@@ -516,7 +516,9 @@ void ArcSessionManager::Initialize() {
auto* prefs = profile_->GetPrefs();
const std::string user_id_hash(
chromeos::ProfileHelper::GetUserIdHashFromProfile(profile_));
arc_session_runner_->SetUserInfo(user_id_hash,
const cryptohome::Identification cryptohome_id(
multi_user_util::GetAccountIdFromProfile(profile_));
arc_session_runner_->SetUserInfo(cryptohome_id, user_id_hash,
GetOrCreateSerialNumber(prefs));
// Create the support host at initialization. Note that, practically,
......@@ -534,9 +536,7 @@ void ArcSessionManager::Initialize() {
support_host_ = std::make_unique<ArcSupportHost>(profile_);
support_host_->SetErrorDelegate(this);
}
data_remover_ = std::make_unique<ArcDataRemover>(
prefs, cryptohome::Identification(
multi_user_util::GetAccountIdFromProfile(profile_)));
data_remover_ = std::make_unique<ArcDataRemover>(prefs, cryptohome_id);
data_remover_->set_user_id_hash_for_profile(user_id_hash);
if (g_enable_check_android_management_in_tests.value_or(g_ui_enabled))
......
......@@ -14,6 +14,10 @@
#include "components/arc/session/arc_start_params.h"
#include "components/arc/session/arc_upgrade_params.h"
namespace cryptohome {
class Identification;
} // namespace cryptohome
namespace arc {
// An adapter to talk to a Chrome OS daemon to manage lifetime of ARC instance.
......@@ -42,9 +46,10 @@ class ArcClientAdapter {
// is called due to the browser being shut down.
virtual void StopArcInstance(bool on_shutdown) = 0;
// Sets a hash string of the profile user ID and an ARC serial number for the
// Sets a hash string of the profile user IDs and an ARC serial number for the
// user.
virtual void SetUserInfo(const std::string& hash,
virtual void SetUserInfo(const cryptohome::Identification& cryptohome_id,
const std::string& hash,
const std::string& serial_number) = 0;
void AddObserver(Observer* observer);
......
......@@ -134,7 +134,8 @@ class ArcContainerClientAdapter
chromeos::SessionManagerClient::Get()->StopArcInstance(base::DoNothing());
}
void SetUserInfo(const std::string& hash,
void SetUserInfo(const cryptohome::Identification& cryptohome_id,
const std::string& hash,
const std::string& serial_number) override {}
// chromeos::SessionManagerClient::Observer overrides:
......
......@@ -21,6 +21,10 @@ namespace chromeos {
class SchedulerConfigurationManagerBase;
}
namespace cryptohome {
class Identification;
}
namespace version_info {
enum class Channel;
}
......@@ -82,9 +86,10 @@ class ArcSession {
// when it has already been called before.
virtual void OnShutdown() = 0;
// Sets a hash string of the profile user ID and an ARC serial number for the
// Sets a hash string of the profile user IDs and an ARC serial number for the
// user.
virtual void SetUserInfo(const std::string& hash,
virtual void SetUserInfo(const cryptohome::Identification& cryptohome_id,
const std::string& hash,
const std::string& serial_number) = 0;
void AddObserver(Observer* observer);
......
......@@ -726,10 +726,12 @@ void ArcSessionImpl::OnShutdown() {
OnStopped(ArcStopReason::SHUTDOWN);
}
void ArcSessionImpl::SetUserInfo(const std::string& hash,
void ArcSessionImpl::SetUserInfo(
const cryptohome::Identification& cryptohome_id,
const std::string& hash,
const std::string& serial_number) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
client_->SetUserInfo(hash, serial_number);
client_->SetUserInfo(cryptohome_id, hash, serial_number);
}
void ArcSessionImpl::OnConfigurationSet(bool success,
......
......@@ -25,6 +25,10 @@ namespace ash {
class DefaultScaleFactorRetriever;
}
namespace cryptohome {
class Identification;
}
namespace arc {
namespace mojom {
......@@ -194,7 +198,8 @@ class ArcSessionImpl
void Stop() override;
bool IsStopRequested() override;
void OnShutdown() override;
void SetUserInfo(const std::string& hash,
void SetUserInfo(const cryptohome::Identification& cryptohome_id,
const std::string& hash,
const std::string& serial_number) override;
// chromeos::SchedulerConfigurationManagerBase::Observer overrides:
......
......@@ -27,6 +27,10 @@
#include "components/version_info/channel.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace cryptohome {
class Identification;
} // namespace cryptohome
namespace arc {
namespace {
......@@ -74,7 +78,8 @@ class FakeArcClientAdapter : public ArcClientAdapter {
base::Unretained(this)));
}
void SetUserInfo(const std::string& hash,
void SetUserInfo(const cryptohome::Identification& cryptohome_id,
const std::string& hash,
const std::string& serial_number) override {}
// Notifies ArcSessionImpl of the ARC instance stop event.
......
......@@ -235,17 +235,20 @@ void ArcSessionRunner::OnShutdown() {
DCHECK(!arc_session_);
}
void ArcSessionRunner::SetUserInfo(const std::string& hash,
void ArcSessionRunner::SetUserInfo(
const cryptohome::Identification& cryptohome_id,
const std::string& hash,
const std::string& serial_number) {
// |hash| can be empty in unit tests. This function can also be called
// multiple times in tests.
// TODO(yusukes): Fix tests and add DCHECKs to make sure |hash| is not
// empty and the function is called only once.
// |cryptohome_id.id()| and |hash| can be empty in unit tests. This function
// can also be called multiple times in tests.
// TODO(yusukes): Fix tests and add DCHECKs to make sure they are not empty
// and the function is called only once.
DCHECK(!serial_number.empty());
cryptohome_id_ = cryptohome_id;
user_id_hash_ = hash;
serial_number_ = serial_number;
if (arc_session_)
arc_session_->SetUserInfo(user_id_hash_, serial_number_);
arc_session_->SetUserInfo(cryptohome_id_, user_id_hash_, serial_number_);
}
void ArcSessionRunner::SetRestartDelayForTesting(
......@@ -263,8 +266,10 @@ void ArcSessionRunner::StartArcSession() {
VLOG(1) << "Starting ARC instance";
if (!arc_session_) {
arc_session_ = factory_.Run();
if (!user_id_hash_.empty() && !serial_number_.empty())
arc_session_->SetUserInfo(user_id_hash_, serial_number_);
if (!cryptohome_id_.id().empty() && !user_id_hash_.empty() &&
!serial_number_.empty()) {
arc_session_->SetUserInfo(cryptohome_id_, user_id_hash_, serial_number_);
}
arc_session_->AddObserver(this);
arc_session_->StartMiniInstance();
// Record the UMA only when |restart_after_crash_count_| is zero to avoid
......
......@@ -17,6 +17,7 @@
#include "base/threading/thread_checker.h"
#include "base/time/time.h"
#include "base/timer/timer.h"
#include "chromeos/cryptohome/cryptohome_parameters.h"
#include "components/arc/session/arc_instance_mode.h"
#include "components/arc/session/arc_session.h"
#include "components/arc/session/arc_stop_reason.h"
......@@ -98,9 +99,11 @@ class ArcSessionRunner : public ArcSession::Observer {
// when this function is called, MessageLoop is no longer exists.
void OnShutdown();
// Sets a hash string of the profile user ID and an ARC serial number for the
// Sets a hash string of the profile user IDs and an ARC serial number for the
// user.
void SetUserInfo(const std::string& hash, const std::string& serial_number);
void SetUserInfo(const cryptohome::Identification& cryptohome_id,
const std::string& hash,
const std::string& serial_number);
// Returns the current ArcSession instance for testing purpose.
ArcSession* GetArcSessionForTesting() { return arc_session_.get(); }
......@@ -150,6 +153,8 @@ class ArcSessionRunner : public ArcSession::Observer {
// Parameters to upgrade request.
UpgradeParams upgrade_params_;
// A cryptohome ID of the profile.
cryptohome::Identification cryptohome_id_;
// A hash string of the profile user ID.
std::string user_id_hash_;
// A serial number for the current profile.
......
......@@ -31,6 +31,7 @@
#include "base/task/thread_pool.h"
#include "base/time/time.h"
#include "chromeos/constants/chromeos_switches.h"
#include "chromeos/cryptohome/cryptohome_parameters.h"
#include "chromeos/dbus/concierge_client.h"
#include "chromeos/dbus/dbus_method_call_status.h"
#include "chromeos/dbus/dbus_thread_manager.h"
......@@ -305,10 +306,10 @@ class ArcVmClientAdapter : public ArcClientAdapter,
// Initializing |is_host_on_vm_| and |is_dev_mode_| is not always very fast.
// Try to initialize them in the constructor and in StartMiniArc respectively.
// They usually run when the system is not busy.
explicit ArcVmClientAdapter() : ArcVmClientAdapter({}) {}
ArcVmClientAdapter() : ArcVmClientAdapter(FileSystemStatusRewriter{}) {}
// For testing purposes and the internal use (by the other ctor) only.
ArcVmClientAdapter(const FileSystemStatusRewriter& rewriter)
explicit ArcVmClientAdapter(const FileSystemStatusRewriter& rewriter)
: is_host_on_vm_(chromeos::system::StatisticsProvider::GetInstance()
->IsRunningOnVm()),
file_system_status_rewriter_for_testing_(rewriter) {
......@@ -391,14 +392,19 @@ class ArcVmClientAdapter : public ArcClientAdapter,
weak_factory_.GetWeakPtr()));
}
void SetUserInfo(const std::string& hash,
void SetUserInfo(const cryptohome::Identification& cryptohome_id,
const std::string& hash,
const std::string& serial_number) override {
DCHECK(cryptohome_id_.id().empty());
DCHECK(user_id_hash_.empty());
DCHECK(serial_number_.empty());
if (cryptohome_id.id().empty())
LOG(WARNING) << "cryptohome_id is empty";
if (hash.empty())
LOG(WARNING) << "hash is empty";
if (serial_number.empty())
LOG(WARNING) << "serial_number is empty";
cryptohome_id_ = cryptohome_id;
user_id_hash_ = hash;
serial_number_ = serial_number;
}
......@@ -629,6 +635,8 @@ class ArcVmClientAdapter : public ArcClientAdapter,
// True when the *host* is running on a VM.
const bool is_host_on_vm_;
// A cryptohome ID of the primary profile.
cryptohome::Identification cryptohome_id_;
// A hash of the primary profile user ID.
std::string user_id_hash_;
// A serial number for the current profile.
......
......@@ -20,6 +20,7 @@
#include "base/task/post_task.h"
#include "base/test/bind_test_util.h"
#include "base/time/time.h"
#include "chromeos/cryptohome/cryptohome_parameters.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/debug_daemon/fake_debug_daemon_client.h"
#include "chromeos/dbus/fake_concierge_client.h"
......@@ -27,6 +28,7 @@
#include "components/arc/arc_util.h"
#include "components/arc/session/arc_session.h"
#include "components/arc/session/file_system_status.h"
#include "components/user_manager/user_names.h"
#include "content/public/test/browser_task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -191,8 +193,12 @@ class ArcVmClientAdapterTest : public testing::Test,
GetTestDebugDaemonClient()->set_start_concierge_result(response);
}
void SetValidUserInfo() {
adapter()->SetUserInfo(kUserIdHash, kSerialNumber);
void SetValidUserInfo() { SetUserInfo(kUserIdHash, kSerialNumber); }
void SetUserInfo(const std::string& hash, const std::string& serial) {
adapter()->SetUserInfo(
cryptohome::Identification(user_manager::StubAccountId()), hash,
serial);
}
void StartMiniArcWithParams(bool expect_success, StartParams params) {
......@@ -341,7 +347,7 @@ class ArcVmClientAdapterTest : public testing::Test,
// Tests that SetUserInfo() doesn't crash.
TEST_F(ArcVmClientAdapterTest, SetUserInfo) {
adapter()->SetUserInfo(kUserIdHash, kSerialNumber);
SetUserInfo(kUserIdHash, kSerialNumber);
}
// Tests that StartMiniArc() succeeds by default.
......@@ -472,7 +478,7 @@ TEST_F(ArcVmClientAdapterTest, UpgradeArc_StartConciergeFailure) {
TEST_F(ArcVmClientAdapterTest, UpgradeArc_NoUserId) {
// Don't set the user id hash. Note that we cannot call StartArcVm() without
// it.
adapter()->SetUserInfo(std::string(), kSerialNumber);
SetUserInfo(std::string(), kSerialNumber);
StartMiniArc();
UpgradeArc(false);
......@@ -495,7 +501,7 @@ TEST_F(ArcVmClientAdapterTest, UpgradeArc_NoUserId) {
TEST_F(ArcVmClientAdapterTest, UpgradeArc_NoSerial) {
// Don't set the serial number. Note that we cannot call StartArcVm() without
// it.
adapter()->SetUserInfo(kUserIdHash, std::string());
SetUserInfo(kUserIdHash, std::string());
StartMiniArc();
UpgradeArc(false);
......
......@@ -40,7 +40,9 @@ void FakeArcSession::OnShutdown() {
StopWithReason(ArcStopReason::SHUTDOWN);
}
void FakeArcSession::SetUserInfo(const std::string& hash,
void FakeArcSession::SetUserInfo(
const cryptohome::Identification& cryptohome_id,
const std::string& hash,
const std::string& serial_number) {}
void FakeArcSession::StopWithReason(ArcStopReason reason) {
......
......@@ -12,6 +12,10 @@
#include "components/arc/session/arc_session.h"
#include "components/arc/session/arc_stop_reason.h"
namespace cryptohome {
class Identification;
} // namespace cryptohome
namespace arc {
// A fake ArcSession that creates a local connection.
......@@ -26,7 +30,8 @@ class FakeArcSession : public ArcSession {
void Stop() override;
bool IsStopRequested() override;
void OnShutdown() override;
void SetUserInfo(const std::string& hash,
void SetUserInfo(const cryptohome::Identification& cryptohome_id,
const std::string& hash,
const std::string& serial_number) override;
// To emulate unexpected stop, such as crash.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment