fix(screencast): guard frame callback and cleanup listeners

packages/screencast/src/index.js

@@ -3,16 +3,28 @@
 module.exports = (page, opts) => {
   const cdp = page._client()
   let onFrame
+  let isStopped = false
 
-  cdp.on('Page.screencastFrame', ({ data, metadata, sessionId }) => {
+  const onScreencastFrame = ({ data, metadata, sessionId }) => {
     cdp.send('Page.screencastFrameAck', { sessionId }).catch(() => {})
-    if (metadata.timestamp) onFrame(data, metadata)
-  })
+    if (metadata.timestamp && typeof onFrame === 'function') onFrame(data, metadata)
+  }
+
+  cdp.on('Page.screencastFrame', onScreencastFrame)
 
   return {
     // https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-startScreencast
     start: () => cdp.send('Page.startScreencast', opts),
     onFrame: fn => (onFrame = fn),
-    stop: () => cdp.send('Page.stopScreencast').catch(() => {})
+    stop: () => {
+      if (isStopped) return Promise.resolve()
+      isStopped = true
+      if (typeof cdp.off === 'function') {
+        cdp.off('Page.screencastFrame', onScreencastFrame)
+      } else if (typeof cdp.removeListener === 'function') {
+        cdp.removeListener('Page.screencastFrame', onScreencastFrame)
+      }
+      return cdp.send('Page.stopScreencast').catch(() => {})
+    }
   }
 }

packages/screencast/test/unit.js

@@ -0,0 +1,69 @@
+'use strict'
+
+const { EventEmitter } = require('events')
+const test = require('ava')
+
+const createScreencast = require('..')
+
+test('it does not throw if frame arrives before onFrame is set', async t => {
+  const cdp = new EventEmitter()
+  const calls = []
+
+  cdp.send = async (method, payload) => {
+    calls.push({ method, payload })
+  }
+
+  const page = {
+    _client: () => cdp
+  }
+
+  const screencast = createScreencast(page, {})
+
+  t.notThrows(() => {
+    cdp.emit('Page.screencastFrame', {
+      data: 'frame',
+      metadata: { timestamp: 1 },
+      sessionId: 42
+    })
+  })
+
+  await Promise.resolve()
+  t.true(calls.some(({ method }) => method === 'Page.screencastFrameAck'))
+
+  let frameCalls = 0
+  screencast.onFrame(() => {
+    frameCalls += 1
+  })
+
+  cdp.emit('Page.screencastFrame', {
+    data: 'frame',
+    metadata: { timestamp: 2 },
+    sessionId: 43
+  })
+
+  t.is(frameCalls, 1)
+})
+
+test('stop removes screencast listener and is idempotent', async t => {
+  const cdp = new EventEmitter()
+  const calls = []
+
+  cdp.send = async (method, payload) => {
+    calls.push({ method, payload })
+  }
+
+  const page = {
+    _client: () => cdp
+  }
+
+  const screencast = createScreencast(page, {})
+  t.is(cdp.listenerCount('Page.screencastFrame'), 1)
+
+  await screencast.stop()
+  t.is(cdp.listenerCount('Page.screencastFrame'), 0)
+
+  await screencast.stop()
+  t.is(cdp.listenerCount('Page.screencastFrame'), 0)
+
+  t.is(calls.filter(({ method }) => method === 'Page.stopScreencast').length, 1)
+})