Skip to content

Commit d25e511

Browse files
trop[bot]codebytere
trop[bot]
authored and
codebytere
committed
fix: ensure document.visibilityState aligns with the visibility of the TopLevelWindow (electron#20134)
* fix: ensure document.visibilityState aligns with the visibility of the TopLevelWindow * chore: disable the specs on linux on CI
1 parent b4ba2f6 commit d25e511

File tree

8 files changed

+269
-4
lines changed

8 files changed

+269
-4
lines changed

‎docs/api/browser-window.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ This event is usually emitted after the `did-finish-load` event, but for
5151
pages with many remote resources, it may be emitted before the `did-finish-load`
5252
event.
5353

54+
Please note that using this event implies that the renderer will be considered "visible" and
55+
paint even though `show` is false. This event will never fire if you use `paintWhenInitiallyHidden: false`
56+
5457
## Setting `backgroundColor`
5558

5659
For a complex app, the `ready-to-show` event could be emitted too late, making
@@ -184,6 +187,7 @@ It creates a new `BrowserWindow` with native properties as set by the `options`.
184187
leave it undefined so the executable's icon will be used.
185188
* `show` Boolean (optional) - Whether window should be shown when created. Default is
186189
`true`.
190+
* `paintWhenInitiallyHidden` Boolean (optional) - Whether the renderer should be active when `show` is `false` and it has just been created. In order for `document.visibilityState` to work correctly on first load with `show: false` you should set this to `false`. Setting this to `false` will cause the `ready-to-show` event to not fire. Default is `true`.
187191
* `frame` Boolean (optional) - Specify `false` to create a
188192
[Frameless Window](frameless-window.md). Default is `true`.
189193
* `parent` BrowserWindow (optional) - Specify parent window. Default is `null`.
@@ -485,6 +489,9 @@ Emitted when the window is hidden.
485489
Emitted when the web page has been rendered (while not being shown) and window can be displayed without
486490
a visual flash.
487491

492+
Please note that using this event implies that the renderer will be considered "visible" and
493+
paint even though `show` is false. This event will never fire if you use `paintWhenInitiallyHidden: false`
494+
488495
#### Event: 'maximize'
489496

490497
Emitted when window is maximized.

‎shell/browser/api/atom_api_browser_window.cc‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "base/threading/thread_task_runner_handle.h"
1010
#include "content/browser/renderer_host/render_widget_host_impl.h" // nogncheck
1111
#include "content/browser/renderer_host/render_widget_host_owner_delegate.h" // nogncheck
12+
#include "content/browser/web_contents/web_contents_impl.h" // nogncheck
1213
#include "content/public/browser/render_process_host.h"
1314
#include "content/public/browser/render_view_host.h"
1415
#include "gin/converter.h"
@@ -44,10 +45,21 @@ BrowserWindow::BrowserWindow(v8::Isolate* isolate,
4445
if (options.Get(options::kBackgroundColor, &value))
4546
web_preferences.Set(options::kBackgroundColor, value);
4647

48+
// Copy the transparent setting to webContents
4749
v8::Local<v8::Value> transparent;
4850
if (options.Get("transparent", &transparent))
4951
web_preferences.Set("transparent", transparent);
5052

53+
// Copy the show setting to webContents, but only if we don't want to paint
54+
// when initially hidden
55+
bool paint_when_initially_hidden = true;
56+
options.Get("paintWhenInitiallyHidden", &paint_when_initially_hidden);
57+
if (!paint_when_initially_hidden) {
58+
bool show = true;
59+
options.Get(options::kShow, &show);
60+
web_preferences.Set(options::kShow, show);
61+
}
62+
5163
if (options.Get("webContents", &web_contents) && !web_contents.IsEmpty()) {
5264
// Set webPreferences from options if using an existing webContents.
5365
// These preferences will be used when the webContent launches new
@@ -422,6 +434,16 @@ void BrowserWindow::Cleanup() {
422434
Observe(nullptr);
423435
}
424436

437+
void BrowserWindow::OnWindowShow() {
438+
web_contents()->WasShown();
439+
TopLevelWindow::OnWindowShow();
440+
}
441+
442+
void BrowserWindow::OnWindowHide() {
443+
web_contents()->WasHidden();
444+
TopLevelWindow::OnWindowHide();
445+
}
446+
425447
// static
426448
mate::WrappableBase* BrowserWindow::New(mate::Arguments* args) {
427449
if (!Browser::Get()->is_ready()) {

‎shell/browser/api/atom_api_browser_window.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ class BrowserWindow : public TopLevelWindow,
7676
void RemoveBrowserView(v8::Local<v8::Value> value) override;
7777
void ResetBrowserViews() override;
7878
void SetVibrancy(v8::Isolate* isolate, v8::Local<v8::Value> value) override;
79+
void OnWindowShow() override;
80+
void OnWindowHide() override;
7981

8082
// BrowserWindow APIs.
8183
void FocusOnWebView();

‎shell/browser/api/atom_api_web_contents.cc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,9 @@ WebContents::WebContents(v8::Isolate* isolate, const mate::Dictionary& options)
364364
// Whether to enable DevTools.
365365
options.Get("devTools", &enable_devtools_);
366366

367+
bool initially_shown = true;
368+
options.Get(options::kShow, &initially_shown);
369+
367370
// Obtain the session.
368371
std::string partition;
369372
mate::Handle<api::Session> session;
@@ -418,6 +421,7 @@ WebContents::WebContents(v8::Isolate* isolate, const mate::Dictionary& options)
418421
#endif
419422
} else {
420423
content::WebContents::CreateParams params(session->browser_context());
424+
params.initially_hidden = !initially_shown;
421425
web_contents = content::WebContents::Create(params);
422426
}
423427

‎spec-main/events-helpers.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@ export const waitForEvent = (target: EventTarget, eventName: string) => {
1919
* @param {string} eventName
2020
* @return {!Promise<!Array>} With Event as the first item.
2121
*/
22-
export const emittedOnce = (emitter: NodeJS.EventEmitter, eventName: string) => {
23-
return emittedNTimes(emitter, eventName, 1).then(([result]) => result)
22+
export const emittedOnce = (emitter: NodeJS.EventEmitter, eventName: string, trigger?: () => void) => {
23+
return emittedNTimes(emitter, eventName, 1, trigger).then(([result]) => result)
2424
}
2525

26-
export const emittedNTimes = (emitter: NodeJS.EventEmitter, eventName: string, times: number) => {
26+
export const emittedNTimes = async (emitter: NodeJS.EventEmitter, eventName: string, times: number, trigger?: () => void) => {
2727
const events: any[][] = []
28-
return new Promise<any[][]>(resolve => {
28+
const p = new Promise<any[][]>(resolve => {
2929
const handler = (...args: any[]) => {
3030
events.push(args)
3131
if (events.length === times) {
@@ -35,4 +35,8 @@ export const emittedNTimes = (emitter: NodeJS.EventEmitter, eventName: string, t
3535
}
3636
emitter.on(eventName, handler)
3737
})
38+
if (trigger) {
39+
await Promise.resolve(trigger())
40+
}
41+
return p
3842
}

‎spec-main/fixtures/chromium/other-window.js‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
const { app, BrowserWindow } = require('electron')
2+
3+
const ints = (...args) => args.map(a => parseInt(a, 10))
4+
5+
const [x, y, width, height] = ints(...process.argv.slice(2))
6+
7+
let w
8+
9+
app.whenReady().then(() => {
10+
w = new BrowserWindow({
11+
x,
12+
y,
13+
width,
14+
height
15+
})
16+
console.log('__ready__')
17+
})
18+
19+
process.on('SIGTERM', () => {
20+
process.exit(0)
21+
})

‎spec-main/fixtures/chromium/visibilitystate.html‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="UTF-8">
5+
<meta name="viewport" content="width=device-width, initial-scale=1.0">
6+
<meta http-equiv="X-UA-Compatible" content="ie=edge">
7+
<title>Document</title>
8+
</head>
9+
<body>
10+
<script>
11+
require('electron').ipcRenderer.on('get-visibility-state', (event) => {
12+
event.sender.send('visibility-state', document.visibilityState)
13+
})
14+
document.addEventListener('visibilitychange', () => {
15+
require('electron').ipcRenderer.send('visibility-change')
16+
})
17+
</script>
18+
</body>
19+
</html>

‎spec-main/visibility-state-spec.ts‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
import { expect } from 'chai'
2+
import * as cp from 'child_process';
3+
import { BrowserWindow, BrowserWindowConstructorOptions, ipcMain } from 'electron'
4+
import * as path from 'path'
5+
6+
import { emittedOnce } from './events-helpers'
7+
import { closeWindow } from './window-helpers';
8+
import { ifdescribe } from './spec-helpers';
9+
10+
// visibilityState specs pass on linux with a real window manager but on CI
11+
// the environment does not let these specs pass
12+
ifdescribe(process.platform !== 'linux' || !isCI)('document.visibilityState', () => {
13+
let w: BrowserWindow
14+
15+
afterEach(() => {
16+
return closeWindow(w)
17+
})
18+
19+
const load = () => w.loadFile(path.resolve(__dirname, 'fixtures', 'chromium', 'visibilitystate.html'))
20+
21+
const itWithOptions = (name: string, options: BrowserWindowConstructorOptions, fn: Mocha.Func) => {
22+
return it(name, async function (...args) {
23+
w = new BrowserWindow({
24+
...options,
25+
paintWhenInitiallyHidden: false,
26+
webPreferences: {
27+
...(options.webPreferences || {}),
28+
nodeIntegration: true
29+
}
30+
})
31+
await Promise.resolve(fn.apply(this, args))
32+
})
33+
}
34+
35+
const getVisibilityState = async (): Promise<string> => {
36+
const response = emittedOnce(ipcMain, 'visibility-state')
37+
w.webContents.send('get-visibility-state')
38+
return (await response)[1]
39+
}
40+
41+
itWithOptions('should be visible when the window is initially shown by default', {}, async () => {
42+
await load()
43+
const state = await getVisibilityState()
44+
expect(state).to.equal('visible')
45+
})
46+
47+
itWithOptions('should be visible when the window is initially shown', {
48+
show: true,
49+
}, async () => {
50+
await load()
51+
const state = await getVisibilityState()
52+
expect(state).to.equal('visible')
53+
})
54+
55+
itWithOptions('should be hidden when the window is initially hidden', {
56+
show: false,
57+
}, async () => {
58+
await load()
59+
const state = await getVisibilityState()
60+
expect(state).to.equal('hidden')
61+
})
62+
63+
itWithOptions('should be visible when the window is initially hidden but shown before the page is loaded', {
64+
show: false,
65+
}, async () => {
66+
w.show()
67+
await load()
68+
const state = await getVisibilityState()
69+
expect(state).to.equal('visible')
70+
})
71+
72+
itWithOptions('should be hidden when the window is initially shown but hidden before the page is loaded', {
73+
show: true,
74+
}, async () => {
75+
// TODO(MarshallOfSound): Figure out if we can work around this 1 tick issue for users
76+
if (process.platform === 'darwin') {
77+
// Wait for a tick, the window being "shown" takes 1 tick on macOS
78+
await new Promise(r => setTimeout(r, 0))
79+
}
80+
w.hide()
81+
await load()
82+
const state = await getVisibilityState()
83+
expect(state).to.equal('hidden')
84+
})
85+
86+
itWithOptions('should be toggle between visible and hidden as the window is hidden and shown', {}, async () => {
87+
await load()
88+
expect(await getVisibilityState()).to.equal('visible')
89+
await emittedOnce(ipcMain, 'visibility-change', () => w.hide())
90+
expect(await getVisibilityState()).to.equal('hidden')
91+
await emittedOnce(ipcMain, 'visibility-change', () => w.show())
92+
expect(await getVisibilityState()).to.equal('visible')
93+
})
94+
95+
itWithOptions('should become hidden when a window is minimized', {}, async () => {
96+
await load()
97+
expect(await getVisibilityState()).to.equal('visible')
98+
await emittedOnce(ipcMain, 'visibility-change', () => w.minimize())
99+
expect(await getVisibilityState()).to.equal('hidden')
100+
})
101+
102+
itWithOptions('should become visible when a window is restored', {}, async () => {
103+
await load()
104+
expect(await getVisibilityState()).to.equal('visible')
105+
await emittedOnce(ipcMain, 'visibility-change', () => w.minimize())
106+
await emittedOnce(ipcMain, 'visibility-change', () => w.restore())
107+
expect(await getVisibilityState()).to.equal('visible')
108+
})
109+
110+
describe('on platforms that support occlusion detection', () => {
111+
let child: cp.ChildProcess
112+
113+
before(function() {
114+
if (process.platform !== 'darwin') this.skip()
115+
})
116+
117+
const makeOtherWindow = (opts: { x: number; y: number; width: number; height: number; }) => {
118+
child = cp.spawn(process.execPath, [path.resolve(__dirname, 'fixtures', 'chromium', 'other-window.js'), `${opts.x}`, `${opts.y}`, `${opts.width}`, `${opts.height}`])
119+
return new Promise(resolve => {
120+
child.stdout!.on('data', (chunk) => {
121+
if (chunk.toString().includes('__ready__')) resolve()
122+
})
123+
})
124+
}
125+
126+
afterEach(() => {
127+
if (child && !child.killed) {
128+
child.kill('SIGTERM')
129+
}
130+
})
131+
132+
itWithOptions('should be visible when two windows are on screen', {
133+
x: 0,
134+
y: 0,
135+
width: 200,
136+
height: 200,
137+
}, async () => {
138+
await makeOtherWindow({
139+
x: 200,
140+
y: 0,
141+
width: 200,
142+
height: 200,
143+
})
144+
await load()
145+
const state = await getVisibilityState()
146+
expect(state).to.equal('visible')
147+
})
148+
149+
itWithOptions('should be visible when two windows are on screen that overlap partially', {
150+
x: 50,
151+
y: 50,
152+
width: 150,
153+
height: 150,
154+
}, async () => {
155+
await makeOtherWindow({
156+
x: 100,
157+
y: 0,
158+
width: 200,
159+
height: 200,
160+
})
161+
await load()
162+
const state = await getVisibilityState()
163+
expect(state).to.equal('visible')
164+
})
165+
166+
itWithOptions('should be hidden when a second window completely conceals the current window', {
167+
x: 50,
168+
y: 50,
169+
width: 50,
170+
height: 50,
171+
}, async function () {
172+
this.timeout(240000)
173+
await load()
174+
await emittedOnce(ipcMain, 'visibility-change', async () => {
175+
await makeOtherWindow({
176+
x: 0,
177+
y: 0,
178+
width: 300,
179+
height: 300,
180+
})
181+
})
182+
const state = await getVisibilityState()
183+
expect(state).to.equal('hidden')
184+
})
185+
})
186+
})

0 commit comments

Comments
 (0)