fix(webkit): handle frameAttached event (#227)

package.json

@@ -10,7 +10,7 @@
   "playwright": {
     "chromium_revision": "719491",
     "firefox_revision": "1004",
-    "webkit_revision": "1029"
+    "webkit_revision": "1030"
   },
   "scripts": {
     "unit": "node test/test.js",

src/webkit/FrameManager.ts

@@ -132,6 +132,7 @@ export class FrameManager extends EventEmitter implements PageDelegate {
     this._sessionListeners = [
       helper.addEventListener(this._session, 'Page.frameNavigated', event => this._onFrameNavigated(event.frame, false)),
       helper.addEventListener(this._session, 'Page.navigatedWithinDocument', event => this._onFrameNavigatedWithinDocument(event.frameId, event.url)),
+      helper.addEventListener(this._session, 'Page.frameAttached', event => this._onFrameAttached(event.frameId, event.parentFrameId)),
       helper.addEventListener(this._session, 'Page.frameDetached', event => this._onFrameDetached(event.frameId)),
       helper.addEventListener(this._session, 'Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId)),
       helper.addEventListener(this._session, 'Page.loadEventFired', event => this._onLifecycleEvent(event.frameId, 'load')),
@@ -185,7 +186,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
   }
 
   _handleFrameTree(frameTree: Protocol.Page.FrameResourceTree) {
-    if (frameTree.frame.parentId)
     this._onFrameAttached(frameTree.frame.id, frameTree.frame.parentId);
     this._onFrameNavigated(frameTree.frame, true);
     if (!frameTree.childFrames)
@@ -216,16 +216,16 @@ export class FrameManager extends EventEmitter implements PageDelegate {
   }
 
   _onFrameAttached(frameId: string, parentFrameId: string | null) {
-    if (this._frames.has(frameId))
+    assert(!this._frames.has(frameId));
-      return;
+    const parentFrame = parentFrameId ? this._frames.get(parentFrameId) : null;
-    assert(parentFrameId);
-    const parentFrame = this._frames.get(parentFrameId);
     const frame = new frames.Frame(this._page, 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;
@@ -233,10 +233,9 @@ export class FrameManager extends EventEmitter implements PageDelegate {
 
   _onFrameNavigated(framePayload: Protocol.Page.Frame, initial: boolean) {
     const isMainFrame = !framePayload.parentId;
-    let frame = isMainFrame ? this._mainFrame : this._frames.get(framePayload.id);
+    const frame = isMainFrame ? this._mainFrame : this._frames.get(framePayload.id);
 
     // Detach all child frames first.
-    if (frame) {
     for (const child of frame.childFrames())
       this._removeFramesRecursively(child);
     if (isMainFrame) {
@@ -246,21 +245,6 @@ export class FrameManager extends EventEmitter implements PageDelegate {
       data.id = framePayload.id;
       this._frames.set(data.id, frame);
     }
-    } else if (isMainFrame) {
-      // Initial frame navigation.
-      frame = new frames.Frame(this._page, null);
-      const data: FrameData = {
-        id: framePayload.id,
-      };
-      frame[frameDataSymbol] = data;
-      this._frames.set(framePayload.id, frame);
-    } else {
-      // FIXME(WebKit): there is no Page.frameAttached event in WK.
-      frame = this._onFrameAttached(framePayload.id, framePayload.parentId);
-    }
-    // Update or create main frame.
-    if (isMainFrame)
-      this._mainFrame = frame;
 
     for (const context of this._contextIdToContext.values()) {
       if (context.frame() === frame) {

test/navigation.spec.js

@@ -47,34 +47,34 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME
       await page.goto(server.EMPTY_PAGE);
       expect(page.url()).toBe(server.EMPTY_PAGE);
 
-      const iframeUrl = server.PREFIX + '/frame.html';
       let requestFrame;
       page.on('request', r => {
-        if (r.url() === iframeUrl)
+        if (r.url() === server.PREFIX + '/frames/frame.html')
           requestFrame = r.frame();
       });
-      const response = await page.goto(server.PREFIX + '/one-frame.html');
+      const response = await page.goto(server.PREFIX + '/frames/one-frame.html');
-      expect(page.url()).toBe(server.PREFIX + '/one-frame.html');
+      expect(page.url()).toBe(server.PREFIX + '/frames/one-frame.html');
       expect(response.frame()).toBe(page.mainFrame());
-      expect(response.url()).toBe(server.PREFIX + '/one-frame.html');
+      expect(response.url()).toBe(server.PREFIX + '/frames/one-frame.html');
 
+      expect(page.frames().length).toBe(2);
       expect(requestFrame).toBe(page.frames()[1]);
     });
     it('should capture cross-process iframe navigation request', async({page, server}) => {
       await page.goto(server.EMPTY_PAGE);
       expect(page.url()).toBe(server.EMPTY_PAGE);
 
-      const iframeUrl = server.CROSS_PROCESS_PREFIX + '/frame.html';
       let requestFrame;
       page.on('request', r => {
-        if (r.url() === iframeUrl)
+        if (r.url() === server.CROSS_PROCESS_PREFIX + '/frames/frame.html')
           requestFrame = r.frame();
       });
-      const response = await page.goto(server.CROSS_PROCESS_PREFIX + '/one-frame.html');
+      const response = await page.goto(server.CROSS_PROCESS_PREFIX + '/frames/one-frame.html');
-      expect(page.url()).toBe(server.CROSS_PROCESS_PREFIX + '/one-frame.html');
+      expect(page.url()).toBe(server.CROSS_PROCESS_PREFIX + '/frames/one-frame.html');
       expect(response.frame()).toBe(page.mainFrame());
-      expect(response.url()).toBe(server.CROSS_PROCESS_PREFIX + '/one-frame.html');
+      expect(response.url()).toBe(server.CROSS_PROCESS_PREFIX + '/frames/one-frame.html');
 
+      expect(page.frames().length).toBe(2);
       expect(requestFrame).toBe(page.frames()[1]);
     });
     it('should work with anchor navigation', async({page, server}) => {