fix: report new window downloads (#2019)

Author: yury-s

browsers.json

@@ -6,7 +6,7 @@
     },
     {
       "name": "firefox",
-      "revision": "1087"
+      "revision": "1088"
     },
     {
       "name": "webkit",

packages/playwright-firefox/browsers.json

@@ -2,7 +2,7 @@
   "browsers": [
     {
       "name": "firefox",
-      "revision": "1087"
+      "revision": "1088"
     }
   ]
 }

packages/playwright/browsers.json

@@ -6,7 +6,7 @@
     },
     {
       "name": "firefox",
-      "revision": "1087"
+      "revision": "1088"
     },
     {
       "name": "webkit",

src/chromium/crPage.ts

@@ -49,7 +49,7 @@ export class CRPage implements PageDelegate {
   readonly rawMouse: RawMouseImpl;
   readonly rawKeyboard: RawKeyboardImpl;
   readonly _targetId: string;
-  private readonly _opener: CRPage | null;
+  readonly _opener: CRPage | null;
   private readonly _pdf: CRPDF;
   private readonly _coverage: CRCoverage;
   readonly _browserContext: CRBrowserContext;
@@ -654,7 +654,18 @@ class FrameSession {
   }
 
   _onDownloadWillBegin(payload: Protocol.Page.downloadWillBeginPayload) {
-    this._crPage._browserContext._browser._downloadCreated(this._page, payload.guid, payload.url);
+    let originPage = this._crPage._initializedPage;
+    // If it's a new window download, report it on the opener page.
+    if (!originPage) {
+      // Resume the page creation with an error. The page will automatically close right
+      // after the download begins.
+      this._firstNonInitialNavigationCommittedReject(new Error('Starting new page download'));
+      if (this._crPage._opener)
+        originPage = this._crPage._opener._initializedPage;
+    }
+    if (!originPage)
+      return;
+    this._crPage._browserContext._browser._downloadCreated(originPage, payload.guid, payload.url);
   }
 
   _onDownloadProgress(payload: Protocol.Page.downloadProgressPayload) {

src/firefox/ffBrowser.ts

@@ -153,7 +153,18 @@ export class FFBrowser extends BrowserBase {
     assert(ffPage);
     if (!ffPage)
       return;
-    this._downloadCreated(ffPage._page, payload.uuid, payload.url);
+    let originPage = ffPage._initializedPage;
+    // If it's a new window download, report it on the opener page.
+    if (!originPage) {
+      // Resume the page creation with an error. The page will automatically close right
+      // after the download begins.
+      ffPage._pageCallback(new Error('Starting new page download'));
+      if (ffPage._opener)
+        originPage = ffPage._opener._initializedPage;
+    }
+    if (!originPage)
+      return;
+    this._downloadCreated(originPage, payload.uuid, payload.url);
   }
 
   _onDownloadFinished(payload: Protocol.Browser.downloadFinishedPayload) {

src/firefox/ffPage.ts

@@ -45,9 +45,9 @@ export class FFPage implements PageDelegate {
   readonly _networkManager: FFNetworkManager;
   readonly _browserContext: FFBrowserContext;
   private _pagePromise: Promise<Page | Error>;
-  private _pageCallback: (pageOrError: Page | Error) => void = () => {};
+  _pageCallback: (pageOrError: Page | Error) => void = () => {};
   _initializedPage: Page | null = null;
-  private readonly _opener: FFPage | null;
+  readonly _opener: FFPage | null;
   private readonly _contextIdToContext: Map<string, dom.FrameExecutionContext>;
   private _eventListeners: RegisteredListener[];
   private _workers = new Map<string, { frameId: string, session: FFSession }>();

src/webkit/wkBrowser.ts

@@ -107,7 +107,18 @@ export class WKBrowser extends BrowserBase {
       // here by simulating cancelled provisional load which matches downloads from network.
       frameManager.provisionalLoadFailed(frame, '', 'Download is starting');
     }
-    this._downloadCreated(page._page, payload.uuid, payload.url);
+    let originPage = page._initializedPage;
+    // If it's a new window download, report it on the opener page.
+    if (!originPage) {
+      // Resume the page creation with an error. The page will automatically close right
+      // after the download begins.
+      page._firstNonInitialNavigationCommittedReject(new Error('Starting new page download'));
+      if (page._opener)
+        originPage = page._opener._initializedPage;
+    }
+    if (!originPage)
+      return;
+    this._downloadCreated(originPage, payload.uuid, payload.url);
   }
 
   _onDownloadFinished(payload: Protocol.Playwright.downloadFinishedPayload) {

src/webkit/wkPage.ts

@@ -53,7 +53,7 @@ export class WKPage implements PageDelegate {
   private readonly _pagePromise: Promise<Page | Error>;
   private _pagePromiseCallback: (page: Page | Error) => void = () => {};
   private readonly _pageProxySession: WKSession;
-  private readonly _opener: WKPage | null;
+  readonly _opener: WKPage | null;
   private readonly _requestIdToRequest = new Map<string, WKInterceptableRequest>();
   private readonly _workers: WKWorkers;
   private readonly _contextIdToContext: Map<number, dom.FrameExecutionContext>;
@@ -65,7 +65,7 @@ export class WKPage implements PageDelegate {
   _initializedPage: Page | null = null;
   private _firstNonInitialNavigationCommittedPromise: Promise<void>;
   private _firstNonInitialNavigationCommittedFulfill = () => {};
-  private _firstNonInitialNavigationCommittedReject = (e: Error) => {};
+  _firstNonInitialNavigationCommittedReject = (e: Error) => {};
   private _lastConsoleMessage: { derivedType: string, text: string, handles: JSHandle[]; count: number, location: ConsoleMessageLocation; } | null = null;
 
   constructor(browserContext: WKBrowserContext, pageProxySession: WKSession, opener: WKPage | null) {

test/download.spec.js

@@ -117,7 +117,10 @@ describe('Download', function() {
     expect(fs.readFileSync(path).toString()).toBe('Hello world');
     await page.close();
   });
-  it.fail(CHROMIUM || WEBKIT || FFOX)('should report new window downloads', async({browser, server}) => {
+  it('should report new window downloads', async({browser, server}) => {
+    // TODO: - the test fails in headful Chromium as the popup page gets closed along
+    // with the session before download completed event arrives.
+    // - WebKit doesn't close the popup page
     const page = await browser.newPage({ acceptDownloads: true });
     await page.setContent(`<a target=_blank href="${server.PREFIX}/download">download</a>`);
     const [ download ] = await Promise.all([