Skip to content
This repository was archived by the owner on Nov 12, 2022. It is now read-only.

Add glue methods to get JSValue of a private reference#515

Merged
bors-servo merged 1 commit intoservo:masterfrom
CYBAI:msvc-get-private
Jul 19, 2020
Merged

Add glue methods to get JSValue of a private reference#515
bors-servo merged 1 commit intoservo:masterfrom
CYBAI:msvc-get-private

Conversation

@CYBAI
Copy link
Copy Markdown
Member

@CYBAI CYBAI commented Jul 18, 2020

While working on servo/servo#27026, we've noticed calling
GetScriptPrivate will hit a CRASH consistently in Windows. jdm noticed
that this is caused by "can't return JSValue by value on Windows"; thus,
we need to expose these 2 glue methods so that we can handle the
GetScriptPrivate and GetModulePrivate correctly.


This will help servo/servo#27026.

We might want to handle the ModulePrivate much more correctly in the future so it might be worth adding it here as well?

@CYBAI
Copy link
Copy Markdown
Member Author

CYBAI commented Jul 18, 2020

Btw, I just noticed the Microsoft documentation link in line 1072 is 404 now.

I tried to search it but looks like I can't find exactly same one.

But found 2 links might be related; ☝️ is any of them I should update to the comment?

@CYBAI CYBAI force-pushed the msvc-get-private branch from 5d023c7 to 8d19da9 Compare July 18, 2020 06:09
@jdm
Copy link
Copy Markdown
Member

jdm commented Jul 18, 2020

Please file an issue about the broken doc link.

While working on servo/servo#27026, we've noticed calling
`GetScriptPrivate` will hit a CRASH consistently in Windows. jdm noticed
that this is caused by "can't return JSValue by value on Windows"; thus,
we need to expose these 2 glue methods so that we can handle the
`GetScriptPrivate` and `GetModulePrivate` correctly.
@jdm
Copy link
Copy Markdown
Member

jdm commented Jul 19, 2020

@bors-servo r+

@bors-servo
Copy link
Copy Markdown
Contributor

📌 Commit cc7aeda has been approved by jdm

@bors-servo
Copy link
Copy Markdown
Contributor

⌛ Testing commit cc7aeda with merge 716dede...

@bors-servo
Copy link
Copy Markdown
Contributor

☀️ Test successful - checks-travis, status-appveyor
Approved by: jdm
Pushing 716dede to master...

@bors-servo bors-servo merged commit 716dede into servo:master Jul 19, 2020
@CYBAI CYBAI deleted the msvc-get-private branch July 19, 2020 06:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants