fix: report new window downloads

microsoft · Apr 28, 2020 · 4f65550 · 4f65550
1 parent c7b2c87
commit 4f65550
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 8 deletions.
diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts
@@ -649,8 +649,18 @@ class FrameSession {
     this._page._onFileChooserOpened(handle);
   }
 
-  _onDownloadWillBegin(payload: Protocol.Page.downloadWillBeginPayload) {
-    this._crPage._browserContext._browser._downloadCreated(this._page, payload.guid, payload.url);
+  async _onDownloadWillBegin(payload: Protocol.Page.downloadWillBeginPayload) {
+    let originPage = this._crPage._initializedPage;
+    // If it's a new window download, report it on the opener page.
+    if (!originPage) {
+      originPage = await this._crPage.opener();
+      // 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 (!originPage)
+      return;
+    this._crPage._browserContext._browser._downloadCreated(originPage, payload.guid, payload.url);
   }
 
   _onDownloadProgress(payload: Protocol.Page.downloadProgressPayload) {

diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts
@@ -148,12 +148,22 @@ export class FFBrowser extends BrowserBase {
     });
   }
 
-  _onDownloadCreated(payload: Protocol.Browser.downloadCreatedPayload) {
+  async _onDownloadCreated(payload: Protocol.Browser.downloadCreatedPayload) {
     const ffPage = this._ffPages.get(payload.pageTargetId)!;
     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) {
+      originPage = await ffPage.opener();
+      // 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 (!originPage)
+      return;
+    this._downloadCreated(originPage, payload.uuid, payload.url);
   }
 
   _onDownloadFinished(payload: Protocol.Browser.downloadFinishedPayload) {

diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts
@@ -45,7 +45,7 @@ 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;
   private readonly _contextIdToContext: Map<string, dom.FrameExecutionContext>;

diff --git a/src/page.ts b/src/page.ts
@@ -538,6 +538,10 @@ export class Page extends ExtendedEventEmitter implements InnerLogger {
   _log(log: Log, message: string | Error, ...args: any[]) {
     return this._browserContext._log(log, message, ...args);
   }
+
+  toString(): string {
+    return `[Page ${this.url()}]`;
+  }
 }
 
 export class Worker extends EventEmitter {

diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts
@@ -95,7 +95,7 @@ export class WKBrowser extends BrowserBase {
     return this._firstPagePromise;
   }
 
-  _onDownloadCreated(payload: Protocol.Playwright.downloadCreatedPayload) {
+  async _onDownloadCreated(payload: Protocol.Playwright.downloadCreatedPayload) {
     const page = this._wkPages.get(payload.pageProxyId);
     if (!page)
       return;
@@ -107,7 +107,13 @@ 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)
+      originPage = await page.opener();
+    if (!originPage)
+      return;
+    this._downloadCreated(originPage, payload.uuid, payload.url);
   }
 
   _onDownloadFinished(payload: Protocol.Playwright.downloadFinishedPayload) {

diff --git a/test/download.spec.js b/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([