Commit d9251a5f authored by Kyle Horimoto's avatar Kyle Horimoto Committed by Commit Bot

[CrOS MultiDevice] Add shutdown flow for DeviceSync service.

Previously, the DeviceSync service would retain references to its
dependencies until it was deleted when the browser process was killed.

When attempting to enable the MultiDevice flags by default, this
causes crashes in some tests due to the service not removing itself as
an observer. This CL adds an explicit shutdown step which removes these
references and causes the observer to be removed.

Sample failure:
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8933085811650027344/+/steps/sync_integration_tests__with_patch_/0/logs/USS__x2f_SingleClientWalletSyncTest.DownloadProfileStorage__x2f_0/0

Bug: 884066
Change-Id: I87bcc9ab27f0a1032bc6990a71f863786ddf97fc
Reviewed-on: https://chromium-review.googlesource.com/c/1274886
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarJeremy Klein <jlklein@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598789}
parent 08ec72e1
......@@ -4,13 +4,17 @@
#include <utility>
#include "base/bind.h"
#include "chromeos/services/device_sync/device_sync_base.h"
namespace chromeos {
namespace device_sync {
DeviceSyncBase::DeviceSyncBase() = default;
DeviceSyncBase::DeviceSyncBase() {
bindings_.set_connection_error_handler(base::BindRepeating(
&DeviceSyncBase::OnDisconnection, base::Unretained(this)));
}
DeviceSyncBase::~DeviceSyncBase() = default;
......@@ -33,6 +37,12 @@ void DeviceSyncBase::NotifyOnNewDevicesSynced() {
observers_.ForAllPtrs([](auto* observer) { observer->OnNewDevicesSynced(); });
}
void DeviceSyncBase::OnDisconnection() {
// If all clients have disconnected, shut down.
if (bindings_.empty())
Shutdown();
}
} // namespace device_sync
} // namespace chromeos
......@@ -31,10 +31,16 @@ class DeviceSyncBase : public mojom::DeviceSync {
protected:
DeviceSyncBase();
// Derived types should override this function to remove references to any
// dependencies.
virtual void Shutdown() {}
void NotifyOnEnrollmentFinished();
void NotifyOnNewDevicesSynced();
private:
void OnDisconnection();
mojo::InterfacePtrSet<mojom::DeviceSyncObserver> observers_;
mojo::BindingSet<mojom::DeviceSync> bindings_;
......@@ -45,4 +51,4 @@ class DeviceSyncBase : public mojom::DeviceSync {
} // namespace chromeos
#endif // CHROMEOS_SERVICES_DEVICE_SYNC_DEVICE_SYNC_BASE_H_
\ No newline at end of file
#endif // CHROMEOS_SERVICES_DEVICE_SYNC_DEVICE_SYNC_BASE_H_
......@@ -262,6 +262,23 @@ void DeviceSyncImpl::OnSyncDeviceListChanged() {
NotifyOnNewDevicesSynced();
}
void DeviceSyncImpl::Shutdown() {
software_feature_manager_.reset();
remote_device_provider_.reset();
cryptauth_device_manager_.reset();
cryptauth_enrollment_manager_.reset();
cryptauth_client_factory_.reset();
cryptauth_gcm_manager_.reset();
pref_connection_delegate_.reset();
identity_manager_ = nullptr;
gcm_driver_ = nullptr;
connector_ = nullptr;
gcm_device_info_provider_ = nullptr;
url_loader_factory_ = nullptr;
clock_ = nullptr;
}
void DeviceSyncImpl::ProcessPrimaryAccountInfo(
const AccountInfo& primary_account_info) {
// Note: We cannot use |primary_account_info.IsValid()| here because
......
......@@ -141,6 +141,9 @@ class DeviceSyncImpl : public DeviceSyncBase,
base::Clock* clock,
std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate);
// DeviceSyncBase:
void Shutdown() override;
void ProcessPrimaryAccountInfo(const AccountInfo& primary_account_info);
void ConnectToPrefStore();
void OnConnectedToPrefService(std::unique_ptr<PrefService> pref_service);
......
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