Commit 63b98149 authored by Christopher Morin's avatar Christopher Morin Committed by Commit Bot

Revert "Start arc data removal directly from Chrome"

This reverts commit 756ebd67.

Reason for revert: Missing file on chromeos side to support this change.

Original change's description:
> Start arc data removal directly from Chrome
> 
> Chrome had to proxy through session_manager to start the arc-remove-data
> Upstart job, but now it calls it directly.
> 
> BUG=b:115779632
> TEST=Disable ARC++ and ensure arc data removal occurs
> 
> Change-Id: I0f30570c665efc85538b9de4bcd63bb158b37417
> Reviewed-on: https://chromium-review.googlesource.com/c/1311124
> Reviewed-by: Yusuke Sato <yusukes@chromium.org>
> Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
> Commit-Queue: Christopher Morin <cmtm@google.com>
> Cr-Commit-Position: refs/heads/master@{#604724}

TBR=hashimoto@chromium.org,yusukes@chromium.org,alemate@chromium.org,hidehiko@chromium.org,cmtm@google.com,ereth@chromium.org

Change-Id: I46bc58d45f3749363a0f4e72948d13293832b186
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b:115779632
Reviewed-on: https://chromium-review.googlesource.com/c/1313755Reviewed-by: default avatarRyo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Christopher Morin <cmtm@google.com>
Cr-Commit-Position: refs/heads/master@{#604731}
parent 84ae0017
...@@ -589,6 +589,13 @@ void FakeSessionManagerClient::GetArcStartTime( ...@@ -589,6 +589,13 @@ void FakeSessionManagerClient::GetArcStartTime(
arc_available_ ? base::make_optional(arc_start_time_) : base::nullopt); arc_available_ ? base::make_optional(arc_start_time_) : base::nullopt);
} }
void FakeSessionManagerClient::RemoveArcData(
const cryptohome::AccountIdentifier& cryptohome_id,
VoidDBusMethodCallback callback) {
if (!callback.is_null())
PostReply(FROM_HERE, std::move(callback), arc_available_);
}
void FakeSessionManagerClient::NotifyArcInstanceStopped( void FakeSessionManagerClient::NotifyArcInstanceStopped(
login_manager::ArcContainerStopReason reason, login_manager::ArcContainerStopReason reason,
const std::string& container_instance_id) { const std::string& container_instance_id) {
......
...@@ -109,6 +109,8 @@ class FakeSessionManagerClient : public SessionManagerClient { ...@@ -109,6 +109,8 @@ class FakeSessionManagerClient : public SessionManagerClient {
void EmitArcBooted(const cryptohome::AccountIdentifier& cryptohome_id, void EmitArcBooted(const cryptohome::AccountIdentifier& cryptohome_id,
VoidDBusMethodCallback callback) override; VoidDBusMethodCallback callback) override;
void GetArcStartTime(DBusMethodCallback<base::TimeTicks> callback) override; void GetArcStartTime(DBusMethodCallback<base::TimeTicks> callback) override;
void RemoveArcData(const cryptohome::AccountIdentifier& cryptohome_id,
VoidDBusMethodCallback callback) override;
// Notifies observers as if ArcInstanceStopped signal is received. // Notifies observers as if ArcInstanceStopped signal is received.
void NotifyArcInstanceStopped(login_manager::ArcContainerStopReason, void NotifyArcInstanceStopped(login_manager::ArcContainerStopReason,
......
...@@ -474,6 +474,18 @@ class SessionManagerClientImpl : public SessionManagerClient { ...@@ -474,6 +474,18 @@ class SessionManagerClientImpl : public SessionManagerClient {
weak_ptr_factory_.GetWeakPtr(), std::move(callback))); weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
} }
void RemoveArcData(const cryptohome::AccountIdentifier& cryptohome_id,
VoidDBusMethodCallback callback) override {
dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
login_manager::kSessionManagerRemoveArcData);
dbus::MessageWriter writer(&method_call);
writer.AppendString(cryptohome_id.account_id());
session_manager_proxy_->CallMethod(
&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
base::BindOnce(&SessionManagerClientImpl::OnVoidMethod,
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}
protected: protected:
void Init(dbus::Bus* bus) override { void Init(dbus::Bus* bus) override {
session_manager_proxy_ = bus->GetObjectProxy( session_manager_proxy_ = bus->GetObjectProxy(
......
...@@ -365,6 +365,13 @@ class CHROMEOS_EXPORT SessionManagerClient : public DBusClient { ...@@ -365,6 +365,13 @@ class CHROMEOS_EXPORT SessionManagerClient : public DBusClient {
virtual void GetArcStartTime( virtual void GetArcStartTime(
DBusMethodCallback<base::TimeTicks> callback) = 0; DBusMethodCallback<base::TimeTicks> callback) = 0;
// Asynchronously removes all ARC user data for the user whose cryptohome is
// located by |cryptohome_id|. Upon completion, invokes |callback| with the
// result; true on success, false on failure (either session manager failed
// to remove user data or session manager can not be reached).
virtual void RemoveArcData(const cryptohome::AccountIdentifier& cryptohome_id,
VoidDBusMethodCallback callback) = 0;
// Creates the instance. // Creates the instance.
static SessionManagerClient* Create(DBusClientImplementationType type); static SessionManagerClient* Create(DBusClientImplementationType type);
......
...@@ -10,15 +10,11 @@ ...@@ -10,15 +10,11 @@
#include "base/callback_helpers.h" #include "base/callback_helpers.h"
#include "base/logging.h" #include "base/logging.h"
#include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/upstart_client.h" #include "chromeos/dbus/session_manager_client.h"
#include "components/arc/arc_prefs.h" #include "components/arc/arc_prefs.h"
namespace arc { namespace arc {
// The conversion of upstart job names to dbus object paths is undocumented. See
// function nih_dbus_path in libnih for the implementation.
constexpr char kArcRemoveDataUpstartJob[] = "arc_2dremove_2ddata";
ArcDataRemover::ArcDataRemover(PrefService* prefs, ArcDataRemover::ArcDataRemover(PrefService* prefs,
const cryptohome::Identification& cryptohome_id) const cryptohome::Identification& cryptohome_id)
: cryptohome_id_(cryptohome_id), weak_factory_(this) { : cryptohome_id_(cryptohome_id), weak_factory_(this) {
...@@ -46,13 +42,11 @@ void ArcDataRemover::Run(RunCallback callback) { ...@@ -46,13 +42,11 @@ void ArcDataRemover::Run(RunCallback callback) {
} }
VLOG(1) << "Starting ARC data removal"; VLOG(1) << "Starting ARC data removal";
auto* upstart_client = chromeos::DBusThreadManager::Get()->GetUpstartClient(); auto* session_manager_client =
DCHECK(upstart_client); chromeos::DBusThreadManager::Get()->GetSessionManagerClient();
const std::string account_id = DCHECK(session_manager_client);
cryptohome::CreateAccountIdentifierFromIdentification(cryptohome_id_) session_manager_client->RemoveArcData(
.account_id(); cryptohome::CreateAccountIdentifierFromIdentification(cryptohome_id_),
upstart_client->StartJob(
kArcRemoveDataUpstartJob, {"CHROMEOS_USER=" + account_id},
base::AdaptCallbackForRepeating( base::AdaptCallbackForRepeating(
base::BindOnce(&ArcDataRemover::OnDataRemoved, base::BindOnce(&ArcDataRemover::OnDataRemoved,
weak_factory_.GetWeakPtr(), std::move(callback)))); weak_factory_.GetWeakPtr(), std::move(callback))));
...@@ -65,7 +59,7 @@ void ArcDataRemover::OnDataRemoved(RunCallback callback, bool success) { ...@@ -65,7 +59,7 @@ void ArcDataRemover::OnDataRemoved(RunCallback callback, bool success) {
VLOG(1) << "ARC data removal successful"; VLOG(1) << "ARC data removal successful";
} else { } else {
LOG(ERROR) << "Request for ARC user data removal failed. " LOG(ERROR) << "Request for ARC user data removal failed. "
<< "See upstart logs for more details."; << "See session_manager logs for more details.";
} }
pref_.SetValue(false); pref_.SetValue(false);
......
...@@ -9,10 +9,9 @@ ...@@ -9,10 +9,9 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "base/threading/thread_task_runner_handle.h"
#include "chromeos/cryptohome/cryptohome_parameters.h" #include "chromeos/cryptohome/cryptohome_parameters.h"
#include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/fake_upstart_client.h" #include "chromeos/dbus/fake_session_manager_client.h"
#include "components/account_id/account_id.h" #include "components/account_id/account_id.h"
#include "components/arc/arc_prefs.h" #include "components/arc/arc_prefs.h"
#include "components/prefs/testing_pref_service.h" #include "components/prefs/testing_pref_service.h"
...@@ -21,33 +20,13 @@ ...@@ -21,33 +20,13 @@
namespace arc { namespace arc {
namespace { namespace {
class TestUpstartClient : public chromeos::FakeUpstartClient {
public:
TestUpstartClient() = default;
~TestUpstartClient() override = default;
void StartJob(const std::string& job,
const std::vector<std::string>& upstart_env,
chromeos::VoidDBusMethodCallback callback) override {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(callback), arc_available_));
}
void set_arc_available(bool arc_available) { arc_available_ = arc_available; }
private:
bool arc_available_ = false;
DISALLOW_COPY_AND_ASSIGN(TestUpstartClient);
};
class ArcDataRemoverTest : public testing::Test { class ArcDataRemoverTest : public testing::Test {
public: public:
ArcDataRemoverTest() = default; ArcDataRemoverTest() = default;
void SetUp() override { void SetUp() override {
chromeos::DBusThreadManager::GetSetterForTesting()->SetUpstartClient( chromeos::DBusThreadManager::GetSetterForTesting()->SetSessionManagerClient(
std::make_unique<TestUpstartClient>()); std::make_unique<chromeos::FakeSessionManagerClient>());
chromeos::DBusThreadManager::Initialize(); chromeos::DBusThreadManager::Initialize();
prefs::RegisterProfilePrefs(prefs_.registry()); prefs::RegisterProfilePrefs(prefs_.registry());
...@@ -61,9 +40,9 @@ class ArcDataRemoverTest : public testing::Test { ...@@ -61,9 +40,9 @@ class ArcDataRemoverTest : public testing::Test {
return cryptohome_id_; return cryptohome_id_;
} }
TestUpstartClient* upstart_client() { chromeos::FakeSessionManagerClient* session_manager_client() {
return static_cast<TestUpstartClient*>( return static_cast<chromeos::FakeSessionManagerClient*>(
chromeos::DBusThreadManager::Get()->GetUpstartClient()); chromeos::DBusThreadManager::Get()->GetSessionManagerClient());
} }
private: private:
...@@ -89,7 +68,7 @@ TEST_F(ArcDataRemoverTest, NotScheduled) { ...@@ -89,7 +68,7 @@ TEST_F(ArcDataRemoverTest, NotScheduled) {
} }
TEST_F(ArcDataRemoverTest, Success) { TEST_F(ArcDataRemoverTest, Success) {
upstart_client()->set_arc_available(true); session_manager_client()->set_arc_available(true);
ArcDataRemover data_remover(prefs(), cryptohome_id()); ArcDataRemover data_remover(prefs(), cryptohome_id());
data_remover.Schedule(); data_remover.Schedule();
......
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