chore: move frame id to Frame, remove unused events (#235)

src/chromium/FrameManager.ts

@@ -45,19 +45,6 @@ import { ConsoleMessage } from '../console';
 
 const UTILITY_WORLD_NAME = '__playwright_utility_world__';
 
-export const FrameManagerEvents = {
-  FrameAttached: Symbol('Events.FrameManager.FrameAttached'),
-  FrameNavigated: Symbol('Events.FrameManager.FrameNavigated'),
-  FrameDetached: Symbol('Events.FrameManager.FrameDetached'),
-  LifecycleEvent: Symbol('Events.FrameManager.LifecycleEvent'),
-  FrameNavigatedWithinDocument: Symbol('Events.FrameManager.FrameNavigatedWithinDocument'),
-};
-
-const frameDataSymbol = Symbol('frameData');
-type FrameData = {
-  id: string,
-};
-
 export class FrameManager extends EventEmitter implements PageDelegate {
   _client: CDPSession;
   private _page: Page;
@@ -131,10 +118,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     return this._networkManager;
   }
 
-  _frameData(frame: frames.Frame): FrameData {
-    return (frame as any)[frameDataSymbol];
-  }
-
   async navigateFrame(frame: frames.Frame, url: string, options: frames.GotoOptions = {}): Promise<network.Response | null> {
     assertNoLegacyNavigationOptions(options);
     const {
@@ -146,7 +129,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     const watcher = new frames.LifecycleWatcher(frame, waitUntil, timeout);
     let ensureNewDocumentNavigation = false;
     let error = await Promise.race([
-      navigate(this._client, url, referer, this._frameData(frame).id),
+      navigate(this._client, url, referer, frame._id),
       watcher.timeoutOrTerminationPromise,
     ]);
     if (!error) {
@@ -224,7 +207,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     } else if (event.name === 'DOMContentLoaded') {
       frame._lifecycleEvent('domcontentloaded');
     }
-    this.emit(FrameManagerEvents.LifecycleEvent, frame);
   }
 
   _onFrameStoppedLoading(frameId: string) {
@@ -233,7 +215,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
       return;
     frame._lifecycleEvent('domcontentloaded');
     frame._lifecycleEvent('load');
-    this.emit(FrameManagerEvents.LifecycleEvent, frame);
   }
 
   _handleFrameTree(frameTree: Protocol.Page.FrameTree) {
@@ -268,13 +249,8 @@ export class FrameManager extends EventEmitter implements PageDelegate {
       return;
     assert(parentFrameId);
     const parentFrame = this._frames.get(parentFrameId);
-    const frame = new frames.Frame(this._page, parentFrame);
+    const frame = new frames.Frame(this._page, frameId, parentFrame);
-    const data: FrameData = {
-      id: frameId,
-    };
-    (frame as any)[frameDataSymbol] = data;
     this._frames.set(frameId, frame);
-    this.emit(FrameManagerEvents.FrameAttached, frame);
     this._page.emit(Events.Page.FrameAttached, frame);
   }
 
@@ -293,16 +269,11 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     if (isMainFrame) {
       if (frame) {
         // Update frame id to retain frame identity on cross-process navigation.
-        const data = this._frameData(frame);
+        this._frames.delete(frame._id);
-        this._frames.delete(data.id);
+        frame._id = framePayload.id;
-        data.id = framePayload.id;
       } else {
         // Initial main frame navigation.
-        frame = new frames.Frame(this._page, null);
+        frame = new frames.Frame(this._page, framePayload.id, null);
-        const data: FrameData = {
-          id: framePayload.id,
-        };
-        (frame as any)[frameDataSymbol] = data;
       }
       this._frames.set(framePayload.id, frame);
       this._mainFrame = frame;
@@ -310,7 +281,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
 
     frame._onCommittedNewDocumentNavigation(framePayload.url, framePayload.name, framePayload.loaderId);
 
-    this.emit(FrameManagerEvents.FrameNavigated, frame);
     this._page.emit(Events.Page.FrameNavigated, frame);
   }
 
@@ -323,7 +293,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
       worldName: name,
     });
     await Promise.all(this.frames().map(frame => this._client.send('Page.createIsolatedWorld', {
-      frameId: this._frameData(frame).id,
+      frameId: frame._id,
       grantUniveralAccess: true,
       worldName: name,
     }).catch(debugError))); // frames might be removed before we send this
@@ -334,8 +304,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     if (!frame)
       return;
     frame._onCommittedSameDocumentNavigation(url);
-    this.emit(FrameManagerEvents.FrameNavigatedWithinDocument, frame);
-    this.emit(FrameManagerEvents.FrameNavigated, frame);
     this._page.emit(Events.Page.FrameNavigated, frame);
   }
 
@@ -387,8 +355,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     for (const child of frame.childFrames())
       this._removeFramesRecursively(child);
     frame._onDetached();
-    this._frames.delete(this._frameData(frame).id);
+    this._frames.delete(frame._id);
-    this.emit(FrameManagerEvents.FrameDetached, frame);
     this._page.emit(Events.Page.FrameDetached, frame);
   }
 

src/firefox/FrameManager.ts

@@ -35,19 +35,6 @@ import { Accessibility } from './features/accessibility';
 import * as network from '../network';
 import * as types from '../types';
 
-export const FrameManagerEvents = {
-  FrameNavigated: Symbol('FrameManagerEvents.FrameNavigated'),
-  FrameAttached: Symbol('FrameManagerEvents.FrameAttached'),
-  FrameDetached: Symbol('FrameManagerEvents.FrameDetached'),
-  Load: Symbol('FrameManagerEvents.Load'),
-  DOMContentLoaded: Symbol('FrameManagerEvents.DOMContentLoaded'),
-};
-
-const frameDataSymbol = Symbol('frameData');
-type FrameData = {
-  frameId: string,
-};
-
 export class FrameManager extends EventEmitter implements PageDelegate {
   readonly rawMouse: RawMouseImpl;
   readonly rawKeyboard: RawKeyboardImpl;
@@ -129,10 +116,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
       context.frame()._contextDestroyed(context as dom.FrameExecutionContext);
   }
 
-  _frameData(frame: frames.Frame): FrameData {
-    return (frame as any)[frameDataSymbol];
-  }
-
   frame(frameId: string): frames.Frame {
     return this._frames.get(frameId);
   }
@@ -166,30 +149,23 @@ export class FrameManager extends EventEmitter implements PageDelegate {
   _onNavigationCommitted(params) {
     const frame = this._frames.get(params.frameId);
     frame._onCommittedNewDocumentNavigation(params.url, params.name, params.navigationId);
-    this.emit(FrameManagerEvents.FrameNavigated, frame);
     this._page.emit(Events.Page.FrameNavigated, frame);
   }
 
   _onSameDocumentNavigation(params) {
     const frame = this._frames.get(params.frameId);
     frame._onCommittedSameDocumentNavigation(params.url);
-    this.emit(FrameManagerEvents.FrameNavigated, frame);
     this._page.emit(Events.Page.FrameNavigated, frame);
   }
 
   _onFrameAttached(params) {
     const parentFrame = this._frames.get(params.parentFrameId) || null;
-    const frame = new frames.Frame(this._page, parentFrame);
+    const frame = new frames.Frame(this._page, params.frameId, parentFrame);
-    const data: FrameData = {
-      frameId: params.frameId,
-    };
-    frame[frameDataSymbol] = data;
     if (!parentFrame) {
       assert(!this._mainFrame, 'INTERNAL ERROR: re-attaching main frame!');
       this._mainFrame = frame;
     }
     this._frames.set(params.frameId, frame);
-    this.emit(FrameManagerEvents.FrameAttached, frame);
     this._page.emit(Events.Page.FrameAttached, frame);
   }
 
@@ -197,7 +173,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     const frame = this._frames.get(params.frameId);
     this._frames.delete(params.frameId);
     frame._onDetached();
-    this.emit(FrameManagerEvents.FrameDetached, frame);
     this._page.emit(Events.Page.FrameDetached, frame);
   }
 
@@ -205,17 +180,13 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     const frame = this._frames.get(frameId);
     if (name === 'load') {
       frame._lifecycleEvent('load');
-      if (frame === this._mainFrame) {
+      if (frame === this._mainFrame)
-        this.emit(FrameManagerEvents.Load);
         this._page.emit(Events.Page.Load);
-      }
     }
     if (name === 'DOMContentLoaded') {
       frame._lifecycleEvent('domcontentloaded');
-      if (frame === this._mainFrame) {
+      if (frame === this._mainFrame)
-        this.emit(FrameManagerEvents.DOMContentLoaded);
         this._page.emit(Events.Page.DOMContentLoaded);
-      }
     }
   }
 
@@ -288,7 +259,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     } = options;
     const watcher = new frames.LifecycleWatcher(frame, waitUntil, timeout);
     await this._session.send('Page.navigate', {
-      frameId: this._frameData(frame).frameId,
+      frameId: frame._id,
       referer,
       url,
     });
@@ -391,15 +362,15 @@ export class FrameManager extends EventEmitter implements PageDelegate {
   }
 
   reload(options?: frames.NavigateOptions): Promise<network.Response | null> {
-    return this._go(() => this._session.send('Page.reload', { frameId: this._frameData(this.mainFrame()).frameId }), options);
+    return this._go(() => this._session.send('Page.reload', { frameId: this.mainFrame()._id }), options);
   }
 
   goBack(options?: frames.NavigateOptions): Promise<network.Response | null> {
-    return this._go(() => this._session.send('Page.goBack', { frameId: this._frameData(this.mainFrame()).frameId }), options);
+    return this._go(() => this._session.send('Page.goBack', { frameId: this.mainFrame()._id }), options);
   }
 
   goForward(options?: frames.NavigateOptions): Promise<network.Response | null> {
-    return this._go(() => this._session.send('Page.goForward', { frameId: this._frameData(this.mainFrame()).frameId }), options);
+    return this._go(() => this._session.send('Page.goForward', { frameId: this.mainFrame()._id }), options);
   }
 
   async evaluateOnNewDocument(source: string): Promise<void> {
@@ -411,7 +382,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
   }
 
   getBoundingBoxForScreenshot(handle: dom.ElementHandle<Node>): Promise<types.Rect | null> {
-    const frameId = this._frameData(handle.executionContext().frame()).frameId;
+    const frameId = handle._context.frame()._id;
     return this._session.send('Page.getBoundingBox', {
       frameId,
       objectId: handle._remoteObject.objectId,
@@ -442,7 +413,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
 
   async getContentFrame(handle: dom.ElementHandle): Promise<frames.Frame | null> {
     const { frameId } = await this._session.send('Page.contentFrame', {
-      frameId: this._frameData(handle._context.frame()).frameId,
+      frameId: handle._context.frame()._id,
       objectId: toRemoteObject(handle).objectId,
     });
     if (!frameId)
@@ -475,7 +446,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
 
   async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
     const result = await this._session.send('Page.getContentQuads', {
-      frameId: this._frameData(handle._context.frame()).frameId,
+      frameId: handle._context.frame()._id,
       objectId: toRemoteObject(handle).objectId,
     }).catch(debugError);
     if (!result)

src/frames.ts

@@ -49,6 +49,7 @@ export type GotoOptions = NavigateOptions & {
 export type LifecycleEvent = 'load' | 'domcontentloaded';
 
 export class Frame {
+  _id: string;
   readonly _firedLifecycleEvents: Set<LifecycleEvent>;
   _lastDocumentId: string;
   readonly _page: Page;
@@ -59,7 +60,8 @@ export class Frame {
   private _childFrames = new Set<Frame>();
   private _name: string;
 
-  constructor(page: Page, parentFrame: Frame | null) {
+  constructor(page: Page, id: string, parentFrame: Frame | null) {
+    this._id = id;
     this._firedLifecycleEvents = new Set();
     this._lastDocumentId = '';
     this._page = page;

src/webkit/FrameManager.ts

@@ -39,19 +39,6 @@ import { PNG } from 'pngjs';
 const UTILITY_WORLD_NAME = '__playwright_utility_world__';
 const BINDING_CALL_MESSAGE = '__playwright_binding_call__';
 
-export const FrameManagerEvents = {
-  FrameNavigatedWithinDocument: Symbol('FrameNavigatedWithinDocument'),
-  FrameAttached: Symbol('FrameAttached'),
-  FrameDetached: Symbol('FrameDetached'),
-  FrameNavigated: Symbol('FrameNavigated'),
-  LifecycleEvent: Symbol('LifecycleEvent'),
-};
-
-const frameDataSymbol = Symbol('frameData');
-type FrameData = {
-  id: string,
-};
-
 let lastDocumentId = 0;
 
 export class FrameManager extends EventEmitter implements PageDelegate {
@@ -164,7 +151,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     const hasLoad = frame._firedLifecycleEvents.has('load');
     frame._lifecycleEvent('domcontentloaded');
     frame._lifecycleEvent('load');
-    this.emit(FrameManagerEvents.LifecycleEvent, frame);
     if (frame === this.mainFrame() && !hasDOMContentLoaded)
       this._page.emit(Events.Page.DOMContentLoaded);
     if (frame === this.mainFrame() && !hasLoad)
@@ -176,7 +162,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     if (!frame)
       return;
     frame._lifecycleEvent(event);
-    this.emit(FrameManagerEvents.LifecycleEvent, frame);
     if (frame === this.mainFrame()) {
       if (event === 'load')
         this._page.emit(Events.Page.Load);
@@ -211,22 +196,13 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     return this._frames.get(frameId) || null;
   }
 
-  _frameData(frame: frames.Frame): FrameData {
-    return (frame as any)[frameDataSymbol];
-  }
-
   _onFrameAttached(frameId: string, parentFrameId: string | null) {
     assert(!this._frames.has(frameId));
     const parentFrame = parentFrameId ? this._frames.get(parentFrameId) : null;
-    const frame = new frames.Frame(this._page, parentFrame);
+    const frame = new frames.Frame(this._page, frameId, parentFrame);
-    const data: FrameData = {
-      id: frameId,
-    };
-    frame[frameDataSymbol] = data;
     this._frames.set(frameId, frame);
     if (!parentFrame)
       this._mainFrame = frame;
-    this.emit(FrameManagerEvents.FrameAttached, frame);
     this._page.emit(Events.Page.FrameAttached, frame);
     return frame;
   }
@@ -240,10 +216,9 @@ export class FrameManager extends EventEmitter implements PageDelegate {
       this._removeFramesRecursively(child);
     if (isMainFrame) {
       // Update frame id to retain frame identity on cross-process navigation.
-      const data = this._frameData(frame);
+      this._frames.delete(frame._id);
-      this._frames.delete(data.id);
+      frame._id = framePayload.id;
-      data.id = framePayload.id;
+      this._frames.set(framePayload.id, frame);
-      this._frames.set(data.id, frame);
     }
 
     for (const context of this._contextIdToContext.values()) {
@@ -261,7 +236,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
       frame._onExpectedNewDocumentNavigation(documentId);
     frame._onCommittedNewDocumentNavigation(framePayload.url, framePayload.name, documentId);
 
-    this.emit(FrameManagerEvents.FrameNavigated, frame);
     this._page.emit(Events.Page.FrameNavigated, frame);
   }
 
@@ -270,8 +244,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     if (!frame)
       return;
     frame._onCommittedSameDocumentNavigation(url);
-    this.emit(FrameManagerEvents.FrameNavigatedWithinDocument, frame);
-    this.emit(FrameManagerEvents.FrameNavigated, frame);
     this._page.emit(Events.Page.FrameNavigated, frame);
   }
 
@@ -313,8 +285,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     for (const child of frame.childFrames())
       this._removeFramesRecursively(child);
     frame._onDetached();
-    this._frames.delete(this._frameData(frame).id);
+    this._frames.delete(frame._id);
-    this.emit(FrameManagerEvents.FrameDetached, frame);
     this._page.emit(Events.Page.FrameDetached, frame);
   }
 
@@ -324,7 +295,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
       waitUntil = (['load'] as frames.LifecycleEvent[])
     } = options;
     const watchDog = new frames.LifecycleWatcher(frame, waitUntil, timeout);
-    await this._session.send('Page.navigate', {url, frameId: this._frameData(frame).id});
+    await this._session.send('Page.navigate', {url, frameId: frame._id});
     const error = await Promise.race([
       watchDog.timeoutOrTerminationPromise,
       watchDog.newDocumentNavigationPromise,