chore: remove most usages of session from firefox Page (#169)

src/chromium/EmulationManager.ts

@@ -1,51 +0,0 @@
-/**
- * Copyright 2017 Google Inc. All rights reserved.
- * Modifications copyright (c) Microsoft Corporation.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import { CDPSession } from './Connection';
-import { Protocol } from './protocol';
-import * as types from '../types';
-
-export class EmulationManager {
-  private _client: CDPSession;
-  private _emulatingMobile = false;
-  private _hasTouch = false;
-
-  constructor(client: CDPSession) {
-    this._client = client;
-  }
-
-  async emulateViewport(viewport: types.Viewport): Promise<boolean> {
-    const mobile = viewport.isMobile || false;
-    const width = viewport.width;
-    const height = viewport.height;
-    const deviceScaleFactor = viewport.deviceScaleFactor || 1;
-    const screenOrientation: Protocol.Emulation.ScreenOrientation = viewport.isLandscape ? { angle: 90, type: 'landscapePrimary' } : { angle: 0, type: 'portraitPrimary' };
-    const hasTouch = viewport.hasTouch || false;
-
-    await Promise.all([
-      this._client.send('Emulation.setDeviceMetricsOverride', { mobile, width, height, deviceScaleFactor, screenOrientation }),
-      this._client.send('Emulation.setTouchEmulationEnabled', {
-        enabled: hasTouch
-      })
-    ]);
-
-    const reloadNeeded = this._emulatingMobile !== mobile || this._hasTouch !== hasTouch;
-    this._emulatingMobile = mobile;
-    this._hasTouch = hasTouch;
-    return reloadNeeded;
-  }
-}

src/chromium/Page.ts

@@ -30,7 +30,6 @@ import * as types from '../types';
 import { Browser } from './Browser';
 import { BrowserContext } from './BrowserContext';
 import { CDPSession } from './Connection';
-import { EmulationManager } from './EmulationManager';
 import { Events } from './events';
 import { Accessibility } from './features/accessibility';
 import { Coverage } from './features/coverage';
@@ -42,6 +41,7 @@ import { FrameManager, FrameManagerEvents } from './FrameManager';
 import { RawKeyboardImpl, RawMouseImpl } from './Input';
 import { NetworkManagerEvents } from './NetworkManager';
 import { CRScreenshotDelegate } from './Screenshotter';
+import { Protocol } from './protocol';
 
 export class Page extends EventEmitter {
   private _closed = false;
@@ -56,7 +56,6 @@ export class Page extends EventEmitter {
   readonly mouse: input.Mouse;
   private _timeoutSettings: TimeoutSettings;
   private _frameManager: FrameManager;
-  private _emulationManager: EmulationManager;
   readonly accessibility: Accessibility;
   readonly coverage: Coverage;
   readonly overrides: Overrides;
@@ -89,7 +88,6 @@ export class Page extends EventEmitter {
     this._timeoutSettings = new TimeoutSettings();
     this.accessibility = new Accessibility(client);
     this._frameManager = new FrameManager(client, this, ignoreHTTPSErrors, this._timeoutSettings);
-    this._emulationManager = new EmulationManager(client);
     this.coverage = new Coverage(client);
     this.pdf = new PDF(client);
     this.workers = new Workers(client, this._addConsoleMessage.bind(this), error => this.emit(Events.Page.PageError, error));
@@ -381,9 +379,25 @@ export class Page extends EventEmitter {
   }
 
   async setViewport(viewport: types.Viewport) {
-    const needsReload = await this._emulationManager.emulateViewport(viewport);
+    const {
+      width,
+      height,
+      isMobile = false,
+      deviceScaleFactor = 1,
+      hasTouch = false,
+      isLandscape = false,
+    } = viewport;
+    const screenOrientation: Protocol.Emulation.ScreenOrientation = isLandscape ? { angle: 90, type: 'landscapePrimary' } : { angle: 0, type: 'portraitPrimary' };
+    await Promise.all([
+      this._client.send('Emulation.setDeviceMetricsOverride', { mobile: isMobile, width, height, deviceScaleFactor, screenOrientation }),
+      this._client.send('Emulation.setTouchEmulationEnabled', {
+        enabled: hasTouch
+      })
+    ]);
+    const oldIsMobile = this._viewport ? !!this._viewport.isMobile : false;
+    const oldHasTouch = this._viewport ? !!this._viewport.hasTouch : false;
     this._viewport = viewport;
-    if (needsReload)
+    if (oldIsMobile !== isMobile || oldHasTouch !== hasTouch)
       await this.reload();
   }
 

src/firefox/Browser.ts

@@ -18,7 +18,7 @@
 import { EventEmitter } from 'events';
 import { assert, helper, RegisteredListener } from '../helper';
 import { filterCookies, NetworkCookie, SetNetworkCookieParam, rewriteCookies } from '../network';
-import { Connection, ConnectionEvents } from './Connection';
+import { Connection, ConnectionEvents, JugglerSessionEvents } from './Connection';
 import { Events } from './events';
 import { Permissions } from './features/permissions';
 import { Page } from './Page';
@@ -233,7 +233,10 @@ export class Target {
   async page() {
     if (this._type === 'page' && !this._pagePromise) {
       const session = await this._connection.createSession(this._targetId);
-      this._pagePromise = Page.create(session, this._context, this._browser._defaultViewport);
+      this._pagePromise = Page.create(session, this._context, this._browser._defaultViewport).then(page => {
+        session.once(JugglerSessionEvents.Disconnected, () => page._didDisconnect());
+        return page;
+      });
     }
     return this._pagePromise;
   }

src/firefox/FrameManager.ts

@@ -18,7 +18,7 @@
 import { EventEmitter } from 'events';
 import { TimeoutError } from '../Errors';
 import * as frames from '../frames';
-import { assert, helper, RegisteredListener } from '../helper';
+import { assert, helper, RegisteredListener, debugError } from '../helper';
 import * as js from '../javascript';
 import * as dom from '../dom';
 import { TimeoutSettings } from '../TimeoutSettings';
@@ -28,6 +28,9 @@ import { NavigationWatchdog, NextNavigationWatchdog } from './NavigationWatchdog
 import { Page } from './Page';
 import { NetworkManager } from './NetworkManager';
 import { DOMWorldDelegate } from './JSHandle';
+import { Events } from './events';
+import * as dialog from '../dialog';
+import { Protocol } from './protocol';
 
 export const FrameManagerEvents = {
   FrameNavigated: Symbol('FrameManagerEvents.FrameNavigated'),
@@ -71,6 +74,11 @@ export class FrameManager extends EventEmitter implements frames.FrameDelegate {
       helper.addEventListener(this._session, 'Page.sameDocumentNavigation', this._onSameDocumentNavigation.bind(this)),
       helper.addEventListener(this._session, 'Runtime.executionContextCreated', this._onExecutionContextCreated.bind(this)),
       helper.addEventListener(this._session, 'Runtime.executionContextDestroyed', this._onExecutionContextDestroyed.bind(this)),
+      helper.addEventListener(this._session, 'Page.uncaughtError', this._onUncaughtError.bind(this)),
+      helper.addEventListener(this._session, 'Runtime.console', this._onConsole.bind(this)),
+      helper.addEventListener(this._session, 'Page.dialogOpened', this._onDialogOpened.bind(this)),
+      helper.addEventListener(this._session, 'Page.bindingCalled', this._onBindingCalled.bind(this)),
+      helper.addEventListener(this._session, 'Page.fileChooserOpened', this._onFileChooserOpened.bind(this)),
     ];
   }
 
@@ -173,6 +181,44 @@ export class FrameManager extends EventEmitter implements frames.FrameDelegate {
     }
   }
 
+  _onUncaughtError(params) {
+    const error = new Error(params.message);
+    error.stack = params.stack;
+    this._page.emit(Events.Page.PageError, error);
+  }
+
+  _onConsole({type, args, executionContextId, location}) {
+    const context = this.executionContextById(executionContextId);
+    this._page._addConsoleMessage(type, args.map(arg => context._createHandle(arg)), location);
+  }
+
+  _onDialogOpened(params) {
+    this._page.emit(Events.Page.Dialog, new dialog.Dialog(
+      params.type as dialog.DialogType,
+      params.message,
+      async (accept: boolean, promptText?: string) => {
+        await this._session.send('Page.handleDialog', { dialogId: params.dialogId, accept, promptText }).catch(debugError);
+      },
+      params.defaultValue));
+  }
+
+  _onBindingCalled(event: Protocol.Page.bindingCalledPayload) {
+    const context = this.executionContextById(event.executionContextId);
+    this._page._onBindingCalled(event.payload, context);
+  }
+
+  async _onFileChooserOpened({executionContextId, element}) {
+    const context = this.executionContextById(executionContextId);
+    const handle = context._createHandle(element).asElement()!;
+    this._page._onFileChooserOpened(handle);
+  }
+
+  async _exposeBinding(name: string, bindingFunction: string) {
+    await this._session.send('Page.addBinding', {name: name});
+    await this._session.send('Page.addScriptToEvaluateOnNewDocument', {script: bindingFunction});
+    await Promise.all(this.frames().map(frame => frame.evaluate(bindingFunction).catch(debugError)));
+  }
+
   dispose() {
     helper.removeEventListeners(this._eventListeners);
   }

src/firefox/Page.ts

@@ -17,7 +17,6 @@
 
 import { EventEmitter } from 'events';
 import * as console from '../console';
-import * as dialog from '../dialog';
 import * as dom from '../dom';
 import { TimeoutError } from '../Errors';
 import * as frames from '../frames';
@@ -29,7 +28,7 @@ import { Screenshotter } from '../screenshotter';
 import { TimeoutSettings } from '../TimeoutSettings';
 import * as types from '../types';
 import { BrowserContext } from './Browser';
-import { JugglerSession, JugglerSessionEvents } from './Connection';
+import { JugglerSession } from './Connection';
 import { Events } from './events';
 import { Accessibility } from './features/accessibility';
 import { Interception } from './features/interception';
@@ -50,13 +49,15 @@ export class Page extends EventEmitter {
   private _closed: boolean;
   private _closedCallback: () => void;
   private _closedPromise: Promise<void>;
+  private _disconnected = false;
+  private _disconnectedCallback: (e: Error) => void;
+  private _disconnectedPromise: Promise<Error>;
   private _pageBindings: Map<string, Function>;
   private _networkManager: NetworkManager;
   _frameManager: FrameManager;
   _javascriptEnabled = true;
   private _eventListeners: RegisteredListener[];
   private _viewport: types.Viewport;
-  private _disconnectPromise: Promise<Error>;
   private _fileChooserInterceptors = new Set<(chooser: FileChooser) => void>();
   _screenshotter: Screenshotter;
 
@@ -84,17 +85,13 @@ export class Page extends EventEmitter {
     this.accessibility = new Accessibility(session);
     this._closed = false;
     this._closedPromise = new Promise(f => this._closedCallback = f);
+    this._disconnectedPromise = new Promise(f => this._disconnectedCallback = f);
     this._pageBindings = new Map();
     this._networkManager = new NetworkManager(session);
     this._frameManager = new FrameManager(session, this, this._networkManager, this._timeoutSettings);
     this._networkManager.setFrameManager(this._frameManager);
     this.interception = new Interception(this._networkManager);
     this._eventListeners = [
-      helper.addEventListener(this._session, 'Page.uncaughtError', this._onUncaughtError.bind(this)),
-      helper.addEventListener(this._session, 'Runtime.console', this._onConsole.bind(this)),
-      helper.addEventListener(this._session, 'Page.dialogOpened', this._onDialogOpened.bind(this)),
-      helper.addEventListener(this._session, 'Page.bindingCalled', this._onBindingCalled.bind(this)),
-      helper.addEventListener(this._session, 'Page.fileChooserOpened', this._onFileChooserOpened.bind(this)),
       helper.addEventListener(this._frameManager, FrameManagerEvents.Load, () => this.emit(Events.Page.Load)),
       helper.addEventListener(this._frameManager, FrameManagerEvents.DOMContentLoaded, () => this.emit(Events.Page.DOMContentLoaded)),
       helper.addEventListener(this._frameManager, FrameManagerEvents.FrameAttached, frame => this.emit(Events.Page.FrameAttached, frame)),
@@ -119,6 +116,12 @@ export class Page extends EventEmitter {
     this._closedCallback();
   }
 
+  _didDisconnect() {
+    assert(!this._disconnected, 'Page disconnected twice');
+    this._disconnected = true;
+    this._disconnectedCallback(new Error('Target closed'));
+  }
+
   async setExtraHTTPHeaders(headers) {
     await this._networkManager.setExtraHTTPHeaders(headers);
   }
@@ -135,11 +138,7 @@ export class Page extends EventEmitter {
     if (this._pageBindings.has(name))
       throw new Error(`Failed to add page binding with name ${name}: window['${name}'] already exists!`);
     this._pageBindings.set(name, playwrightFunction);
-
+    await this._frameManager._exposeBinding(name, helper.evaluationString(addPageBinding, name));
-    const expression = helper.evaluationString(addPageBinding, name);
-    await this._session.send('Page.addBinding', {name: name});
-    await this._session.send('Page.addScriptToEvaluateOnNewDocument', {script: expression});
-    await Promise.all(this.frames().map(frame => frame.evaluate(expression).catch(debugError)));
 
     function addPageBinding(bindingName: string) {
       const binding: (string) => void = window[bindingName];
@@ -159,8 +158,8 @@ export class Page extends EventEmitter {
     }
   }
 
-  async _onBindingCalled(event: any) {
+  async _onBindingCalled(payload: string, context: js.ExecutionContext) {
-    const {name, seq, args} = JSON.parse(event.payload);
+    const {name, seq, args} = JSON.parse(payload);
     let expression = null;
     try {
       const result = await this._pageBindings.get(name)(...args);
@@ -171,7 +170,7 @@ export class Page extends EventEmitter {
       else
         expression = helper.evaluationString(deliverErrorValue, name, seq, error);
     }
-    this._session.send('Runtime.evaluate', { expression, executionContextId: event.executionContextId }).catch(debugError);
+    context.evaluate(expression).catch(debugError);
 
     function deliverResult(name: string, seq: number, result: any) {
       window[name]['callbacks'].get(seq).resolve(result);
@@ -191,12 +190,6 @@ export class Page extends EventEmitter {
     }
   }
 
-  _sessionClosePromise() {
-    if (!this._disconnectPromise)
-      this._disconnectPromise = new Promise<Error>(fulfill => this._session.once(JugglerSessionEvents.Disconnected, () => fulfill(new Error('Target closed'))));
-    return this._disconnectPromise;
-  }
-
   async waitForRequest(urlOrPredicate: (string | Function), options: { timeout?: number; } | undefined = {}): Promise<network.Request> {
     const {
       timeout = this._timeoutSettings.timeout(),
@@ -207,7 +200,7 @@ export class Page extends EventEmitter {
       if (typeof urlOrPredicate === 'function')
         return !!(urlOrPredicate(request));
       return false;
-    }, timeout, this._sessionClosePromise());
+    }, timeout, this._disconnectedPromise);
   }
 
   async waitForResponse(urlOrPredicate: (string | Function), options: { timeout?: number; } | undefined = {}): Promise<network.Response> {
@@ -220,7 +213,7 @@ export class Page extends EventEmitter {
       if (typeof urlOrPredicate === 'function')
         return !!(urlOrPredicate(response));
       return false;
-    }, timeout, this._sessionClosePromise());
+    }, timeout, this._disconnectedPromise);
   }
 
   setDefaultNavigationTimeout(timeout: number) {
@@ -259,12 +252,6 @@ export class Page extends EventEmitter {
     return this._browserContext;
   }
 
-  _onUncaughtError(params) {
-    const error = new Error(params.message);
-    error.stack = params.stack;
-    this.emit(Events.Page.PageError, error);
-  }
-
   viewport() {
     return this._viewport;
   }
@@ -305,16 +292,6 @@ export class Page extends EventEmitter {
     return this._frameManager.frames();
   }
 
-  _onDialogOpened(params) {
-    this.emit(Events.Page.Dialog, new dialog.Dialog(
-      params.type as dialog.DialogType,
-      params.message,
-      async (accept: boolean, promptText?: string) => {
-        await this._session.send('Page.handleDialog', { dialogId: params.dialogId, accept, promptText }).catch(debugError);
-      },
-      params.defaultValue));
-  }
-
   mainFrame(): frames.Frame {
     return this._frameManager.mainFrame();
   }
@@ -518,6 +495,7 @@ export class Page extends EventEmitter {
   }
 
   async close(options: any = {}) {
+    assert(!this._disconnected, 'Protocol error: Connection closed. Most likely the page has been closed.');
     const {
       runBeforeUnload = false,
     } = options;
@@ -534,9 +512,12 @@ export class Page extends EventEmitter {
     return await this._frameManager.mainFrame().setContent(html);
   }
 
-  _onConsole({type, args, executionContextId, location}) {
+  _addConsoleMessage(type: string, args: js.JSHandle[], location: console.ConsoleMessageLocation) {
-    const context = this._frameManager.executionContextById(executionContextId);
+    if (!this.listenerCount(Events.Page.Console)) {
-    this.emit(Events.Page.Console, new console.ConsoleMessage(type, undefined, args.map(arg => context._createHandle(arg)), location));
+      args.forEach(arg => arg.dispose());
+      return;
+    }
+    this.emit(Events.Page.Console, new console.ConsoleMessage(type, undefined, args, location));
   }
 
   isClosed(): boolean {
@@ -556,11 +537,11 @@ export class Page extends EventEmitter {
     });
   }
 
-  async _onFileChooserOpened({executionContextId, element}) {
+  async _onFileChooserOpened(handle: dom.ElementHandle) {
-    if (!this._fileChooserInterceptors.size)
+    if (!this._fileChooserInterceptors.size) {
+      await handle.dispose();
       return;
-    const context = this._frameManager.executionContextById(executionContextId);
+    }
-    const handle = context._createHandle(element).asElement()!;
     const interceptors = Array.from(this._fileChooserInterceptors);
     this._fileChooserInterceptors.clear();
     const multiple = await handle.evaluate((element: HTMLInputElement) => !!element.multiple);