Skip to content
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.

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

Allow calling managed methods from objects bound with RegisterAsyncJsObject that return arrays #1979

Closed
wants to merge 2 commits into from

Conversation

joaompneves
Copy link
Contributor

Problem:
When using RegisterAsyncJsObject to bind a C# object it was not possible to call managed methods that return arrays, due to a crash in the browser sub process.

Details:
CefV8Value::CreateArray in DeserializeV8Object expects prior call to Enter of the V8 context, and without that call, CreateArray returned null.

Fix:
Call V8 context Enter in OnProcessMessageReceived from CefAppUnmanagedWrapper.

callback->Fail(argList->GetString(3));
}
//dispose
delete callback;
Copy link
Member

Choose a reason for hiding this comment

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

If it somehow fails this wont be called, you should place it inside the finally block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't change that bit, but will do it.

@amaitland
Copy link
Member

Need a better description for this PR. If I was to look at the change log and see Fixed problem when calling CefV8Value::CreateArray that doesn't really mean a whole heap. At a minimum need to make reference to it being related to RegisterAsyncJsObject

@joaompneves joaompneves changed the title Fixed problem when calling CefV8Value::CreateArray Allow calling managed methods from objects bound with RegisterAsyncJsObject that return arrays Mar 10, 2017
@amaitland
Copy link
Member

This PR is redundant

@amaitland amaitland closed this Mar 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants