Allow web frame methods to return async promises

[MarshallOfSound]

Fixes #7532

Basically this handles executeJavaScript calls returning instances of Promise and resolving that promise before passing it back through IPC to the caller.

This trick works because

Promise.resolve(123).then((resolvedValue) => console.log(resolvedValue))
// This would print out 123
// Promise.resolve either immediately resolves with the given value or resolves the given promise

It allows for truly asyncronous execution like so

webContents.executeJavaScript(
  `new Promise(resolve => {
    setTimeout(() => resolve('myString'), 2000);
  });`,
  (returnValue) => {
     // returnValue === 'myString'
  }
);

This is non-blocking async execution 👍

[kevinsawicki]

Would be great to get some spec coverage for this case.

[MarshallOfSound]

@kevinsawicki Added an async spec 👍

[anaisbetts]

This Feels Bad, shouldn't it rethrow on the other side?

[MarshallOfSound]

I'm not sure how it can rethrow. Because it is async you can't throw Error as that would be uncatchable. And the standard callback syntax (err, result) isn't in use here so switching to that would be a massive breaking change for anyone using executeJavaScript.

We also currently allow syncronous errors to just fly through as uncaught exceptions in the renderer process so not sure how this is much different 👍

If we're OK with making the breaking change to the callback I'm happy to do that

[anaisbetts]

It seems like you could serialize the error, send it over the wire, then return a Promise.reject(new Error(thatError)), no?

[MarshallOfSound]

So we want a callback that returns a rejected promise. I was under the impression that Promises didn't transition well across IPC though. I'll see if I can come up with a simple / nice way of doing this 👍

[MarshallOfSound]

@paulcbetts I just modified the code so that the callback syntax remains the same but the executeJavaScript method now returns a promise that resolves the the result and rejects with any thrown error. Let me know if that works for you 👍

[kevinsawicki]

Missing . at end of sentence

[kevinsawicki]

Maybe we should switch these two new code strings and the existing code variable to use template strings for clarity.

[kevinsawicki]

Hmm, so this event listener will hang around forever if the promise is never rejected, and the other one will hang around forever if the promise is rejected right?

Maybe we can use ELECTRON_INTERNAL_BROWSER_ASYNC_WEB_FRAME_RESPONSE_ for both cases and just have two arguments (event, result, error) and reject when error is present and resolve otherwise with result.

[MarshallOfSound]

We could also (and probably should anyway) remove the event listener with .removeListener once we have recieved the first response / first error

[MarshallOfSound]

@kevinsawicki Just realized I hadn't updated this 😆

I think I addressed your comments 👍

[kevinsawicki]

Thanks for this @MarshallOfSound 👍