fix(har): save popup's main request/response (#6562)

This migrates server side code from networks events on the Page
to network events on the BrowserContext.

Author: dgozman

src/server/frames.ts

@@ -271,7 +271,6 @@ export class FrameManager {
     if (response.request()._isFavicon)
       return;
     this._responses.push(response);
-    this._page.emit(Page.Events.Response, response);
     this._page._browserContext.emit(BrowserContext.Events.Response, response);
   }
 
@@ -280,7 +279,6 @@ export class FrameManager {
     if (request._isFavicon)
       return;
     this._page._browserContext.emit(BrowserContext.Events.RequestFinished, request);
-    this._page.emit(Page.Events.RequestFinished, request);
   }
 
   requestFailed(request: network.Request, canceled: boolean) {
@@ -295,7 +293,6 @@ export class FrameManager {
     if (request._isFavicon)
       return;
     this._page._browserContext.emit(BrowserContext.Events.RequestFailed, request);
-    this._page.emit(Page.Events.RequestFailed, request);
   }
 
   removeChildFramesRecursively(frame: Frame) {

src/server/page.ts

@@ -104,10 +104,6 @@ export class Page extends SdkObject {
     // Can't use just 'error' due to node.js special treatment of error events.
     // @see https://nodejs.org/api/events.html#events_error_events
     PageError: 'pageerror',
-    Request: 'request',
-    Response: 'response',
-    RequestFailed: 'requestfailed',
-    RequestFinished: 'requestfinished',
     FrameAttached: 'frameattached',
     FrameDetached: 'framedetached',
     InternalFrameNavigatedToNewDocument: 'internalframenavigatedtonewdocument',
@@ -411,7 +407,6 @@ export class Page extends SdkObject {
   }
 
   _requestStarted(request: network.Request) {
-    this.emit(Page.Events.Request, request);
     const route = request._route();
     if (!route)
       return;

src/server/snapshot/snapshotter.ts

@@ -67,7 +67,7 @@ export class Snapshotter {
     // Replay resources loaded in all pages.
     for (const page of this._context.pages()) {
       for (const response of page._frameManager._responses)
-        this._saveResource(page, response).catch(e => debugLogger.log('error', e));
+        this._saveResource(response).catch(e => debugLogger.log('error', e));
     }
   }
 
@@ -80,6 +80,9 @@ export class Snapshotter {
       this._onPage(page);
     this._eventListeners = [
       helper.addEventListener(this._context, BrowserContext.Events.Page, this._onPage.bind(this)),
+      helper.addEventListener(this._context, BrowserContext.Events.Response, (response: network.Response) => {
+        this._saveResource(response).catch(e => debugLogger.log('error', e));
+      }),
     ];
 
     const initScript = `(${frameSnapshotStreamer})("${this._snapshotStreamer}")`;
@@ -149,13 +152,9 @@ export class Snapshotter {
     for (const frame of page.frames())
       this._annotateFrameHierarchy(frame);
     this._eventListeners.push(helper.addEventListener(page, Page.Events.FrameAttached, frame => this._annotateFrameHierarchy(frame)));
-
-    this._eventListeners.push(helper.addEventListener(page, Page.Events.Response, (response: network.Response) => {
-      this._saveResource(page, response).catch(e => debugLogger.log('error', e));
-    }));
   }
 
-  private async _saveResource(page: Page, response: network.Response) {
+  private async _saveResource(response: network.Response) {
     if (!this._started)
       return;
     const isRedirect = response.status() >= 300 && response.status() <= 399;
@@ -198,7 +197,7 @@ export class Snapshotter {
     }
 
     const resource: ResourceSnapshot = {
-      pageId: page.guid,
+      pageId: response.frame()._page.guid,
       frameId: response.frame().guid,
       resourceId: 'resource@' + createGuid(),
       url,

src/server/supplements/har/harTracer.ts

@@ -52,48 +52,58 @@ export class HarTracer {
       pages: [],
       entries: []
     };
-    context.on(BrowserContext.Events.Page, page => this._onPage(page));
+    context.on(BrowserContext.Events.Page, (page: Page) => this._ensurePageEntry(page));
+    context.on(BrowserContext.Events.Request, (request: network.Request) => this._onRequest(request));
+    context.on(BrowserContext.Events.Response, (response: network.Response) => this._onResponse(response));
   }
 
-  private _onPage(page: Page) {
-    const pageEntry: har.Page = {
-      startedDateTime: new Date(),
-      id: `page_${this._lastPage++}`,
-      title: '',
-      pageTimings: {
-        onContentLoad: -1,
-        onLoad: -1,
-      },
-    };
-    this._pageEntries.set(page, pageEntry);
-    this._log.pages.push(pageEntry);
-    page.on(Page.Events.Request, (request: network.Request) => this._onRequest(page, request));
-    page.on(Page.Events.Response, (response: network.Response) => this._onResponse(page, response));
+  private _ensurePageEntry(page: Page) {
+    let pageEntry = this._pageEntries.get(page);
+    if (!pageEntry) {
+      page.on(Page.Events.DOMContentLoaded, () => this._onDOMContentLoaded(page));
+      page.on(Page.Events.Load, () => this._onLoad(page));
 
-    page.on(Page.Events.DOMContentLoaded, () => {
-      const promise = page.mainFrame().evaluateExpression(String(() => {
-        return {
-          title: document.title,
-          domContentLoaded: performance.timing.domContentLoadedEventStart,
-        };
-      }), true, undefined, 'utility').then(result => {
-        pageEntry.title = result.title;
-        pageEntry.pageTimings.onContentLoad = result.domContentLoaded;
-      }).catch(() => {});
-      this._addBarrier(page, promise);
-    });
-    page.on(Page.Events.Load, () => {
-      const promise = page.mainFrame().evaluateExpression(String(() => {
-        return {
-          title: document.title,
-          loaded: performance.timing.loadEventStart,
-        };
-      }), true, undefined, 'utility').then(result => {
-        pageEntry.title = result.title;
-        pageEntry.pageTimings.onLoad = result.loaded;
-      }).catch(() => {});
-      this._addBarrier(page, promise);
-    });
+      pageEntry = {
+        startedDateTime: new Date(),
+        id: `page_${this._lastPage++}`,
+        title: '',
+        pageTimings: {
+          onContentLoad: -1,
+          onLoad: -1,
+        },
+      };
+      this._pageEntries.set(page, pageEntry);
+      this._log.pages.push(pageEntry);
+    }
+    return pageEntry;
+  }
+
+  private _onDOMContentLoaded(page: Page) {
+    const pageEntry = this._ensurePageEntry(page);
+    const promise = page.mainFrame().evaluateExpression(String(() => {
+      return {
+        title: document.title,
+        domContentLoaded: performance.timing.domContentLoadedEventStart,
+      };
+    }), true, undefined, 'utility').then(result => {
+      pageEntry.title = result.title;
+      pageEntry.pageTimings.onContentLoad = result.domContentLoaded;
+    }).catch(() => {});
+    this._addBarrier(page, promise);
+  }
+
+  private _onLoad(page: Page) {
+    const pageEntry = this._ensurePageEntry(page);
+    const promise = page.mainFrame().evaluateExpression(String(() => {
+      return {
+        title: document.title,
+        loaded: performance.timing.loadEventStart,
+      };
+    }), true, undefined, 'utility').then(result => {
+      pageEntry.title = result.title;
+      pageEntry.pageTimings.onLoad = result.loaded;
+    }).catch(() => {});
+    this._addBarrier(page, promise);
   }
 
   private _addBarrier(page: Page, promise: Promise<void>) {
@@ -107,12 +117,13 @@ export class HarTracer {
     this._barrierPromises.add(race);
   }
 
-  private _onRequest(page: Page, request: network.Request) {
-    const pageEntry = this._pageEntries.get(page)!;
+  private _onRequest(request: network.Request) {
+    const page = request.frame()._page;
     const url = network.parsedURL(request.url());
     if (!url)
       return;
 
+    const pageEntry = this._ensurePageEntry(page);
     const harEntry: har.Entry = {
       pageref: pageEntry.id,
       startedDateTime: new Date(),
@@ -160,8 +171,9 @@ export class HarTracer {
     this._entries.set(request, harEntry);
   }
 
-  private _onResponse(page: Page, response: network.Response) {
-    const pageEntry = this._pageEntries.get(page)!;
+  private _onResponse(response: network.Response) {
+    const page = response.frame()._page;
+    const pageEntry = this._ensurePageEntry(page);
     const harEntry = this._entries.get(response.request())!;
     // Rewrite provisional headers with actual
     const request = response.request();

tests/har.spec.ts

@@ -258,3 +258,26 @@ it('should calculate time', async ({ contextFactory, server }, testInfo) => {
   const log = await getLog();
   expect(log.entries[0].time).toBeGreaterThan(0);
 });
+
+it('should have popup requests', async ({ contextFactory, server }, testInfo) => {
+  const { page, getLog } = await pageWithHar(contextFactory, testInfo);
+  await page.goto(server.EMPTY_PAGE);
+  await page.setContent('<a target=_blank rel=noopener href="/one-style.html">yo</a>');
+  const [popup] = await Promise.all([
+    page.waitForEvent('popup'),
+    page.click('a'),
+  ]);
+  await popup.waitForLoadState();
+  const log = await getLog();
+
+  expect(log.pages.length).toBe(2);
+  expect(log.pages[0].id).toBe('page_0');
+  expect(log.pages[1].id).toBe('page_1');
+
+  const entries = log.entries.filter(entry => entry.pageref === 'page_1');
+  expect(entries.length).toBe(2);
+  expect(entries[0].request.url).toBe(server.PREFIX + '/one-style.html');
+  expect(entries[0].response.status).toBe(200);
+  expect(entries[1].request.url).toBe(server.PREFIX + '/one-style.css');
+  expect(entries[1].response.status).toBe(200);
+});