Skip to content

Commit c438eb0

Browse files
author
Ivan Sandrk
committed
[Merge to M59] Break circular dependency between InitializeDeviceDisablingManager and DeviceDisabledScreen
crrev.com/2714493002 uncovered a circular dependency between BrowserProcessPlatformPart::InitializeDeviceDisablingManager and DeviceDisabledScreen::DeviceDisabledScreen. Previously there was a part in the call stack that forked off into an asynchronous call, DeviceDisablingManager::UpdateFromCrosSettings, which would queue up a callback in case the CrosSettings wasn't ready yet (and it wasn't). The referenced CL fixed a bug by immediately loading the settings, but it unfortunately broke the assumption here. The suggested fix breaks the dependency by moving Init() outside of the constructor (so the object exists at the time further code called from Init() tries to access it). This bug was causing a crash for disabled devices (ui wouldn't start). The circle starts on the "device_disabling_manager_.reset" line. TEST=manual BUG=709518 Review-Url: https://codereview.chromium.org/2815893002 Cr-Commit-Position: refs/heads/master@{#464708} (cherry picked from commit 87c761f) Review-Url: https://codereview.chromium.org/2832673002 . Cr-Commit-Position: refs/branch-heads/3071@{#68} Cr-Branched-From: a106f0a-refs/heads/master@{#464641}
1 parent 254bd12 commit c438eb0

File tree

5 files changed

+6
-6
lines changed

5 files changed

+6
-6
lines changed

chrome/browser/browser_process_platform_part_chromeos.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ void BrowserProcessPlatformPart::InitializeDeviceDisablingManager() {
6666
device_disabling_manager_delegate_.get(),
6767
chromeos::CrosSettings::Get(),
6868
user_manager::UserManager::Get()));
69+
device_disabling_manager_->Init();
6970
}
7071

7172
void BrowserProcessPlatformPart::ShutdownDeviceDisablingManager() {

chrome/browser/chromeos/login/screens/device_disabled_screen.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ DeviceDisabledScreen::DeviceDisabledScreen(
2121
device_disabling_manager_(
2222
g_browser_process->platform_part()->device_disabling_manager()),
2323
showing_(false) {
24-
DCHECK(view_);
25-
if (view_)
26-
view_->SetDelegate(this);
24+
view_->SetDelegate(this);
2725
device_disabling_manager_->AddObserver(this);
2826
}
2927

chrome/browser/chromeos/system/device_disabling_manager.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ DeviceDisablingManager::DeviceDisablingManager(
4343
device_disabled_(false),
4444
weak_factory_(this) {
4545
CHECK(delegate_);
46-
Init();
4746
}
4847

4948
DeviceDisablingManager::~DeviceDisablingManager() {

chrome/browser/chromeos/system/device_disabling_manager.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ class DeviceDisablingManager {
8585
user_manager::UserManager* user_manager);
8686
~DeviceDisablingManager();
8787

88+
// Must be called after construction.
89+
void Init();
90+
8891
void AddObserver(Observer* observer);
8992
void RemoveObserver(Observer* observer);
9093

@@ -107,8 +110,6 @@ class DeviceDisablingManager {
107110
static bool HonorDeviceDisablingDuringNormalOperation();
108111

109112
private:
110-
void Init();
111-
112113
// Cache the disabled message and inform observers if it changed.
113114
void CacheDisabledMessageAndNotify(const std::string& disabled_message);
114115

chrome/browser/chromeos/system/device_disabling_manager_unittest.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ void DeviceDisablingManagerTestBase::CreateDeviceDisablingManager() {
9595
this,
9696
CrosSettings::Get(),
9797
&fake_user_manager_));
98+
device_disabling_manager_->Init();
9899
}
99100

100101
void DeviceDisablingManagerTestBase::DestroyDeviceDisablingManager() {

0 commit comments

Comments
 (0)