fix(binding): dispatch binding after the page has been initialized (#2938)

... but not after it was closed.

Author: dgozman

src/chromium/crPage.ts

@@ -643,9 +643,11 @@ class FrameSession {
     ]);
   }
 
-  _onBindingCalled(event: Protocol.Runtime.bindingCalledPayload) {
+  async _onBindingCalled(event: Protocol.Runtime.bindingCalledPayload) {
     const context = this._contextIdToContext.get(event.executionContextId)!;
-    this._page._onBindingCalled(event.payload, context);
+    const pageOrError = await this._crPage.pageOrError();
+    if (!(pageOrError instanceof Error))
+      this._page._onBindingCalled(event.payload, context);
   }
 
   _onDialog(event: Protocol.Page.javascriptDialogOpeningPayload) {

src/firefox/ffPage.ts

@@ -195,9 +195,11 @@ export class FFPage implements PageDelegate {
         params.defaultValue));
   }
 
-  _onBindingCalled(event: Protocol.Page.bindingCalledPayload) {
+  async _onBindingCalled(event: Protocol.Page.bindingCalledPayload) {
     const context = this._contextIdToContext.get(event.executionContextId)!;
-    this._page._onBindingCalled(event.payload, context);
+    const pageOrError = await this.pageOrError();
+    if (!(pageOrError instanceof Error))
+      this._page._onBindingCalled(event.payload, context);
   }
 
   async _onFileChooserOpened(payload: Protocol.Page.fileChooserOpenedPayload) {

src/page.ts

@@ -293,6 +293,8 @@ export class Page extends EventEmitter {
   }
 
   async _onBindingCalled(payload: string, context: dom.FrameExecutionContext) {
+    if (this._disconnected || this._closedState === 'closed')
+      return;
     await PageBinding.dispatch(this, payload, context);
   }
 

src/webkit/wkPage.ts

@@ -455,7 +455,10 @@ export class WKPage implements PageDelegate {
     if (level === 'debug' && parameters && parameters[0].value === BINDING_CALL_MESSAGE) {
       const parsedObjectId = JSON.parse(parameters[1].objectId!);
       const context = this._contextIdToContext.get(parsedObjectId.injectedScriptId)!;
-      this._page._onBindingCalled(parameters[2].value, context);
+      this.pageOrError().then(pageOrError => {
+        if (!(pageOrError instanceof Error))
+          this._page._onBindingCalled(parameters[2].value, context);
+      });
       return;
     }
     if (level === 'error' && source === 'javascript') {

test/browsercontext.spec.js

@@ -131,7 +131,7 @@ describe('BrowserContext', function() {
   });
   it('should not report frameless pages on error', async({browser, server}) => {
     const context = await browser.newContext();
-    page = await context.newPage();
+    const page = await context.newPage();
     server.setRoute('/empty.html', (req, res) => {
       res.end(`<a href="${server.EMPTY_PAGE}" target="_blank">Click me</a>`);
     });
@@ -145,6 +145,22 @@ describe('BrowserContext', function() {
       expect(popup.isClosed()).toBeTruthy();
       expect(popup.mainFrame()).toBeTruthy();
     }
+  });
+  it('should not call binding on errored pages', async({browser, server}) => {
+    const context = await browser.newContext();
+    let gotBinding = 0;
+    await context.exposeFunction('add', (a, b) => {
+      gotBinding++;
+      return a + b;
+    });
+    const page = await context.newPage();
+    server.setRoute('/empty.html', (req, res) => {
+      res.end(`<script>window.add(2, 3)</script><a href="${server.EMPTY_PAGE}" target="_blank">Click me</a>`);
+    });
+    await page.goto(server.EMPTY_PAGE);
+    await page.click('"Click me"');
+    await context.close();
+    expect(gotBinding).toBe(1);
   })
 });
 

test/popup.spec.js

@@ -200,18 +200,51 @@ describe('window.open', function() {
     expect(await popup.evaluate('injected')).toBe(123);
     await context.close();
   });
-
   it('should expose function from browser context', async function({browser, server}) {
     const context = await browser.newContext();
-    await context.exposeFunction('add', (a, b) => a + b);
+    const messages = [];
+    await context.exposeFunction('add', (a, b) => {
+      messages.push('binding');
+      return a + b;
+    });
     const page = await context.newPage();
+    context.on('page', () => messages.push('page'));
     await page.goto(server.EMPTY_PAGE);
     const added = await page.evaluate(async () => {
       const win = window.open('about:blank');
       return win.add(9, 4);
     });
     await context.close();
     expect(added).toBe(13);
+    expect(messages.join('|')).toBe('page|binding');
+  });
+  it('should not dispatch binding on a closed page', async function({browser, server}) {
+    const context = await browser.newContext();
+    const messages = [];
+    await context.exposeFunction('add', (a, b) => {
+      messages.push('binding');
+      return a + b;
+    });
+    const page = await context.newPage();
+    await page.goto(server.EMPTY_PAGE);
+    await Promise.all([
+      page.waitForEvent('popup').then(popup => {
+        if (popup.isClosed())
+          messages.push('alreadyclosed');
+        else
+          return popup.waitForEvent('close').then(() => messages.push('close'));
+      }),
+      page.evaluate(async () => {
+        const win = window.open('about:blank');
+        win.add(9, 4);
+        win.close();
+      }),
+    ]);
+    await context.close();
+    if (FFOX)
+      expect(messages.join('|')).toBe('alreadyclosed');
+    else
+      expect(messages.join('|')).toBe('binding|close');
   });
 });
 
@@ -252,7 +285,7 @@ describe('Page.Events.Popup', function() {
     expect(popup).toBeTruthy();
     await context.close();
   });
-  it('should emit for immediately closed popups', async({browser, server}) => {
+  it('should emit for immediately closed popups 2', async({browser, server}) => {
     const context = await browser.newContext();
     const page = await context.newPage();
     await page.goto(server.EMPTY_PAGE);