Javascript Binding - IJavascriptCallbackProxy.ExecuteAsync correctly creates pending task using callback Id

- Previously a new PendingTask(TaskCompletionSource) was created using a new Id which worked when the Ids matched
  between the browser and render process. If you skipped executing a callback then the Ids would get out of sync.

Issue #3979

Author: amaitland

CefSharp.Core.Runtime/Internals/ClientAdapter.cpp

@@ -1393,7 +1393,10 @@ namespace CefSharp
                 auto success = argList->GetBool(0);
                 auto callbackId = GetInt64(argList, 1);
 
-                auto pendingTask = _pendingTaskRepository->RemovePendingTask(callbackId);
+                auto pendingTask = name == kEvaluateJavascriptResponse ?
+                    _pendingTaskRepository->RemovePendingTask(callbackId) :
+                    _pendingTaskRepository->RemoveJavascriptCallbackPendingTask(callbackId);
+
                 if (pendingTask != nullptr)
                 {
                     auto response = gcnew JavascriptResponse();

CefSharp.Core.Runtime/Internals/JavascriptCallbackProxy.cpp

@@ -33,7 +33,7 @@ namespace CefSharp
             auto browserWrapper = static_cast<CefBrowserWrapper^>(browser);
             auto javascriptNameConverter = GetJavascriptNameConverter();
 
-            auto doneCallback = _pendingTasks->CreatePendingTask(timeout);
+            auto doneCallback = _pendingTasks->CreateJavascriptCallbackPendingTask(_callback->Id, timeout);
 
             auto callbackMessage = CefProcessMessage::Create(kJavascriptCallbackRequest);
             auto argList = callbackMessage->GetArgumentList();

CefSharp.Example/JavascriptBinding/AsyncBoundObject.cs

@@ -256,5 +256,20 @@ public uint UIntAdd(uint paramA, uint paramB)
         {
             return paramA + paramB;
         }
+
+        public async Task<string> JavascriptOptionalCallbackEvalPromise(bool invokeCallback, string msg, IJavascriptCallback callback)
+        {
+            using (callback)
+            {
+                if (invokeCallback)
+                {
+                    var response = await callback.ExecuteAsync(callback.Id, msg).ConfigureAwait(false);
+                    //Echo the response
+                    return (string)response.Result;
+                }
+
+                return msg;
+            }
+        }
     }
 }

CefSharp.Example/Resources/BindingTestSingle.html

@@ -17,12 +17,60 @@
             await CefSharp.BindObjectAsync("boundAsync");
         });
 
-        QUnit.test("Async call (Divide 16 / 2):", async (assert) =>
+        QUnit.test("Pass callback(promise) that is conditionally invoked:", async (assert) =>
         {
-            const expectedResult = 8
-            const actualResult = await boundAsync.div(16, 2);
+            let convertPromiseToCefSharpCallback = function (p)
+            {
+                let f = function (callbackId, ...args)
+                {
+                    //We immediately return CefSharpDefEvalScriptRes as we will be
+                    //using a promise and will call sendEvalScriptResponse when our
+                    //promise has completed
+                    (async function ()
+                    {
+                        try
+                        {
+                            //Await the promise
+                            let response = await p(...args);
 
-            assert.equal(expectedResult, actualResult, "Divide 16 / 2 resulted in " + expectedResult);
+                            //We're done, let's send our response back to our .Net App
+                            cefSharp.sendEvalScriptResponse(callbackId, true, response, true);
+                        }
+                        catch (err)
+                        {
+                            //An error occurred let's send the response back to our .Net App
+                            cefSharp.sendEvalScriptResponse(callbackId, false, err.message, true);
+                        }
+                    })();
+
+                    //Let CefSharp know we're going to be defering our response as we have some async/await
+                    //processing to happen before our callback returns it's value
+                    return "CefSharpDefEvalScriptRes";
+                }
+
+                return f;
+            }
+
+            //The function convertPromiseToCefSharpCallback can be used in your own projects
+            //Pass in a function that wraps your promise, not your promise directly
+            let callback = convertPromiseToCefSharpCallback(function (msg)
+            {
+                return new Promise((resolve, reject) =>
+                {
+                    setTimeout(() =>
+                    {
+                        resolve(msg);
+                    }, 300);
+                });
+            });
+
+            const expectedResult = "Callback has not been invoked";
+            const actualResult = await boundAsync.javascriptOptionalCallbackEvalPromise(false, expectedResult, callback);
+            assert.equal(expectedResult, actualResult, "Echo response after promise execution");
+
+            const expectedSecondResult = "Callback has been invoked";
+            const actualSecondResult = await boundAsync.javascriptOptionalCallbackEvalPromise(true, expectedSecondResult, callback);
+            assert.equal(expectedSecondResult, actualSecondResult, "Echo response after promise execution");
         });
     });
     </script>

CefSharp.Example/Resources/BindingTestsAsyncTask.html

@@ -98,8 +98,6 @@
 
             QUnit.test("JavascriptCallback EvalPromise:", async (assert) =>
             {
-                const expectedResult = "JavascriptCallback after promise";
-
                 let convertPromiseToCefSharpCallback = function (p)
                 {
                     let f = function (callbackId, ...args)
@@ -135,21 +133,79 @@
 
                 //The function convertPromiseToCefSharpCallback can be used in your own projects
                 //Pass in a function that wraps your promise, not your promise directly
-                let callback = convertPromiseToCefSharpCallback(function ()
+                let callback = convertPromiseToCefSharpCallback(function (msg)
                 {
                     return new Promise((resolve, reject) =>
                     {
                         setTimeout(() =>
                         {
-                            resolve(expectedResult);
+                            resolve(msg);
                         }, 300);
                     });
                 });
 
+                const expectedResult = "JavascriptCallback after promise";
                 const actualResult = await boundAsync.javascriptCallbackEvalPromise(expectedResult, callback);
 
                 assert.equal(expectedResult, actualResult, "Echo response after promise execution");
             });
+
+            // Issue #3979
+            QUnit.test("JavascriptCallback conditionally EvalPromise:", async (assert) =>
+            {
+                let convertPromiseToCefSharpCallback = function (p)
+                {
+                    let f = function (callbackId, ...args)
+                    {
+                        //We immediately return CefSharpDefEvalScriptRes as we will be
+                        //using a promise and will call sendEvalScriptResponse when our
+                        //promise has completed
+                        (async function ()
+                        {
+                            try
+                            {
+                                //Await the promise
+                                let response = await p(...args);
+
+                                //We're done, let's send our response back to our .Net App
+                                cefSharp.sendEvalScriptResponse(callbackId, true, response, true);
+                            }
+                            catch (err)
+                            {
+                                //An error occurred let's send the response back to our .Net App
+                                cefSharp.sendEvalScriptResponse(callbackId, false, err.message, true);
+                            }
+                        })();
+
+                        //Let CefSharp know we're going to be defering our response as we have some async/await
+                        //processing to happen before our callback returns it's value
+                        return "CefSharpDefEvalScriptRes";
+                    }
+
+                    return f;
+                }
+
+                //The function convertPromiseToCefSharpCallback can be used in your own projects
+                //Pass in a function that wraps your promise, not your promise directly
+                let callback = convertPromiseToCefSharpCallback(function (msg)
+                {
+                    return new Promise((resolve, reject) =>
+                    {
+                        setTimeout(() =>
+                        {
+                            resolve(msg);
+                        }, 300);
+                    });
+                });
+
+                const expectedResult = "Callback has not been invoked";
+                const actualResult = await boundAsync.javascriptOptionalCallbackEvalPromise(false, expectedResult, callback);
+                assert.equal(expectedResult, actualResult, "Echo response after promise execution");
+
+                const expectedSecondResult = "Callback has been invoked";
+                const actualSecondResult = await boundAsync.javascriptOptionalCallbackEvalPromise(true, expectedSecondResult, callback);
+                assert.equal(expectedSecondResult, actualSecondResult, "Echo response after promise execution");
+            });
         });
     </script>
 

CefSharp/Internals/PendingTaskRepository.cs

@@ -11,13 +11,18 @@
 namespace CefSharp.Internals
 {
     /// <summary>
-    /// Class to store TaskCompletionSources indexed by a unique id.
+    /// Class to store TaskCompletionSources indexed by a unique id. There are two distinct ConcurrentDictionary
+    /// instances as we have some Tasks that are created from the browser process (EvaluateScriptAsync) calls, and
+    /// some that are created for <see cref="IJavascriptCallback"/> instances for which the Id's are created
+    /// in the render process. 
     /// </summary>
     /// <typeparam name="TResult">The type of the result produced by the tasks held.</typeparam>
     public sealed class PendingTaskRepository<TResult>
     {
         private readonly ConcurrentDictionary<long, TaskCompletionSource<TResult>> pendingTasks =
             new ConcurrentDictionary<long, TaskCompletionSource<TResult>>();
+        private readonly ConcurrentDictionary<long, TaskCompletionSource<TResult>> callbackPendingTasks =
+            new ConcurrentDictionary<long, TaskCompletionSource<TResult>>();
         //should only be accessed by Interlocked.Increment
         private long lastId;
 
@@ -42,17 +47,64 @@ public KeyValuePair<long, TaskCompletionSource<TResult>> CreatePendingTask(TimeS
         }
 
         /// <summary>
-        /// Gets and removed pending task by id.
+        /// Creates a new pending task with a timeout.
+        /// </summary>
+        /// <param name="id">Id passed in from the render process</param>
+        /// <param name="timeout">The maximum running time of the task.</param>
+        /// <returns>The unique id of the newly created pending task and the newly created <see cref="TaskCompletionSource{TResult}"/>.</returns>
+        public KeyValuePair<long, TaskCompletionSource<TResult>> CreateJavascriptCallbackPendingTask(long id, TimeSpan? timeout = null)
+        {
+            var taskCompletionSource = new TaskCompletionSource<TResult>();
+
+            callbackPendingTasks.TryAdd(id, taskCompletionSource);
+
+            if (timeout.HasValue)
+            {
+                taskCompletionSource = taskCompletionSource.WithTimeout(timeout.Value, () => RemoveJavascriptCallbackPendingTask(id));
+            }
+
+            return new KeyValuePair<long, TaskCompletionSource<TResult>>(id, taskCompletionSource);
+        }
+
+        /// <summary>
+        /// If a <see cref="TaskCompletionSource{TResult}"/> is found matching <paramref name="id"/>
+        /// then it is removed from the ConcurrentDictionary and returned.
         /// </summary>
         /// <param name="id">Unique id of the pending task.</param>
         /// <returns>
-        /// The <see cref="TaskCompletionSource{TResult}"/> associated with the given id.
+        /// The <see cref="TaskCompletionSource{TResult}"/> associated with the given id
+        /// or null if no matching TaskComplectionSource found.
         /// </returns>
         public TaskCompletionSource<TResult> RemovePendingTask(long id)
         {
             TaskCompletionSource<TResult> result;
-            pendingTasks.TryRemove(id, out result);
-            return result;
+            if (pendingTasks.TryRemove(id, out result))
+            {
+                return result;
+            }
+
+            return null;
+        }
+
+        /// <summary>
+        /// If a <see cref="TaskCompletionSource{TResult}"/> is found matching <paramref name="id"/>
+        /// then it is removed from the ConcurrentDictionary and returned.
+        /// </summary>
+        /// <param name="id">Unique id of the pending task.</param>
+        /// <returns>
+        /// The <see cref="TaskCompletionSource{TResult}"/> associated with the given id
+        /// or null if no matching TaskComplectionSource found.
+        /// </returns>
+        public TaskCompletionSource<TResult> RemoveJavascriptCallbackPendingTask(long id)
+        {
+            TaskCompletionSource<TResult> result;
+
+            if (callbackPendingTasks.TryRemove(id, out result))
+            {
+                return result;
+            }
+
+            return null;
         }
     }
-}
+}