feat(downloads): subsribe to download events in Browser domain instea…

…d of Page

src/server/chromium/crBrowser.ts

@@ -53,7 +53,10 @@ export class CRBrowser extends Browser {
     if ((options as any).__testHookOnConnectToBrowser)
       await (options as any).__testHookOnConnectToBrowser();
 
-    const version = await session.send('Browser.getVersion');
+    const [version] = await Promise.all([
+      session.send('Browser.getVersion'),
+      session.send('Browser.enable'),
+    ]);
     browser._isMac = version.userAgent.includes('Macintosh');
     browser._version = version.product.substring(version.product.indexOf('/') + 1);
     if (!options.persistent) {
@@ -77,6 +80,8 @@ export class CRBrowser extends Browser {
     this._connection.on(ConnectionEvents.Disconnected, () => this._didClose());
     this._session.on('Target.attachedToTarget', this._onAttachedToTarget.bind(this));
     this._session.on('Target.detachedFromTarget', this._onDetachedFromTarget.bind(this));
+    this._session.on('Browser.downloadWillBegin', this._onDownloadWillBegin.bind(this));
+    this._session.on('Browser.downloadProgress', this._onDownloadProgress.bind(this));
   }
 
   async newContext(options: types.BrowserContextOptions): Promise<BrowserContext> {
@@ -180,6 +185,27 @@ export class CRBrowser extends Browser {
     }
   }
 
+  _onDownloadWillBegin(payload: any) {
+    const page = this._crPages.get(payload.pageId);
+    assert(page, 'Download started in unknown page: ' + JSON.stringify(payload));
+    page.willBeginDownload();
+
+    let originPage = page._initializedPage;
+    // If it's a new window download, report it on the opener page.
+    if (!originPage && page._opener)
+      originPage = page._opener._initializedPage;
+    if (!originPage)
+      return;
+    this._downloadCreated(originPage, payload.guid, payload.url, payload.suggestedFilename);
+  }
+
+  _onDownloadProgress(payload: any) {
+    if (payload.state === 'completed')
+      this._downloadFinished(payload.guid, '');
+    if (payload.state === 'canceled')
+      this._downloadFinished(payload.guid, 'canceled');
+  }
+
   async _closePage(crPage: CRPage) {
     await this._session.send('Target.closeTarget', { targetId: crPage._targetId });
   }

src/server/chromium/crPage.ts

@@ -148,6 +148,10 @@ export class CRPage implements PageDelegate {
     return this._sessionForFrame(frame);
   }
 
+  willBeginDownload() {
+    this._mainFrameSession._willBeginDownload();
+  }
+
   async pageOrError(): Promise<Page | Error> {
     return this._pagePromise;
   }
@@ -400,8 +404,6 @@ class FrameSession {
   private _addBrowserListeners() {
     this._eventListeners.push(...[
       helper.addEventListener(this._client, 'Inspector.targetCrashed', event => this._onTargetCrashed()),
-      helper.addEventListener(this._client, 'Page.downloadWillBegin', event => this._onDownloadWillBegin(event)),
-      helper.addEventListener(this._client, 'Page.downloadProgress', event => this._onDownloadProgress(event)),
       helper.addEventListener(this._client, 'Page.screencastFrame', event => this._onScreencastFrame(event)),
       helper.addEventListener(this._client, 'Page.windowOpen', event => this._onWindowOpen(event)),
     ]);
@@ -799,26 +801,14 @@ class FrameSession {
     await this._page._onFileChooserOpened(handle);
   }
 
-  _onDownloadWillBegin(payload: Protocol.Page.downloadWillBeginPayload) {
+  _willBeginDownload() {
     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, payload.suggestedFilename);
-  }
-
-  _onDownloadProgress(payload: Protocol.Page.downloadProgressPayload) {
-    if (payload.state === 'completed')
-      this._crPage._browserContext._browser._downloadFinished(payload.guid, '');
-    if (payload.state === 'canceled')
-      this._crPage._browserContext._browser._downloadFinished(payload.guid, 'canceled');
   }
 
   _onScreencastFrame(payload: Protocol.Page.screencastFramePayload) {

tests/download.spec.ts

@@ -272,8 +272,8 @@ it.describe('download event', () => {
     await page.close();
   });
 
-  it('should report new window downloads', async ({browser, server, browserName, headful}) => {
-    it.fixme(browserName === 'chromium' && headful);
+  it('should report new window downloads', async ({browser, server, browserName, headful, browserChannel}) => {
+    it.fixme(browserName === 'chromium' && headful && !!browserChannel, 'Disabled until upstream fix hits stable');
 
     // TODO: - the test fails in headful Chromium as the popup page gets closed along
     // with the session before download completed event arrives.