Skip to content

Conversation

@MikeHolman
Copy link
Contributor

This change adds a laundry list of new (still very experimental) JSRT APIs, to support improvements in chakra shim functionality and performance.

curtisman and others added 30 commits June 20, 2018 20:51
Also allow Js[Has|Get|Set]ExternalData to apply to the target if it is
a proxy.
Currently support serializing/deserializing only objects managed by javascript engine.
Support for DOMObject is yet to come
Moving SCA to chakracore Part1

Currently support serializing/deserializing only objects managed by javascript engine.
Support for DOMObject is yet to come
Support for transferring arraybuffer.
Part2 Serialization/Deserialization for ArrayBuffer

Support for transferring arraybuffer.
…hookup

Part 3 : Serialization/Deserialization Host object hookup
…nterceptors.

The CEO will be only when interceptors are needed. Otherwise, we will still use the JsrtExternalObject.
…c in the shim.

Avoid determining whether a property name is numeric in the shim.
…nterceptors

don't create property string for indexed property interceptors
Fix JsrtExternalObject creation asserts.
Copy link
Collaborator

@liminzhu liminzhu left a comment

Choose a reason for hiding this comment

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

Only leaving some high level comments for now.

/// <returns>
/// The code <c>JsNoError</c> if the operation succeeded, a failure code otherwise.
/// </returns>
CHAKRA_API
Copy link
Collaborator

@liminzhu liminzhu Feb 5, 2019

Choose a reason for hiding this comment

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

Can we get this and the other typedefs into ChakraCore.h? ChakraCommon is for shared APIs between Chakra and ChakraCore.

/// <param name="data">
/// The external data that was passed in when creating the object being traced.
/// </param>
typedef void (CHAKRA_CALLBACK *JsTraceCallback)(_In_opt_ void *data);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missed this guy - should be in chakracore.h

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried moving it to ChakraCore.h, but we still use this typedef in ChakraFull, so it seemed kind of annoying. I guess I can #ifdef out all the places though

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmmmm, i thought the callback is only used with JsCreateTracedExternalObject and JsCreateCustomExternalObject which is in chakracore.h? Btw is there any reason these two API are not merged into one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess the APIs for these could be merged, but underneath they create very different types of objects so I'd rather hold off for now.

We share code for JsrtExternalObjects between ChakraCore and ChakraFull, and even though trace callback is only used in ChakraCore, it was referenced there in both builds... I moved it all behind ifdef now though

Copy link
Collaborator

Choose a reason for hiding this comment

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

SGTM, thx!

@MikeHolman MikeHolman force-pushed the mergeshim branch 5 times, most recently from e3a1f7b to 08a4d40 Compare February 7, 2019 04:03
@chakrabot chakrabot merged commit 282f7a2 into chakra-core:master Feb 8, 2019
chakrabot pushed a commit that referenced this pull request Feb 8, 2019
Merge pull request #5923 from MikeHolman:mergeshim

This change adds a laundry list of new (still very experimental) JSRT APIs, to support improvements in chakra shim functionality and performance.
@rhuanjl rhuanjl mentioned this pull request Apr 2, 2020
@rhuanjl rhuanjl mentioned this pull request Apr 13, 2020
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.

10 participants