Commit 476e192f authored by Kush Sinha's avatar Kush Sinha Committed by Chromium LUCI CQ

Revert "GuestOsSharePath mark paths no longer shared on VM shutdown"

This reverts commit 56a15a3f.

Reason for revert: Fails unit tests on linux-chromeos-rel. See
https://crbug.com/1158303 for details.

Original change's description:
> GuestOsSharePath mark paths no longer shared on VM shutdown
>
> Bug: 1144138
> Change-Id: I970293c61c9686ab32f574957c028bd3db754ccd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2589049
> Auto-Submit: Joel Hockey <joelhockey@chromium.org>
> Commit-Queue: Jason Lin <lxj@google.com>
> Reviewed-by: Jason Lin <lxj@google.com>
> Cr-Commit-Position: refs/heads/master@{#836531}

TBR=joelhockey@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,lxj@google.com

Change-Id: I4ba6d44fa5b448a4d6899a69dd5848a103a5614c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1144138, 1158303
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2588930Reviewed-by: default avatarKush Sinha <sinhak@chromium.org>
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836575}
parent 6a3d70f3
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/optional.h" #include "base/optional.h"
#include "base/task/thread_pool.h" #include "base/task/thread_pool.h"
#include "chrome/browser/chromeos/arc/session/arc_session_manager.h" #include "chrome/browser/chromeos/arc/session/arc_session_manager.h"
#include "chrome/browser/chromeos/crostini/crostini_manager.h"
#include "chrome/browser/chromeos/crostini/crostini_util.h" #include "chrome/browser/chromeos/crostini/crostini_util.h"
#include "chrome/browser/chromeos/drive/drive_integration_service.h" #include "chrome/browser/chromeos/drive/drive_integration_service.h"
#include "chrome/browser/chromeos/file_manager/path_util.h" #include "chrome/browser/chromeos/file_manager/path_util.h"
...@@ -183,9 +184,6 @@ GuestOsSharePath::GuestOsSharePath(Profile* profile) ...@@ -183,9 +184,6 @@ GuestOsSharePath::GuestOsSharePath(Profile* profile)
file_watcher_task_runner_(base::ThreadPool::CreateSequencedTaskRunner( file_watcher_task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE})), {base::MayBlock(), base::TaskPriority::USER_VISIBLE})),
seneschal_callback_(base::BindRepeating(LogErrorResult)) { seneschal_callback_(base::BindRepeating(LogErrorResult)) {
auto* crostini_manager = crostini::CrostiniManager::GetForProfile(profile_);
crostini_manager->AddVmShutdownObserver(this);
if (auto* vmgr = file_manager::VolumeManager::Get(profile_)) { if (auto* vmgr = file_manager::VolumeManager::Get(profile_)) {
vmgr->AddObserver(this); vmgr->AddObserver(this);
} }
...@@ -204,9 +202,6 @@ GuestOsSharePath::~GuestOsSharePath() = default; ...@@ -204,9 +202,6 @@ GuestOsSharePath::~GuestOsSharePath() = default;
void GuestOsSharePath::Shutdown() { void GuestOsSharePath::Shutdown() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
auto* crostini_manager = crostini::CrostiniManager::GetForProfile(profile_);
crostini_manager->RemoveVmShutdownObserver(this);
for (auto& shared_path : shared_paths_) { for (auto& shared_path : shared_paths_) {
if (shared_path.second.watcher) { if (shared_path.second.watcher) {
file_watcher_task_runner_->DeleteSoon( file_watcher_task_runner_->DeleteSoon(
...@@ -485,7 +480,12 @@ void GuestOsSharePath::UnsharePath(const std::string& vm_name, ...@@ -485,7 +480,12 @@ void GuestOsSharePath::UnsharePath(const std::string& vm_name,
SuccessCallback callback) { SuccessCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (auto* info = FindSharedPathInfo(path)) { if (auto* info = FindSharedPathInfo(path)) {
if (RemoveSharedPathInfo(*info, vm_name)) { info->vm_names.erase(vm_name);
if (info->vm_names.empty()) {
if (info->watcher) {
file_watcher_task_runner_->DeleteSoon(FROM_HERE,
info->watcher.release());
}
shared_paths_.erase(path); shared_paths_.erase(path);
} }
} }
...@@ -589,17 +589,6 @@ bool GuestOsSharePath::IsPathShared(const std::string& vm_name, ...@@ -589,17 +589,6 @@ bool GuestOsSharePath::IsPathShared(const std::string& vm_name,
} }
} }
void GuestOsSharePath::OnVmShutdown(const std::string& vm_name) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
for (auto it = shared_paths_.begin(); it != shared_paths_.end();) {
if (RemoveSharedPathInfo(it->second, vm_name)) {
shared_paths_.erase(it++);
} else {
++it;
}
}
}
void GuestOsSharePath::OnVolumeMounted(chromeos::MountError error_code, void GuestOsSharePath::OnVolumeMounted(chromeos::MountError error_code,
const file_manager::Volume& volume) { const file_manager::Volume& volume) {
if (error_code != chromeos::MountError::MOUNT_ERROR_NONE) { if (error_code != chromeos::MountError::MOUNT_ERROR_NONE) {
...@@ -767,16 +756,4 @@ SharedPathInfo* GuestOsSharePath::FindSharedPathInfo( ...@@ -767,16 +756,4 @@ SharedPathInfo* GuestOsSharePath::FindSharedPathInfo(
return &it->second; return &it->second;
} }
bool GuestOsSharePath::RemoveSharedPathInfo(SharedPathInfo& info,
const std::string& vm_name) {
info.vm_names.erase(vm_name);
if (info.vm_names.empty()) {
if (info.watcher) {
file_watcher_task_runner_->DeleteSoon(FROM_HERE, info.watcher.release());
}
return true;
}
return false;
}
} // namespace guest_os } // namespace guest_os
...@@ -41,7 +41,6 @@ struct SharedPathInfo { ...@@ -41,7 +41,6 @@ struct SharedPathInfo {
// Handles sharing and unsharing paths from the Chrome OS host to guest VMs via // Handles sharing and unsharing paths from the Chrome OS host to guest VMs via
// seneschal. // seneschal.
class GuestOsSharePath : public KeyedService, class GuestOsSharePath : public KeyedService,
public chromeos::VmShutdownObserver,
public file_manager::VolumeManagerObserver, public file_manager::VolumeManagerObserver,
public drivefs::DriveFsHostObserver { public drivefs::DriveFsHostObserver {
public: public:
...@@ -114,9 +113,6 @@ class GuestOsSharePath : public KeyedService, ...@@ -114,9 +113,6 @@ class GuestOsSharePath : public KeyedService,
// Returns true if |path| or a parent is shared with |vm_name|. // Returns true if |path| or a parent is shared with |vm_name|.
bool IsPathShared(const std::string& vm_name, base::FilePath path) const; bool IsPathShared(const std::string& vm_name, base::FilePath path) const;
// chromeos::VmShutdownObserver
void OnVmShutdown(const std::string& vm_name) override;
// file_manager::VolumeManagerObserver // file_manager::VolumeManagerObserver
void OnVolumeMounted(chromeos::MountError error_code, void OnVolumeMounted(chromeos::MountError error_code,
const file_manager::Volume& volume) override; const file_manager::Volume& volume) override;
...@@ -159,10 +155,6 @@ class GuestOsSharePath : public KeyedService, ...@@ -159,10 +155,6 @@ class GuestOsSharePath : public KeyedService,
// Returns info for specified path or nullptr if not found. // Returns info for specified path or nullptr if not found.
SharedPathInfo* FindSharedPathInfo(const base::FilePath& path); SharedPathInfo* FindSharedPathInfo(const base::FilePath& path);
// Removes |vm_name| from |info.vm_names| if it exists, and deletes the
// |info.watcher| if |info.path| is not shared with any other VMs. Returns
// true if path is no longer shared with any VMs.
bool RemoveSharedPathInfo(SharedPathInfo& info, const std::string& vm_name);
Profile* profile_; Profile* profile_;
// Task runner for FilePathWatchers to be created, run, and be destroyed on. // Task runner for FilePathWatchers to be created, run, and be destroyed on.
......
...@@ -4,7 +4,6 @@ ...@@ -4,7 +4,6 @@
#include "chrome/browser/chromeos/guest_os/guest_os_share_path_factory.h" #include "chrome/browser/chromeos/guest_os/guest_os_share_path_factory.h"
#include "chrome/browser/chromeos/crostini/crostini_manager_factory.h"
#include "chrome/browser/chromeos/guest_os/guest_os_share_path.h" #include "chrome/browser/chromeos/guest_os/guest_os_share_path.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/keyed_service/content/browser_context_dependency_manager.h"
...@@ -26,9 +25,7 @@ GuestOsSharePathFactory* GuestOsSharePathFactory::GetInstance() { ...@@ -26,9 +25,7 @@ GuestOsSharePathFactory* GuestOsSharePathFactory::GetInstance() {
GuestOsSharePathFactory::GuestOsSharePathFactory() GuestOsSharePathFactory::GuestOsSharePathFactory()
: BrowserContextKeyedServiceFactory( : BrowserContextKeyedServiceFactory(
"GuestOsSharePath", "GuestOsSharePath",
BrowserContextDependencyManager::GetInstance()) { BrowserContextDependencyManager::GetInstance()) {}
DependsOn(crostini::CrostiniManagerFactory::GetInstance());
}
GuestOsSharePathFactory::~GuestOsSharePathFactory() = default; GuestOsSharePathFactory::~GuestOsSharePathFactory() = default;
......
...@@ -1044,14 +1044,6 @@ TEST_F(GuestOsSharePathTest, IsPathShared) { ...@@ -1044,14 +1044,6 @@ TEST_F(GuestOsSharePathTest, IsPathShared) {
shared_path_.Append("a/b"), shared_path_.DirName(), root_}) { shared_path_.Append("a/b"), shared_path_.DirName(), root_}) {
EXPECT_FALSE(guest_os_share_path_->IsPathShared("not-shared", path)); EXPECT_FALSE(guest_os_share_path_->IsPathShared("not-shared", path));
} }
// IsPathShared should be false after VM shutdown.
vm_tools::concierge::VmStoppedSignal signal;
signal.set_name(crostini::kCrostiniDefaultVmName);
signal.set_owner_id("test");
crostini::CrostiniManager::GetForProfile(profile())->OnVmStopped(signal);
EXPECT_FALSE(guest_os_share_path_->IsPathShared(
crostini::kCrostiniDefaultVmName, shared_path_));
} }
} // namespace guest_os } // namespace guest_os
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