refactor: rewire the desktop capturer API to remove race conditions

atom/browser/api/atom_api_desktop_capturer.cc

@@ -226,7 +226,7 @@ void Initialize(v8::Local<v8::Object> exports,
                 void* priv) {
   v8::Isolate* isolate = context->GetIsolate();
   mate::Dictionary dict(isolate, exports);
-  dict.Set("desktopCapturer", atom::api::DesktopCapturer::Create(isolate));
+  dict.SetMethod("createDesktopCapturer", &atom::api::DesktopCapturer::Create);
 }
 
 }  // namespace

lib/browser/desktop-capturer.js

@@ -2,13 +2,12 @@
 
 const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils')
 
-const { desktopCapturer } = process.electronBinding('desktop_capturer')
+const { createDesktopCapturer } = process.electronBinding('desktop_capturer')
 const eventBinding = process.electronBinding('event')
 
 const deepEqual = (a, b) => JSON.stringify(a) === JSON.stringify(b)
 
-// A queue for holding all requests from renderer process.
-let requestsQueue = []
+let currentlyRunning = []
 
 ipcMainUtils.handle('ELECTRON_BROWSER_DESKTOP_CAPTURER_GET_SOURCES', (event, captureWindow, captureScreen, thumbnailSize, fetchWindowIcons) => {
   const customEvent = eventBinding.createWithSender(event.sender)
@@ -18,70 +17,85 @@ ipcMainUtils.handle('ELECTRON_BROWSER_DESKTOP_CAPTURER_GET_SOURCES', (event, cap
     return []
   }
 
-  return new Promise((resolve, reject) => {
+  const options = {
+    captureWindow,
+    captureScreen,
+    thumbnailSize,
+    fetchWindowIcons
+  }
+
+  for (const running of currentlyRunning) {
+    if (deepEqual(running.options, options)) {
+      // If a request is currently running for the same options
+      // return that promise
+      return running.getSources
+    }
+  }
+
+  const getSources = new Promise((resolve, reject) => {
+    const stopRunning = () => {
+      // Remove from currentlyRunning once we resolve or reject
+      currentlyRunning = currentlyRunning.filter(running => running.options !== options)
+    }
     const request = {
-      options: {
-        captureWindow,
-        captureScreen,
-        thumbnailSize,
-        fetchWindowIcons
+      options,
+      resolve: (value) => {
+        stopRunning()
+        resolve(value)
       },
-      resolve,
-      reject
-    }
-    requestsQueue.push(request)
-    if (requestsQueue.length === 1) {
-      desktopCapturer.startHandling(captureWindow, captureScreen, thumbnailSize, fetchWindowIcons)
+      reject: (err) => {
+        stopRunning()
+        reject(err)
+      },
+      capturer: createDesktopCapturer()
     }
+    request.capturer.emit = createCapturerEmitHandler(request.capturer, request)
+    request.capturer.startHandling(captureWindow, captureScreen, thumbnailSize, fetchWindowIcons)
 
     // If the WebContents is destroyed before receiving result, just remove the
-    // reference from requestsQueue to make the module not send the result to it.
+    // reference to resolve, emit and the capturer itself so that it never dispatches
+    // back to the renderer
     event.sender.once('destroyed', () => {
       request.resolve = null
+      delete request.capturer.emit
+      delete request.capturer
+      stopRunning()
     })
   })
-})
 
-desktopCapturer.emit = (event, name, sources, fetchWindowIcons) => {
-  // Receiving sources result from main process, now send them back to renderer.
-  const handledRequest = requestsQueue.shift()
-  const unhandledRequestsQueue = []
+  currentlyRunning.push({
+    options,
+    getSources
+  })
+
+  return getSources
+})
 
-  if (name === 'error') {
-    const error = sources
-    handledRequest.reject(error)
-    return
-  }
+const createCapturerEmitHandler = (capturer, request) => {
+  return function handlEmitOnCapturer (event, name, sources, fetchWindowIcons) {
+    // Ensure that this capturer instance can only ever receive a single event
+    // if we get more than one it is a bug but will also cause strange behavior
+    // if we still try to handle it
+    delete capturer.emit
 
-  const result = sources.map(source => {
-    return {
-      id: source.id,
-      name: source.name,
-      thumbnail: source.thumbnail.toDataURL(),
-      display_id: source.display_id,
-      appIcon: (fetchWindowIcons && source.appIcon) ? source.appIcon.toDataURL() : null
+    if (name === 'error') {
+      const error = sources
+      request.reject(error)
+      return
     }
-  })
-
-  if (handledRequest.resolve) {
-    handledRequest.resolve(result)
-  }
 
-  // Check the queue to see whether there is another identical request & handle
-  requestsQueue.forEach(request => {
-    if (deepEqual(handledRequest.options, request.options)) {
-      if (request.resolve) {
-        request.resolve(result)
+    const result = sources.map(source => {
+      return {
+        id: source.id,
+        name: source.name,
+        thumbnail: source.thumbnail.toDataURL(),
+        display_id: source.display_id,
+        appIcon: (fetchWindowIcons && source.appIcon) ? source.appIcon.toDataURL() : null
       }
-    } else {
-      unhandledRequestsQueue.push(request)
-    }
-  })
-  requestsQueue = unhandledRequestsQueue
+    })
 
-  // If the requestsQueue is not empty, start a new request handling.
-  if (requestsQueue.length > 0) {
-    const { captureWindow, captureScreen, thumbnailSize, fetchWindowIcons } = requestsQueue[0].options
-    return desktopCapturer.startHandling(captureWindow, captureScreen, thumbnailSize, fetchWindowIcons)
+    if (request.resolve) {
+      request.resolve(result)
+    }
   }
 }