Revert "Disable unmount-on-suspend for FUSE"
This reverts commit 436c119d042f7298482f189956d3bc0c7a3b336c.
Reason for revert: An edge case for platforms without ARC++ was not handled correctly.
Original change's description:
> Disable unmount-on-suspend for FUSE
>
> This removes the code for unmounting the FUSEs (filesystem in userspace)
> on suspend. Instead of unmounting to resolve the FUSE/freeze on suspend
> race condition, we will be using powerd code that orders freeze using
> freezer cgroups.
>
> Crostini still has some work left to do before this can be done for it,
> so unmount on suspend for its FUSE, sshfs, will be done in a later
> patch.
>
> Test: "suspend_stress_test -c 2500" on device
> Bug: b/134792837
> Change-Id: Icb8f9e309f6780e6f935be550c6c4e048a44b1fd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2336869
> Commit-Queue: Derek Basehore <[email protected]>
> Reviewed-by: Joel Hockey <[email protected]>
> Reviewed-by: Sergei Datsenko <[email protected]>
> Reviewed-by: James Cook <[email protected]>
> Reviewed-by: David Munro <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#811950}
[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
# Not skipping CQ checks because original CL landed > 1 day ago.
(cherry picked from commit d0312acf02335caec3670ea6f22244173da276e0)
Bug: b/134792837
Change-Id: Ia50e43dceeb0eea448d6fdf7232af328a9a1fd1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2451400
Reviewed-by: David Munro <[email protected]>
Reviewed-by: Sergei Datsenko <[email protected]>
Reviewed-by: Joel Hockey <[email protected]>
Reviewed-by: James Cook <[email protected]>
Commit-Queue: Derek Basehore <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#814910}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462232
Cr-Commit-Position: refs/branch-heads/4280@{#215}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/chrome/browser/chromeos/drive/drive_integration_service.cc b/chrome/browser/chromeos/drive/drive_integration_service.cc
index 089f77e..7c36d9d0 100644
--- a/chrome/browser/chromeos/drive/drive_integration_service.cc
+++ b/chrome/browser/chromeos/drive/drive_integration_service.cc
@@ -566,7 +566,8 @@
drivefs_holder_(std::make_unique<DriveFsHolder>(
profile_,
this,
- std::move(test_drivefs_mojo_listener_factory))) {
+ std::move(test_drivefs_mojo_listener_factory))),
+ power_manager_observer_(this) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(profile && !profile->IsOffTheRecord());
@@ -591,6 +592,10 @@
blocking_task_runner_.get()));
}
+ // PowerManagerClient is unset in unit tests.
+ if (chromeos::PowerManagerClient::Get()) {
+ power_manager_observer_.Add(chromeos::PowerManagerClient::Get());
+ }
SetEnabled(drive::util::IsDriveEnabledForProfile(profile));
}
@@ -1023,6 +1028,21 @@
profile_->GetPrefs()->SetBoolean(prefs::kDriveFsPinnedMigrated, true);
}
+void DriveIntegrationService::SuspendImminent(
+ power_manager::SuspendImminent::Reason reason) {
+ // This may a bit racy since it doesn't prevent suspend until the unmount is
+ // completed, instead relying on something else to defer suspending long
+ // enough.
+ RemoveDriveMountPoint();
+}
+
+void DriveIntegrationService::SuspendDone(
+ const base::TimeDelta& sleep_duration) {
+ if (is_enabled()) {
+ AddDriveMountPoint();
+ }
+}
+
void DriveIntegrationService::GetQuickAccessItems(
int max_number,
GetQuickAccessItemsCallback callback) {
diff --git a/chrome/browser/chromeos/drive/drive_integration_service.h b/chrome/browser/chromeos/drive/drive_integration_service.h
index e325a95..b7a826a 100644
--- a/chrome/browser/chromeos/drive/drive_integration_service.h
+++ b/chrome/browser/chromeos/drive/drive_integration_service.h
@@ -18,6 +18,7 @@
#include "base/observer_list_types.h"
#include "base/scoped_observer.h"
#include "chromeos/components/drivefs/drivefs_host.h"
+#include "chromeos/dbus/power/power_manager_client.h"
#include "components/drive/drive_notification_observer.h"
#include "components/drive/file_errors.h"
#include "components/drive/file_system_core_util.h"
@@ -90,7 +91,8 @@
// that are used to integrate Drive to Chrome. The object of this class is
// created per-profile.
class DriveIntegrationService : public KeyedService,
- public drivefs::DriveFsHost::MountObserver {
+ public drivefs::DriveFsHost::MountObserver,
+ public chromeos::PowerManagerClient::Observer {
public:
class PreferenceWatcher;
using DriveFsMojoListenerFactory = base::RepeatingCallback<
@@ -269,6 +271,10 @@
// Pin all the files in |files_to_pin| with DriveFS.
void PinFiles(const std::vector<base::FilePath>& files_to_pin);
+ // chromeos::PowerManagerClient::Observer overrides:
+ void SuspendImminent(power_manager::SuspendImminent::Reason reason) override;
+ void SuspendDone(const base::TimeDelta& sleep_duration) override;
+
void OnGetQuickAccessItems(
GetQuickAccessItemsCallback callback,
drive::FileError error,
@@ -300,6 +306,10 @@
base::TimeTicks mount_start_;
+ ScopedObserver<chromeos::PowerManagerClient,
+ chromeos::PowerManagerClient::Observer>
+ power_manager_observer_{this};
+
// Note: This should remain the last member so it'll be destroyed and
// invalidate its weak pointers before any other members are destroyed.
base::WeakPtrFactory<DriveIntegrationService> weak_ptr_factory_{this};
diff --git a/chrome/browser/chromeos/smb_client/smb_service.cc b/chrome/browser/chromeos/smb_client/smb_service.cc
index 32a8fa3..f86c9885a 100644
--- a/chrome/browser/chromeos/smb_client/smb_service.cc
+++ b/chrome/browser/chromeos/smb_client/smb_service.cc
@@ -182,6 +182,9 @@
SmbService::~SmbService() {
net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
+ if (chromeos::PowerManagerClient::Get()) {
+ chromeos::PowerManagerClient::Get()->RemoveObserver(this);
+ }
}
void SmbService::Shutdown() {
@@ -814,6 +817,9 @@
base::Unretained(this))));
RestoreMounts();
net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
+ if (chromeos::PowerManagerClient::Get()) {
+ chromeos::PowerManagerClient::Get()->AddObserver(this);
+ }
if (setup_complete_callback_) {
std::move(setup_complete_callback_).Run();
@@ -1017,5 +1023,53 @@
file_systems.size() + smbfs_shares_.size());
}
+void SmbService::SuspendImminent(
+ power_manager::SuspendImminent::Reason reason) {
+ for (auto it = smbfs_shares_.begin(); it != smbfs_shares_.end(); ++it) {
+ SmbFsShare* share = it->second.get();
+
+ // For each share, block suspend until the unmount has completed, to ensure
+ // that no smbfs instances are active when the system goes to sleep.
+ auto token = base::UnguessableToken::Create();
+ chromeos::PowerManagerClient::Get()->BlockSuspend(token, "SmbService");
+ share->Unmount(
+ base::BindOnce(&SmbService::OnSuspendUnmountDone, AsWeakPtr(), token));
+ }
+}
+
+void SmbService::OnSuspendUnmountDone(
+ base::UnguessableToken power_manager_suspend_token,
+ chromeos::MountError result) {
+ LOG_IF(ERROR, result != chromeos::MountError::MOUNT_ERROR_NONE)
+ << "Could not unmount smbfs share during suspension: "
+ << static_cast<int>(result);
+ // Regardless of the outcome, unblock suspension for this share.
+ chromeos::PowerManagerClient::Get()->UnblockSuspend(
+ power_manager_suspend_token);
+}
+
+void SmbService::SuspendDone(const base::TimeDelta& sleep_duration) {
+ for (auto it = smbfs_shares_.begin(); it != smbfs_shares_.end(); ++it) {
+ SmbFsShare* share = it->second.get();
+ const std::string mount_id = share->mount_id();
+
+ // Don't try to reconnect as we race the network stack in getting an IP
+ // address.
+ SmbFsShare::MountOptions options = share->options();
+ options.skip_connect = true;
+ // Observing power management changes from SmbService allows us to remove
+ // the share in OnSmbfsMountDone if remount fails.
+ share->Remount(
+ options, base::BindOnce(
+ &SmbService::OnSmbfsMountDone, AsWeakPtr(), mount_id,
+ base::BindOnce([](SmbMountResult result,
+ const base::FilePath& mount_path) {
+ LOG_IF(ERROR, result != SmbMountResult::kSuccess)
+ << "Error remounting smbfs share after suspension: "
+ << static_cast<int>(result);
+ })));
+ }
+}
+
} // namespace smb_client
} // namespace chromeos
diff --git a/chrome/browser/chromeos/smb_client/smb_service.h b/chrome/browser/chromeos/smb_client/smb_service.h
index 22c0fe62..acac449 100644
--- a/chrome/browser/chromeos/smb_client/smb_service.h
+++ b/chrome/browser/chromeos/smb_client/smb_service.h
@@ -25,6 +25,7 @@
#include "chrome/browser/chromeos/smb_client/smb_task_queue.h"
#include "chrome/browser/chromeos/smb_client/smbfs_share.h"
#include "chrome/browser/profiles/profile.h"
+#include "chromeos/dbus/power/power_manager_client.h"
#include "chromeos/dbus/smb_provider_client.h"
#include "components/keyed_service/core/keyed_service.h"
#include "net/base/network_change_notifier.h"
@@ -48,6 +49,7 @@
// Creates and manages an smb file system.
class SmbService : public KeyedService,
public net::NetworkChangeNotifier::NetworkChangeObserver,
+ public chromeos::PowerManagerClient::Observer,
public base::SupportsWeakPtr<SmbService> {
public:
using MountResponse = base::OnceCallback<void(SmbMountResult result)>;
@@ -124,6 +126,10 @@
void SetSmbFsMounterCreationCallbackForTesting(
SmbFsShare::MounterCreationCallback callback);
+ // chromeos::PowerManagerClient::Observer overrides
+ void SuspendImminent(power_manager::SuspendImminent::Reason reason) override;
+ void SuspendDone(const base::TimeDelta& sleep_duration) override;
+
private:
friend class SmbServiceTest;
@@ -164,6 +170,11 @@
MountInternalCallback callback,
SmbMountResult result);
+ // Callback passed to SmbFsShare::Unmount() during a power management
+ // suspension. Ensures that suspension is blocked until the unmount completes.
+ void OnSuspendUnmountDone(base::UnguessableToken power_manager_suspend_token,
+ chromeos::MountError result);
+
// Retrieves the mount_id for |file_system_info|.
int32_t GetMountId(const ProvidedFileSystemInfo& info) const;
