refactor: rewire the desktop capturer API to remove race conditions (#18029)

We now create a new instance of atom::api::DesktopCapturer for every
request instead of weirdly re-using the same instance and queuing
requests.  This means there is now a 1:1 relationship between request
and DesktopCapturer so there isn't a race condition between the observer
for one request calling back before the observer of another.  This is an
issue ever since the backing APIs moved to worker threads.

This also does a few things to ensure memory management
* Only ever listen to one event per-request, after that we wipe the emit
function to ignore all future events
* Ensures we clean up the window_capturer_, screen_capturer_ and
captured_sources_ in native land once the request is over.

This _in theory_ fixes a flake we've been seeing on CI where we try to
resolve the promise for a request that no longerr exists.
This commit is contained in:
Samuel Attard 2019-04-29 13:21:28 -07:00 committed by GitHub
parent 8b79776b5e
commit 77a4946069
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 67 additions and 53 deletions

View file

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

View file

@ -2,13 +2,12 @@
const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils') 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 eventBinding = process.electronBinding('event')
const deepEqual = (a, b) => JSON.stringify(a) === JSON.stringify(b) const deepEqual = (a, b) => JSON.stringify(a) === JSON.stringify(b)
// A queue for holding all requests from renderer process. let currentlyRunning = []
let requestsQueue = []
ipcMainUtils.handle('ELECTRON_BROWSER_DESKTOP_CAPTURER_GET_SOURCES', (event, captureWindow, captureScreen, thumbnailSize, fetchWindowIcons) => { ipcMainUtils.handle('ELECTRON_BROWSER_DESKTOP_CAPTURER_GET_SOURCES', (event, captureWindow, captureScreen, thumbnailSize, fetchWindowIcons) => {
const customEvent = eventBinding.createWithSender(event.sender) const customEvent = eventBinding.createWithSender(event.sender)
@ -18,70 +17,85 @@ ipcMainUtils.handle('ELECTRON_BROWSER_DESKTOP_CAPTURER_GET_SOURCES', (event, cap
return [] 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 = { const request = {
options: { options,
captureWindow, resolve: (value) => {
captureScreen, stopRunning()
thumbnailSize, resolve(value)
fetchWindowIcons
}, },
resolve, reject: (err) => {
reject stopRunning()
} reject(err)
requestsQueue.push(request) },
if (requestsQueue.length === 1) { capturer: createDesktopCapturer()
desktopCapturer.startHandling(captureWindow, captureScreen, thumbnailSize, fetchWindowIcons)
} }
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 // 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', () => { event.sender.once('destroyed', () => {
request.resolve = null request.resolve = null
delete request.capturer.emit
delete request.capturer
stopRunning()
}) })
}) })
currentlyRunning.push({
options,
getSources
})
return getSources
}) })
desktopCapturer.emit = (event, name, sources, fetchWindowIcons) => { const createCapturerEmitHandler = (capturer, request) => {
// Receiving sources result from main process, now send them back to renderer. return function handlEmitOnCapturer (event, name, sources, fetchWindowIcons) {
const handledRequest = requestsQueue.shift() // Ensure that this capturer instance can only ever receive a single event
const unhandledRequestsQueue = [] // 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
if (name === 'error') { if (name === 'error') {
const error = sources const error = sources
handledRequest.reject(error) request.reject(error)
return return
}
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 (handledRequest.resolve) { const result = sources.map(source => {
handledRequest.resolve(result) return {
} id: source.id,
name: source.name,
// Check the queue to see whether there is another identical request & handle thumbnail: source.thumbnail.toDataURL(),
requestsQueue.forEach(request => { display_id: source.display_id,
if (deepEqual(handledRequest.options, request.options)) { appIcon: (fetchWindowIcons && source.appIcon) ? source.appIcon.toDataURL() : null
if (request.resolve) {
request.resolve(result)
} }
} else { })
unhandledRequestsQueue.push(request)
}
})
requestsQueue = unhandledRequestsQueue
// If the requestsQueue is not empty, start a new request handling. if (request.resolve) {
if (requestsQueue.length > 0) { request.resolve(result)
const { captureWindow, captureScreen, thumbnailSize, fetchWindowIcons } = requestsQueue[0].options }
return desktopCapturer.startHandling(captureWindow, captureScreen, thumbnailSize, fetchWindowIcons)
} }
} }