fix initial visibility and add tests

shell/browser/api/electron_api_web_contents_view.cc

@@ -16,6 +16,7 @@
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/object_template_builder.h"
 #include "shell/common/node_includes.h"
+#include "shell/common/options_switches.h"
 #include "third_party/skia/include/core/SkRegion.h"
 #include "ui/base/hit_test.h"
 #include "ui/views/layout/flex_layout_types.h"
@@ -119,6 +120,8 @@ gin::Handle<WebContentsView> WebContentsView::Create(
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
   v8::Local<v8::Value> arg = gin::ConvertToV8(isolate, web_preferences);
   v8::Local<v8::Object> obj;
+  if (!web_preferences.Has(options::kShow))
+    gin::Dictionary(isolate, arg.As<v8::Object>()).Set(options::kShow, true);
   if (GetConstructor(isolate)->NewInstance(context, 1, &arg).ToLocal(&obj)) {
     gin::Handle<WebContentsView> web_contents_view;
     if (gin::ConvertFromV8(isolate, obj, &web_contents_view))
@@ -143,6 +146,8 @@ gin_helper::WrappableBase* WebContentsView::New(gin_helper::Arguments* args) {
   gin_helper::Dictionary web_preferences =
       gin::Dictionary::CreateEmpty(args->isolate());
   args->GetNext(&web_preferences);
+  if (!web_preferences.Has(options::kShow))
+    web_preferences.Set(options::kShow, false);
   auto web_contents =
       WebContents::CreateFromWebPreferences(args->isolate(), web_preferences);
 

spec/api-web-contents-view-spec.ts

@@ -1,13 +1,13 @@
-import { closeWindow } from './lib/window-helpers';
+import { closeAllWindows } from './lib/window-helpers';
+import { expect } from 'chai';
 
 import { BaseWindow, WebContentsView } from 'electron/main';
 
 describe('WebContentsView', () => {
-  let w: BaseWindow;
-  afterEach(() => closeWindow(w as any).then(() => { w = null as unknown as BaseWindow; }));
+  afterEach(closeAllWindows);
 
   it('can be used as content view', () => {
-    w = new BaseWindow({ show: false });
+    const w = new BaseWindow({ show: false });
     w.setContentView(new WebContentsView({}));
   });
 
@@ -34,4 +34,71 @@ describe('WebContentsView', () => {
       done();
     });
   });
+
+  describe('visibilityState', () => {
+    it('is initially hidden', async () => {
+      const v = new WebContentsView();
+      await v.webContents.loadURL('data:text/html,<script>initialVisibility = document.visibilityState</script>');
+      expect(await v.webContents.executeJavaScript('initialVisibility')).to.equal('hidden');
+    });
+
+    it('becomes visibile when attached', async () => {
+      const v = new WebContentsView();
+      await v.webContents.loadURL('about:blank');
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('hidden');
+      const p = v.webContents.executeJavaScript('new Promise(resolve => document.addEventListener("visibilitychange", resolve))');
+      const w = new BaseWindow();
+      w.setContentView(v);
+      await p;
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('visible');
+    });
+
+    it('is initially visible if load happens after attach', async () => {
+      const w = new BaseWindow();
+      const v = new WebContentsView();
+      w.contentView = v;
+      await v.webContents.loadURL('data:text/html,<script>initialVisibility = document.visibilityState</script>');
+      expect(await v.webContents.executeJavaScript('initialVisibility')).to.equal('visible');
+    });
+
+    it('becomes hidden when parent window is hidden', async () => {
+      const w = new BaseWindow();
+      const v = new WebContentsView();
+      w.setContentView(v);
+      await v.webContents.loadURL('about:blank');
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('visible');
+      const p = v.webContents.executeJavaScript('new Promise(resolve => document.addEventListener("visibilitychange", resolve))');
+      w.hide();
+      await p;
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('hidden');
+    });
+
+    it('becomes visible when parent window is shown', async () => {
+      const w = new BaseWindow({ show: false });
+      const v = new WebContentsView();
+      w.setContentView(v);
+      await v.webContents.loadURL('about:blank');
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('hidden');
+      const p = v.webContents.executeJavaScript('new Promise(resolve => document.addEventListener("visibilitychange", resolve))');
+      w.show();
+      await p;
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('visible');
+    });
+
+    it('does not change when view is moved between two visible windows', async () => {
+      const w = new BaseWindow();
+      const v = new WebContentsView();
+      w.setContentView(v);
+      await v.webContents.loadURL('about:blank');
+      expect(await v.webContents.executeJavaScript('document.visibilityState')).to.equal('visible');
+
+      const p = v.webContents.executeJavaScript('new Promise(resolve => document.addEventListener("visibilitychange", () => resolve(document.visibilityState)))');
+      const w2 = new BaseWindow();
+      w2.setContentView(v);
+      // A visibilitychange event is triggered, because the page cycled from
+      // visible -> hidden -> visible, but the page's JS can't observe the
+      // 'hidden' state.
+      expect(await p).to.equal('visible');
+    });
+  });
 });

spec/lib/window-helpers.ts

@@ -1,10 +1,10 @@
 import { expect } from 'chai';
-import { BrowserWindow } from 'electron/main';
+import { BaseWindow, BrowserWindow } from 'electron/main';
 import { emittedOnce } from './events-helpers';
 
-async function ensureWindowIsClosed (window: BrowserWindow | null) {
+async function ensureWindowIsClosed (window: BaseWindow | null) {
   if (window && !window.isDestroyed()) {
-    if (window.webContents && !window.webContents.isDestroyed()) {
+    if (window instanceof BrowserWindow && window.webContents && !window.webContents.isDestroyed()) {
       // If a window isn't destroyed already, and it has non-destroyed WebContents,
       // then calling destroy() won't immediately destroy it, as it may have
       // <webview> children which need to be destroyed first. In that case, we
@@ -23,13 +23,13 @@ async function ensureWindowIsClosed (window: BrowserWindow | null) {
 }
 
 export const closeWindow = async (
-  window: BrowserWindow | null = null,
+  window: BaseWindow | null = null,
   { assertNotWindows } = { assertNotWindows: true }
 ) => {
   await ensureWindowIsClosed(window);
 
   if (assertNotWindows) {
-    const windows = BrowserWindow.getAllWindows();
+    const windows = BaseWindow.getAllWindows();
     try {
       expect(windows).to.have.lengthOf(0);
     } finally {
@@ -41,7 +41,7 @@ export const closeWindow = async (
 };
 
 export async function closeAllWindows () {
-  for (const w of BrowserWindow.getAllWindows()) {
+  for (const w of BaseWindow.getAllWindows()) {
     await closeWindow(w, { assertNotWindows: false });
   }
 }

spec/visibility-state-spec.ts

@@ -1,6 +1,6 @@
 import { expect } from 'chai';
 import * as cp from 'child_process';
-import { BrowserWindow, BrowserWindowConstructorOptions, ipcMain } from 'electron/main';
+import { BaseWindow, BrowserWindow, BrowserWindowConstructorOptions, ipcMain, WebContents, WebContentsView } from 'electron/main';
 import * as path from 'path';
 
 import { emittedOnce } from './lib/events-helpers';
@@ -10,16 +10,16 @@ import { ifdescribe, delay } from './lib/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')('document.visibilityState', () => {
-  let w: BrowserWindow;
+  let w: BaseWindow & {webContents: WebContents};
 
   afterEach(() => {
     return closeWindow(w);
   });
 
-  const load = () => w.loadFile(path.resolve(__dirname, 'fixtures', 'chromium', 'visibilitystate.html'));
+  const load = () => w.webContents.loadFile(path.resolve(__dirname, 'fixtures', 'chromium', 'visibilitystate.html'));
 
   const itWithOptions = (name: string, options: BrowserWindowConstructorOptions, fn: Mocha.Func) => {
-    return it(name, async function (...args) {
+    it(name, async function (...args) {
       w = new BrowserWindow({
         ...options,
         paintWhenInitiallyHidden: false,
@@ -31,6 +31,16 @@ ifdescribe(process.platform !== 'linux')('document.visibilityState', () => {
       });
       await Promise.resolve(fn.apply(this, args));
     });
+
+    it(name + ' with BaseWindow', async function (...args) {
+      const baseWindow = new BaseWindow({
+        ...options
+      });
+      const wcv = new WebContentsView({ ...(options.webPreferences ?? {}), nodeIntegration: true, contextIsolation: false });
+      baseWindow.contentView = wcv;
+      w = Object.assign(baseWindow, { webContents: wcv.webContents });
+      await Promise.resolve(fn.apply(this, args));
+    });
   };
 
   itWithOptions('should be visible when the window is initially shown by default', {}, async () => {