Skip to content
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

fix: ensure document.visibilityState aligns with the visibility of the TopLevelWindow #19988

Merged
merged 2 commits into from
Sep 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/api/browser-window.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ This event is usually emitted after the `did-finish-load` event, but for
pages with many remote resources, it may be emitted before the `did-finish-load`
event.

Please note that using this event implies that the renderer will be considered "visible" and
paint even though `show` is false. This event will never fire if you use `paintWhenInitiallyHidden: false`

## Setting `backgroundColor`

For a complex app, the `ready-to-show` event could be emitted too late, making
Expand Down Expand Up @@ -184,6 +187,7 @@ It creates a new `BrowserWindow` with native properties as set by the `options`.
leave it undefined so the executable's icon will be used.
* `show` Boolean (optional) - Whether window should be shown when created. Default is
`true`.
* `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`.
* `frame` Boolean (optional) - Specify `false` to create a
[Frameless Window](frameless-window.md). Default is `true`.
* `parent` BrowserWindow (optional) - Specify parent window. Default is `null`.
Expand Down Expand Up @@ -488,6 +492,9 @@ Emitted when the window is hidden.
Emitted when the web page has been rendered (while not being shown) and window can be displayed without
a visual flash.

Please note that using this event implies that the renderer will be considered "visible" and
paint even though `show` is false. This event will never fire if you use `paintWhenInitiallyHidden: false`

#### Event: 'maximize'

Emitted when window is maximized.
Expand Down
22 changes: 22 additions & 0 deletions shell/browser/api/atom_api_browser_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "base/threading/thread_task_runner_handle.h"
#include "content/browser/renderer_host/render_widget_host_impl.h" // nogncheck
#include "content/browser/renderer_host/render_widget_host_owner_delegate.h" // nogncheck
#include "content/browser/web_contents/web_contents_impl.h" // nogncheck
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h"
#include "gin/converter.h"
Expand Down Expand Up @@ -44,10 +45,21 @@ BrowserWindow::BrowserWindow(v8::Isolate* isolate,
if (options.Get(options::kBackgroundColor, &value))
web_preferences.Set(options::kBackgroundColor, value);

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

// Copy the show setting to webContents, but only if we don't want to paint
// when initially hidden
bool paint_when_initially_hidden = true;
options.Get("paintWhenInitiallyHidden", &paint_when_initially_hidden);
if (!paint_when_initially_hidden) {
bool show = true;
options.Get(options::kShow, &show);
web_preferences.Set(options::kShow, show);
}

if (options.Get("webContents", &web_contents) && !web_contents.IsEmpty()) {
// Set webPreferences from options if using an existing webContents.
// These preferences will be used when the webContent launches new
Expand Down Expand Up @@ -422,6 +434,16 @@ void BrowserWindow::Cleanup() {
Observe(nullptr);
}

void BrowserWindow::OnWindowShow() {
web_contents()->WasShown();
TopLevelWindow::OnWindowShow();
}

void BrowserWindow::OnWindowHide() {
web_contents()->WasHidden();
TopLevelWindow::OnWindowHide();
}

// static
mate::WrappableBase* BrowserWindow::New(mate::Arguments* args) {
if (!Browser::Get()->is_ready()) {
Expand Down
2 changes: 2 additions & 0 deletions shell/browser/api/atom_api_browser_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ class BrowserWindow : public TopLevelWindow,
void RemoveBrowserView(v8::Local<v8::Value> value) override;
void ResetBrowserViews() override;
void SetVibrancy(v8::Isolate* isolate, v8::Local<v8::Value> value) override;
void OnWindowShow() override;
void OnWindowHide() override;

// BrowserWindow APIs.
void FocusOnWebView();
Expand Down
4 changes: 4 additions & 0 deletions shell/browser/api/atom_api_web_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,9 @@ WebContents::WebContents(v8::Isolate* isolate, const mate::Dictionary& options)
// Whether to enable DevTools.
options.Get("devTools", &enable_devtools_);

bool initially_shown = true;
options.Get(options::kShow, &initially_shown);

// Obtain the session.
std::string partition;
mate::Handle<api::Session> session;
Expand Down Expand Up @@ -420,6 +423,7 @@ WebContents::WebContents(v8::Isolate* isolate, const mate::Dictionary& options)
#endif
} else {
content::WebContents::CreateParams params(session->browser_context());
params.initially_hidden = !initially_shown;
web_contents = content::WebContents::Create(params);
}

Expand Down
12 changes: 8 additions & 4 deletions spec-main/events-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ export const waitForEvent = (target: EventTarget, eventName: string) => {
* @param {string} eventName
* @return {!Promise<!Array>} With Event as the first item.
*/
export const emittedOnce = (emitter: NodeJS.EventEmitter, eventName: string) => {
return emittedNTimes(emitter, eventName, 1).then(([result]) => result)
export const emittedOnce = (emitter: NodeJS.EventEmitter, eventName: string, trigger?: () => void) => {
return emittedNTimes(emitter, eventName, 1, trigger).then(([result]) => result)
}

export const emittedNTimes = (emitter: NodeJS.EventEmitter, eventName: string, times: number) => {
export const emittedNTimes = async (emitter: NodeJS.EventEmitter, eventName: string, times: number, trigger?: () => void) => {
const events: any[][] = []
return new Promise<any[][]>(resolve => {
const p = new Promise<any[][]>(resolve => {
const handler = (...args: any[]) => {
events.push(args)
if (events.length === times) {
Expand All @@ -35,4 +35,8 @@ export const emittedNTimes = (emitter: NodeJS.EventEmitter, eventName: string, t
}
emitter.on(eventName, handler)
})
if (trigger) {
await Promise.resolve(trigger())
}
return p
}
21 changes: 21 additions & 0 deletions spec-main/fixtures/chromium/other-window.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
const { app, BrowserWindow } = require('electron')

const ints = (...args) => args.map(a => parseInt(a, 10))

const [x, y, width, height] = ints(...process.argv.slice(2))

let w

app.whenReady().then(() => {
w = new BrowserWindow({
x,
y,
width,
height
})
console.log('__ready__')
})

process.on('SIGTERM', () => {
process.exit(0)
})
19 changes: 19 additions & 0 deletions spec-main/fixtures/chromium/visibilitystate.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<meta http-equiv="X-UA-Compatible" content="ie=edge">
<title>Document</title>
</head>
<body>
<script>
require('electron').ipcRenderer.on('get-visibility-state', (event) => {
event.sender.send('visibility-state', document.visibilityState)
})
document.addEventListener('visibilitychange', () => {
require('electron').ipcRenderer.send('visibility-change')
})
</script>
</body>
</html>
186 changes: 186 additions & 0 deletions spec-main/visibility-state-spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
import { expect } from 'chai'
import * as cp from 'child_process';
import { BrowserWindow, BrowserWindowConstructorOptions, ipcMain } from 'electron'
import * as path from 'path'

import { emittedOnce } from './events-helpers'
import { closeWindow } from './window-helpers';
import { ifdescribe } from './spec-helpers';

// visibilityState specs pass on linux with a real window manager but on CI
// the environment does not let these specs pass
ifdescribe(process.platform !== 'linux' || !isCI)('document.visibilityState', () => {
let w: BrowserWindow

afterEach(() => {
return closeWindow(w)
})

const load = () => w.loadFile(path.resolve(__dirname, 'fixtures', 'chromium', 'visibilitystate.html'))

const itWithOptions = (name: string, options: BrowserWindowConstructorOptions, fn: Mocha.Func) => {
return it(name, async function (...args) {
w = new BrowserWindow({
...options,
paintWhenInitiallyHidden: false,
webPreferences: {
...(options.webPreferences || {}),
nodeIntegration: true
}
})
await Promise.resolve(fn.apply(this, args))
})
}

const getVisibilityState = async (): Promise<string> => {
const response = emittedOnce(ipcMain, 'visibility-state')
w.webContents.send('get-visibility-state')
return (await response)[1]
}

itWithOptions('should be visible when the window is initially shown by default', {}, async () => {
await load()
const state = await getVisibilityState()
expect(state).to.equal('visible')
})

itWithOptions('should be visible when the window is initially shown', {
show: true,
}, async () => {
await load()
const state = await getVisibilityState()
expect(state).to.equal('visible')
})

itWithOptions('should be hidden when the window is initially hidden', {
show: false,
}, async () => {
await load()
const state = await getVisibilityState()
expect(state).to.equal('hidden')
})

itWithOptions('should be visible when the window is initially hidden but shown before the page is loaded', {
show: false,
}, async () => {
w.show()
await load()
const state = await getVisibilityState()
expect(state).to.equal('visible')
})

itWithOptions('should be hidden when the window is initially shown but hidden before the page is loaded', {
show: true,
}, async () => {
// TODO(MarshallOfSound): Figure out if we can work around this 1 tick issue for users
if (process.platform === 'darwin') {
// Wait for a tick, the window being "shown" takes 1 tick on macOS
await new Promise(r => setTimeout(r, 0))
}
w.hide()
await load()
const state = await getVisibilityState()
expect(state).to.equal('hidden')
})

itWithOptions('should be toggle between visible and hidden as the window is hidden and shown', {}, async () => {
await load()
expect(await getVisibilityState()).to.equal('visible')
await emittedOnce(ipcMain, 'visibility-change', () => w.hide())
expect(await getVisibilityState()).to.equal('hidden')
await emittedOnce(ipcMain, 'visibility-change', () => w.show())
expect(await getVisibilityState()).to.equal('visible')
})

itWithOptions('should become hidden when a window is minimized', {}, async () => {
await load()
expect(await getVisibilityState()).to.equal('visible')
await emittedOnce(ipcMain, 'visibility-change', () => w.minimize())
expect(await getVisibilityState()).to.equal('hidden')
})

itWithOptions('should become visible when a window is restored', {}, async () => {
await load()
expect(await getVisibilityState()).to.equal('visible')
await emittedOnce(ipcMain, 'visibility-change', () => w.minimize())
await emittedOnce(ipcMain, 'visibility-change', () => w.restore())
expect(await getVisibilityState()).to.equal('visible')
})

describe('on platforms that support occlusion detection', () => {
let child: cp.ChildProcess

before(function() {
if (process.platform !== 'darwin') this.skip()
})

const makeOtherWindow = (opts: { x: number; y: number; width: number; height: number; }) => {
child = cp.spawn(process.execPath, [path.resolve(__dirname, 'fixtures', 'chromium', 'other-window.js'), `${opts.x}`, `${opts.y}`, `${opts.width}`, `${opts.height}`])
return new Promise(resolve => {
child.stdout!.on('data', (chunk) => {
if (chunk.toString().includes('__ready__')) resolve()
})
})
}

afterEach(() => {
if (child && !child.killed) {
child.kill('SIGTERM')
}
})

itWithOptions('should be visible when two windows are on screen', {
x: 0,
y: 0,
width: 200,
height: 200,
}, async () => {
await makeOtherWindow({
x: 200,
y: 0,
width: 200,
height: 200,
})
await load()
const state = await getVisibilityState()
expect(state).to.equal('visible')
})

itWithOptions('should be visible when two windows are on screen that overlap partially', {
x: 50,
y: 50,
width: 150,
height: 150,
}, async () => {
await makeOtherWindow({
x: 100,
y: 0,
width: 200,
height: 200,
})
await load()
const state = await getVisibilityState()
expect(state).to.equal('visible')
})

itWithOptions('should be hidden when a second window completely conceals the current window', {
x: 50,
y: 50,
width: 50,
height: 50,
}, async function () {
this.timeout(240000)
await load()
await emittedOnce(ipcMain, 'visibility-change', async () => {
await makeOtherWindow({
x: 0,
y: 0,
width: 300,
height: 300,
})
})
const state = await getVisibilityState()
expect(state).to.equal('hidden')
})
})
})