diff --git a/atom/browser/lib/power_observer_linux.cc b/atom/browser/lib/power_observer_linux.cc index 7a24d5c3bda..c0dd0f32b07 100644 --- a/atom/browser/lib/power_observer_linux.cc +++ b/atom/browser/lib/power_observer_linux.cc @@ -33,21 +33,28 @@ namespace atom { PowerObserverLinux::PowerObserverLinux() : lock_owner_name_(get_executable_basename()), weak_ptr_factory_(this) { - auto* dbus_thread_manager = bluez::DBusThreadManagerLinux::Get(); - if (dbus_thread_manager) { - bus_ = dbus_thread_manager->GetSystemBus(); - if (bus_) { - logind_ = bus_->GetObjectProxy(kLogindServiceName, - dbus::ObjectPath(kLogindObjectPath)); - logind_->WaitForServiceToBeAvailable( - base::Bind(&PowerObserverLinux::OnLoginServiceAvailable, - weak_ptr_factory_.GetWeakPtr())); - } else { - LOG(WARNING) << "Failed to get system bus connection"; - } - } else { - LOG(WARNING) << "DBusThreadManagerLinux instance isn't available"; + auto* bus = bluez::DBusThreadManagerLinux::Get()->GetSystemBus(); + if (!bus) { + LOG(WARNING) << "Failed to get system bus connection"; + return; } + + // set up the logind proxy + + const auto weakThis = weak_ptr_factory_.GetWeakPtr(); + + logind_ = bus->GetObjectProxy(kLogindServiceName, + dbus::ObjectPath(kLogindObjectPath)); + logind_->ConnectToSignal( + kLogindManagerInterface, "PrepareForShutdown", + base::BindRepeating(&PowerObserverLinux::OnPrepareForShutdown, weakThis), + base::BindRepeating(&PowerObserverLinux::OnSignalConnected, weakThis)); + logind_->ConnectToSignal( + kLogindManagerInterface, "PrepareForSleep", + base::BindRepeating(&PowerObserverLinux::OnPrepareForSleep, weakThis), + base::BindRepeating(&PowerObserverLinux::OnSignalConnected, weakThis)); + logind_->WaitForServiceToBeAvailable(base::BindRepeating( + &PowerObserverLinux::OnLoginServiceAvailable, weakThis)); } PowerObserverLinux::~PowerObserverLinux() = default; @@ -57,17 +64,6 @@ void PowerObserverLinux::OnLoginServiceAvailable(bool service_available) { LOG(WARNING) << kLogindServiceName << " not available"; return; } - // Connect to PrepareForShutdown/PrepareForSleep signals - logind_->ConnectToSignal(kLogindManagerInterface, "PrepareForShutdown", - base::Bind(&PowerObserverLinux::OnPrepareForShutdown, - weak_ptr_factory_.GetWeakPtr()), - base::Bind(&PowerObserverLinux::OnSignalConnected, - weak_ptr_factory_.GetWeakPtr())); - logind_->ConnectToSignal(kLogindManagerInterface, "PrepareForSleep", - base::Bind(&PowerObserverLinux::OnPrepareForSleep, - weak_ptr_factory_.GetWeakPtr()), - base::Bind(&PowerObserverLinux::OnSignalConnected, - weak_ptr_factory_.GetWeakPtr())); // Take sleep inhibit lock BlockSleep(); } @@ -81,10 +77,10 @@ void PowerObserverLinux::BlockSleep() { inhibit_writer.AppendString(lock_owner_name_); // who inhibit_writer.AppendString("Application cleanup before suspend"); // why inhibit_writer.AppendString("delay"); // mode - logind_->CallMethod(&sleep_inhibit_call, - dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, - base::Bind(&PowerObserverLinux::OnInhibitResponse, - weak_ptr_factory_.GetWeakPtr(), &sleep_lock_)); + logind_->CallMethod( + &sleep_inhibit_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, + base::BindOnce(&PowerObserverLinux::OnInhibitResponse, + weak_ptr_factory_.GetWeakPtr(), &sleep_lock_)); } void PowerObserverLinux::UnblockSleep() { @@ -104,8 +100,8 @@ void PowerObserverLinux::BlockShutdown() { inhibit_writer.AppendString("delay"); // mode logind_->CallMethod( &shutdown_inhibit_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, - base::Bind(&PowerObserverLinux::OnInhibitResponse, - weak_ptr_factory_.GetWeakPtr(), &shutdown_lock_)); + base::BindOnce(&PowerObserverLinux::OnInhibitResponse, + weak_ptr_factory_.GetWeakPtr(), &shutdown_lock_)); } void PowerObserverLinux::UnblockShutdown() { @@ -123,8 +119,10 @@ void PowerObserverLinux::SetShutdownHandler(base::Callback handler) { void PowerObserverLinux::OnInhibitResponse(base::ScopedFD* scoped_fd, dbus::Response* response) { - dbus::MessageReader reader(response); - reader.PopFileDescriptor(scoped_fd); + if (response != nullptr) { + dbus::MessageReader reader(response); + reader.PopFileDescriptor(scoped_fd); + } } void PowerObserverLinux::OnPrepareForSleep(dbus::Signal* signal) { @@ -159,7 +157,7 @@ void PowerObserverLinux::OnPrepareForShutdown(dbus::Signal* signal) { } } -void PowerObserverLinux::OnSignalConnected(const std::string& interface, +void PowerObserverLinux::OnSignalConnected(const std::string& /*interface*/, const std::string& signal, bool success) { LOG_IF(WARNING, !success) << "Failed to connect to " << signal; diff --git a/atom/browser/lib/power_observer_linux.h b/atom/browser/lib/power_observer_linux.h index d7ca6557ce1..f3173e60015 100644 --- a/atom/browser/lib/power_observer_linux.h +++ b/atom/browser/lib/power_observer_linux.h @@ -40,7 +40,6 @@ class PowerObserverLinux : public base::PowerObserver { base::Callback should_shutdown_; - scoped_refptr bus_; scoped_refptr logind_; std::string lock_owner_name_; base::ScopedFD sleep_lock_; diff --git a/spec/api-power-monitor-spec.js b/spec/api-power-monitor-spec.js index 60401d5bcca..c3c5af69df3 100644 --- a/spec/api-power-monitor-spec.js +++ b/spec/api-power-monitor-spec.js @@ -16,8 +16,7 @@ chai.use(dirtyChai) const skip = process.platform !== 'linux' || !process.env.DBUS_SYSTEM_BUS_ADDRESS -// TODO(alexeykuzmin): [Ch66] Crashes on Linux ia32. Fix it and enable back. -xdescribe('powerMonitor', () => { +describe('powerMonitor', () => { let logindMock, dbusMockPowerMonitor, getCalls, emitSignal, reset if (!skip) { @@ -31,7 +30,9 @@ xdescribe('powerMonitor', () => { reset = Promise.promisify(logindMock.Reset, { context: logindMock }) }) - after(reset) + after(async () => { + await reset() + }) } (skip ? describe.skip : describe)('when powerMonitor module is loaded with dbus mock', () => { @@ -129,8 +130,12 @@ xdescribe('powerMonitor', () => { describe('powerMonitor.querySystemIdleState', () => { it('notify current system idle state', done => { + // this function is not mocked out, so we can test the result's + // form and type but not its value. powerMonitor.querySystemIdleState(1, idleState => { - expect(idleState).to.be.true() + expect(idleState).to.be.a('string') + const validIdleStates = [ 'active', 'idle', 'locked', 'unknown' ] + expect(validIdleStates).to.include(idleState) done() }) })