Skip to content

Replaced kPromiseCreatorScript with native promise #5094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Replaced kPromiseCreatorScript with native promise #5094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c4bfd44
Replaced kPromiseCreatorScript with native promise
AgapovAlexsey Apr 24, 2025
fc6021e
Applied camelCase to field names
AgapovAlexsey Apr 26, 2025
f7540d2
BindObjectAsyncHandler should always return a promise
amaitland Apr 30, 2025
82d3721
Fix xunit test
amaitland Apr 30, 2025
5e55774
Readd missing code
amaitland Apr 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions CefSharp.BrowserSubprocess.Core/Async/JavascriptAsyncMethodCallback.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,11 @@ namespace CefSharp
{
void JavascriptAsyncMethodCallback::Success(const CefRefPtr<CefV8Value>& result)
{
if (_resolve.get() && _context.get() && _context->Enter())
if (_promise.get() && _context.get() && _context->Enter())
{
try
{
CefV8ValueList args;
args.push_back(result);
_resolve->ExecuteFunction(nullptr, args);
_promise->ResolvePromise(result);
}
finally
{
Expand All @@ -30,13 +28,11 @@ namespace CefSharp

void JavascriptAsyncMethodCallback::Fail(const CefString& exception)
{
if (_reject.get() && _context.get() && _context->Enter())
if (_promise.get() && _context.get() && _context->Enter())
{
try
{
CefV8ValueList args;
args.push_back(CefV8Value::CreateString(exception));
_reject->ExecuteFunction(nullptr, args);
_promise->RejectPromise(exception);
}
finally
{
Expand Down
10 changes: 4 additions & 6 deletions CefSharp.BrowserSubprocess.Core/Async/JavascriptAsyncMethodCallback.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,19 @@ namespace CefSharp
{
private:
MCefRefPtr<CefV8Context> _context;
MCefRefPtr<CefV8Value> _resolve;
MCefRefPtr<CefV8Value> _reject;
MCefRefPtr<CefV8Value> _promise;

public:
JavascriptAsyncMethodCallback(CefRefPtr<CefV8Context> context, CefRefPtr<CefV8Value> resolve, CefRefPtr<CefV8Value> reject)
:_context(context), _resolve(resolve.get()), _reject(reject.get())
JavascriptAsyncMethodCallback(CefRefPtr<CefV8Context> context, CefRefPtr<CefV8Value> promise)
:_context(context), _promise(promise.get())
{

}

!JavascriptAsyncMethodCallback()
{
_resolve = nullptr;
_reject = nullptr;
_context = nullptr;
_promise = nullptr;
}

~JavascriptAsyncMethodCallback()
Expand Down
28 changes: 3 additions & 25 deletions CefSharp.BrowserSubprocess.Core/Async/JavascriptAsyncMethodHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,32 +24,10 @@ namespace CefSharp
auto context = CefV8Context::GetCurrentContext();
auto frame = context->GetFrame();

CefRefPtr<CefV8Value> promiseData;
CefRefPtr<CefV8Exception> promiseException;
//this will create a promise and give us the reject/resolve functions {p: Promise, res: resolve(), rej: reject()}
if (!context->Eval(CefAppUnmanagedWrapper::kPromiseCreatorScript, CefString(), 0, promiseData, promiseException))
{
LOG(WARNING) << "JavascriptAsyncMethodHandler::Execute promiseData returned exception: " + promiseException->GetMessage().ToString();

exception = promiseException->GetMessage();

return true;
}

//when refreshing the browser this is sometimes null, in this case return true and log message
//https://github.com/cefsharp/CefSharp/pull/2446
if (promiseData == nullptr)
{
LOG(WARNING) << "JavascriptAsyncMethodHandler::Execute promiseData returned nullptr";

return true;
}

retval = promiseData->GetValue("p");
CefRefPtr<CefV8Value> promise = CefV8Value::CreatePromise();
retval = promise;

auto resolve = promiseData->GetValue("res");
auto reject = promiseData->GetValue("rej");
auto callback = gcnew JavascriptAsyncMethodCallback(context, resolve, reject);
auto callback = gcnew JavascriptAsyncMethodCallback(context, promise);
auto callbackId = _methodCallbackSave->Invoke(callback);

auto request = CefProcessMessage::Create(kJavascriptAsyncMethodCallRequest);
Expand Down
88 changes: 30 additions & 58 deletions CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,54 +165,17 @@ namespace CefSharp
rootObject->Bind(cachedObjects, context->GetGlobal());

//Objects already bound or ignore cache
CefRefPtr<CefV8Value> promiseResolve;
CefRefPtr<CefV8Exception> promiseException;

auto promiseResolveScript = StringUtils::ToNative("Promise.resolve({Success:true, Count:" + cachedObjects->Count + ", Message:'OK'});");

if (context->Eval(promiseResolveScript, CefString(), 0, promiseResolve, promiseException))
{
retval = promiseResolve;
}
else
{
exception = promiseException->GetMessage();

return true;
}
auto promiseResolve = CefV8Value::CreatePromise();
promiseResolve->ResolvePromise(CreateResultObject(cachedObjects->Count, "OK", true));
retval = promiseResolve;

NotifyObjectBound(frame, objectNamesWithBoundStatus);
}
else
{
CefRefPtr<CefV8Value> promiseData;
CefRefPtr<CefV8Exception> promiseException;
//this will create a promise and give us the reject/resolve functions {p: Promise, res: resolve(), rej: reject()}
if (!context->Eval(CefAppUnmanagedWrapper::kPromiseCreatorScript, CefString(), 0, promiseData, promiseException))
{
exception = promiseException->GetMessage();

return true;
}

//when refreshing the browser this is sometimes null, in this case return true and log message
//https://github.com/cefsharp/CefSharp/pull/2446
if (promiseData == nullptr)
{
LOG(WARNING) << "BindObjectAsyncHandler::Execute promiseData returned nullptr";

return true;
}

//return the promose
retval = promiseData->GetValue("p");

//References to the promise resolve and reject methods
auto resolve = promiseData->GetValue("res");
auto reject = promiseData->GetValue("rej");

auto callback = gcnew JavascriptAsyncMethodCallback(context, resolve, reject);

CefRefPtr<CefV8Value> promise = CefV8Value::CreatePromise();
retval = promise;
auto callback = gcnew JavascriptAsyncMethodCallback(context, promise);
auto request = CefProcessMessage::Create(kJavascriptRootObjectRequest);
auto argList = request->GetArgumentList();

Expand All @@ -228,23 +191,13 @@ namespace CefSharp
else
{
//Objects already bound or ignore cache
CefRefPtr<CefV8Value> promiseResolve;
CefRefPtr<CefV8Exception> promiseException;
auto promiseResolve = CefV8Value::CreatePromise();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You create a promise and assign it to the result three times. You can create it earlier and assign it to retval, and resolve it later. It will work.
And we can replace exception = "BindObjectAsyncHandler::Execute - Browser wrapper null, unable to bind objects"; with promiseResolve->RejectPromise("BindObjectAsyncHandler::Execute - Browser wrapper null, unable to bind objects");.

promiseResolve->ResolvePromise(CreateResultObject(0, "Object(s) already bound", false));
retval = promiseResolve;

auto promiseResolveScript = CefString("Promise.resolve({Success:false, Count:0, Message:'Object(s) already bound'});");

if (context->Eval(promiseResolveScript, CefString(), 0, promiseResolve, promiseException))
if (notifyIfAlreadyBound)
{
retval = promiseResolve;

if (notifyIfAlreadyBound)
{
NotifyObjectBound(frame, objectNamesWithBoundStatus);
}
}
else
{
exception = promiseException->GetMessage();
NotifyObjectBound(frame, objectNamesWithBoundStatus);
}
}
}
Expand All @@ -267,6 +220,25 @@ namespace CefSharp
return true;
}

static CefRefPtr<CefV8Value> CreateResultObject(int count, String^ message, bool isSuccess)
{
auto response = CefV8Value::CreateObject(nullptr, nullptr);

const auto countResult = CefV8Value::CreateInt(count);
const auto messageResult = CefV8Value::CreateString(StringUtils::ToNative(message));
const auto successResult = CefV8Value::CreateBool(isSuccess);

response->SetValue("Count", countResult, CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);
response->SetValue("Message", messageResult, CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);
response->SetValue("Success", successResult, CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);

response->SetValue("count", countResult, CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);
response->SetValue("message", messageResult, CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);
response->SetValue("success", successResult, CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);

return response;
}

private:
void NotifyObjectBound(const CefRefPtr<CefFrame> frame, List<Tuple<String^, bool, bool>^>^ objectNamesWithBoundStatus)
{
Expand Down
22 changes: 2 additions & 20 deletions CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,6 @@ namespace CefSharp
{
namespace BrowserSubprocess
{
const CefString CefAppUnmanagedWrapper::kPromiseCreatorScript = ""
"(function()"
"{"
" var result = {};"
" var promise = new Promise(function(resolve, reject) {"
" result.res = resolve; result.rej = reject;"
" });"
" result.p = promise;"
" return result;"
"})();";

const CefString kRenderProcessId = CefString("RenderProcessId");
const CefString kRenderProcessIdCamelCase = CefString("renderProcessId");
Expand Down Expand Up @@ -662,23 +652,15 @@ namespace CefSharp

if (_registerBoundObjectRegistry->TryGetAndRemoveMethodCallback(callbackId, callback))
{
//Response object has no Accessor or Interceptor
auto response = CefV8Value::CreateObject(nullptr, nullptr);

response->SetValue("Count", CefV8Value::CreateInt(javascriptObjects->Count), CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);

if (javascriptObjects->Count > 0)
{
//TODO: JSB Should we include a list of successfully bound object names?
response->SetValue("Success", CefV8Value::CreateBool(true), CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);
response->SetValue("Message", CefV8Value::CreateString("OK"), CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);
callback->Success(response);
callback->Success(BindObjectAsyncHandler::CreateResultObject(javascriptObjects->Count, "OK", true));
}
else
{
response->SetValue("Success", CefV8Value::CreateBool(false), CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);
response->SetValue("Message", CefV8Value::CreateString("Zero objects bounds"), CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);
callback->Success(response);
callback->Success(BindObjectAsyncHandler::CreateResultObject(javascriptObjects->Count, "Zero objects bounds", false));
}

//Send message notifying Browser Process of which objects were bound
Expand Down
1 change: 0 additions & 1 deletion CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ namespace CefSharp
gcroot<RegisterBoundObjectRegistry^> _registerBoundObjectRegistry;

public:
static const CefString kPromiseCreatorScript;

CefAppUnmanagedWrapper(IRenderProcessHandler^ handler, List<CefCustomScheme^>^ schemes, bool enableFocusedNodeChanged, Action<CefBrowserWrapper^>^ onBrowserCreated, Action<CefBrowserWrapper^>^ onBrowserDestroyed) : SubProcessApp(schemes)
{
Expand Down
18 changes: 18 additions & 0 deletions CefSharp.Example/Resources/BindingTestAsync.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,4 +286,22 @@ QUnit.module('BindingTestAsync', (hooks) =>
assert.ok(window.boundAsync2 === undefined, "boundAsync2 is now undefined");
});

QUnit.test("Validate BindObjectAsync result object fields:", async (assert) =>
{
const response = await CefSharp.BindObjectAsync({ NotifyIfAlreadyBound: true, IgnoreCache: true }, "boundAsync2");
const keys = Object.getOwnPropertyDescriptors(response);

assert.equal(!!keys["count"], true, "count");
assert.equal(!!keys["Count"], true, "Count");

assert.equal(!!keys["message"], true, "message");
assert.equal(!!keys["Message"], true, "Message");

assert.equal(!!keys["success"], true, "success");
assert.equal(!!keys["Success"], true, "Success");

assert.equal(true, CefSharp.DeleteBoundObject("boundAsync2"), "Object was unbound");
assert.ok(window.boundAsync2 === undefined, "boundAsync2 is now undefined");
});

});