New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(powerMonitor): expose interface to query system idle state #11807
Changes from all commits
b6a4519
64109da
0180d73
82cf1d1
1f3b731
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
#include "atom/browser/lib/power_observer.h" | ||
#include "base/compiler_specific.h" | ||
#include "native_mate/handle.h" | ||
#include "ui/base/idle/idle.h" | ||
|
||
namespace atom { | ||
|
||
|
@@ -41,6 +42,11 @@ class PowerMonitor : public mate::TrackableObject<PowerMonitor>, | |
void OnResume() override; | ||
|
||
private: | ||
void QuerySystemIdleState(v8::Isolate* isolate, | ||
int idle_threshold, | ||
const ui::IdleCallback& callback); | ||
void QuerySystemIdleTime(const ui::IdleTimeCallback& callback); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's make these methods private since we're only using them with the bindings. Permissions can be loosened later as needed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense, moved it. |
||
DISALLOW_COPY_AND_ASSIGN(PowerMonitor); | ||
}; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,26 +10,28 @@ const assert = require('assert') | |
const dbus = require('dbus-native') | ||
const Promise = require('bluebird') | ||
|
||
const skip = process.platform !== 'linux' || !process.env.DBUS_SYSTEM_BUS_ADDRESS; | ||
|
||
(skip ? describe.skip : describe)('powerMonitor', () => { | ||
let logindMock, powerMonitor, getCalls, emitSignal, reset | ||
|
||
before(async () => { | ||
const systemBus = dbus.systemBus() | ||
const loginService = systemBus.getService('org.freedesktop.login1') | ||
const getInterface = Promise.promisify(loginService.getInterface, {context: loginService}) | ||
logindMock = await getInterface('/org/freedesktop/login1', 'org.freedesktop.DBus.Mock') | ||
getCalls = Promise.promisify(logindMock.GetCalls, {context: logindMock}) | ||
emitSignal = Promise.promisify(logindMock.EmitSignal, {context: logindMock}) | ||
reset = Promise.promisify(logindMock.Reset, {context: logindMock}) | ||
}) | ||
const skip = process.platform !== 'linux' || !process.env.DBUS_SYSTEM_BUS_ADDRESS | ||
|
||
after(async () => { | ||
await reset() | ||
}) | ||
describe('powerMonitor', () => { | ||
let logindMock, dbusMockPowerMonitor, getCalls, emitSignal, reset | ||
|
||
describe('when powerMonitor module is loaded', () => { | ||
if (!skip) { | ||
before(async () => { | ||
const systemBus = dbus.systemBus() | ||
const loginService = systemBus.getService('org.freedesktop.login1') | ||
const getInterface = Promise.promisify(loginService.getInterface, {context: loginService}) | ||
logindMock = await getInterface('/org/freedesktop/login1', 'org.freedesktop.DBus.Mock') | ||
getCalls = Promise.promisify(logindMock.GetCalls, {context: logindMock}) | ||
emitSignal = Promise.promisify(logindMock.EmitSignal, {context: logindMock}) | ||
reset = Promise.promisify(logindMock.Reset, {context: logindMock}) | ||
}) | ||
|
||
after(async () => { | ||
await reset() | ||
}) | ||
} | ||
|
||
(skip ? describe.skip : describe)('when powerMonitor module is loaded with dbus mock', () => { | ||
function onceMethodCalled (done) { | ||
function cb () { | ||
logindMock.removeListener('MethodCalled', cb) | ||
|
@@ -41,7 +43,7 @@ const skip = process.platform !== 'linux' || !process.env.DBUS_SYSTEM_BUS_ADDRES | |
before((done) => { | ||
logindMock.on('MethodCalled', onceMethodCalled(done)) | ||
// lazy load powerMonitor after we listen to MethodCalled mock signal | ||
powerMonitor = require('electron').remote.powerMonitor | ||
dbusMockPowerMonitor = require('electron').remote.powerMonitor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tarruda Is it required to lazy initialize the module ? I was hoping we can just initialize the module only once for all the specs here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It has to be lazy loaded if we want to verify that electron inhibits the system suspend (it does this by calling a method via dbus). |
||
}) | ||
|
||
it('should call Inhibit to delay suspend', async () => { | ||
|
@@ -59,14 +61,14 @@ const skip = process.platform !== 'linux' || !process.env.DBUS_SYSTEM_BUS_ADDRES | |
|
||
describe('when PrepareForSleep(true) signal is sent by logind', () => { | ||
it('should emit "suspend" event', (done) => { | ||
powerMonitor.once('suspend', () => done()) | ||
dbusMockPowerMonitor.once('suspend', () => done()) | ||
emitSignal('org.freedesktop.login1.Manager', 'PrepareForSleep', | ||
'b', [['b', true]]) | ||
}) | ||
|
||
describe('when PrepareForSleep(false) signal is sent by logind', () => { | ||
it('should emit "resume" event', (done) => { | ||
powerMonitor.once('resume', () => done()) | ||
dbusMockPowerMonitor.once('resume', () => done()) | ||
emitSignal('org.freedesktop.login1.Manager', 'PrepareForSleep', | ||
'b', [['b', false]]) | ||
}) | ||
|
@@ -90,7 +92,7 @@ const skip = process.platform !== 'linux' || !process.env.DBUS_SYSTEM_BUS_ADDRES | |
before(async () => { | ||
const calls = await getCalls() | ||
assert.equal(calls.length, 2) | ||
powerMonitor.once('shutdown', () => { }) | ||
dbusMockPowerMonitor.once('shutdown', () => { }) | ||
}) | ||
|
||
it('should call Inhibit to delay shutdown', async () => { | ||
|
@@ -108,11 +110,53 @@ const skip = process.platform !== 'linux' || !process.env.DBUS_SYSTEM_BUS_ADDRES | |
|
||
describe('when PrepareForShutdown(true) signal is sent by logind', () => { | ||
it('should emit "shutdown" event', (done) => { | ||
powerMonitor.once('shutdown', () => { done() }) | ||
dbusMockPowerMonitor.once('shutdown', () => { done() }) | ||
emitSignal('org.freedesktop.login1.Manager', 'PrepareForShutdown', | ||
'b', [['b', true]]) | ||
}) | ||
}) | ||
}) | ||
}) | ||
|
||
describe('when powerMonitor module is loaded', () => { | ||
let powerMonitor | ||
before(() => { | ||
powerMonitor = require('electron').remote.powerMonitor | ||
}) | ||
|
||
describe('powerMonitor.querySystemIdleState', () => { | ||
it('notify current system idle state', (done) => { | ||
powerMonitor.querySystemIdleState(1, (idleState) => { | ||
assert.ok(idleState) | ||
done() | ||
}) | ||
}) | ||
|
||
it('does not accept non positive integer threshold', () => { | ||
assert.throws(() => { | ||
powerMonitor.querySystemIdleState(-1, (idleState) => { | ||
}) | ||
}) | ||
|
||
assert.throws(() => { | ||
powerMonitor.querySystemIdleState(NaN, (idleState) => { | ||
}) | ||
}) | ||
|
||
assert.throws(() => { | ||
powerMonitor.querySystemIdleState('a', (idleState) => { | ||
}) | ||
}) | ||
}) | ||
}) | ||
|
||
describe('powerMonitor.querySystemIdleTime', () => { | ||
it('notify current system idle time', (done) => { | ||
powerMonitor.querySystemIdleTime((idleTime) => { | ||
assert.ok(idleTime >= 0) | ||
done() | ||
}) | ||
}) | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know another reviewer suggested this, but are we sure that we want this?
Since we're not publishing the
ui::CheckIdleStateIsLocked()
, if all you want to do is check for system lock it's reasonable to pass a-1
in foridle_threshold
.If we include that method, we can avoid this wart by having a recommended way to test for lock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I didn't thought in that way, but also checking
lock
state is somewhat limited per platform support, so passing-1
doesn't guarantee you can reliable acquire intended state. I think it's rather limit api surface, and exposeCheckIdleStateIsLocked
separately if there's demand for it.