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

Fix EmbindObject's deleteLater to deleteAfter #86

Closed
wants to merge 1 commit into from

Conversation

namse
Copy link
Contributor

@namse namse commented Sep 4, 2021

I found that CanvasKit's EmbindObject type has wrong function name, deleteAfter.

I cound't find any information of function deleteAfter even in google searching. so i digged it.

image

I have no idea with deleteAfter, but EmbindObject has the function name deleteLater, not deleteAfter.

Below is the code of Embind.
https://github.com/emscripten-core/emscripten/blob/dd5733ac80881e55e1f6757caabf617b6b2a7cec/src/embind/embind.js#L1782

@google-cla google-cla bot added the cla: yes A signed CLA is on file. label Sep 4, 2021
@skia-codereview-bot
Copy link
Collaborator

This PR (HEAD: 0b08b6e) has been imported to Gerrit for code review.

Please visit review.skia.org/445736 to see it. Please CC yourself to the Gerrit change.

Note:

  • Skia uses only Gerrit for reviews and submitting code (doc).
  • All comments are handled within Gerrit. Any comments on the GitHub PR will be ignored.
  • The PR author can continue to upload commits to the branch used by the PR in order to address feedback from Gerrit.
  • Once the code is ready to be merged, a maintainer will submit the change on Gerrit and skia-codereview-bot will close this PR.
  • Similarly, if a change is abandoned on Gerrit, the corresponding PR will be closed with a note.

@skia-codereview-bot
Copy link
Collaborator

This PR is being closed because review.skia.org/445736 has been merged.

blueboxd pushed a commit to blueboxd/skia that referenced this pull request Sep 8, 2021
I found that CanvasKit's `EmbindObject` type has wrong function name, `deleteAfter`.

I cound't find any information of function `deleteAfter` even in google searching. so i digged it.

![image](https://user-images.githubusercontent.com/3580430/132103177-9c7c32a3-2107-4b97-9698-f52da5833ba1.png)

I have no idea with `deleteAfter`, but `EmbindObject` has the function name `deleteLater`, not `deleteAfter`.

Below is the code of Embind.
https://github.com/emscripten-core/emscripten/blob/dd5733ac80881e55e1f6757caabf617b6b2a7cec/src/embind/embind.js#L1782

This is an imported pull request from
google#86

GitOrigin-RevId: 0738dc7
Change-Id: I0a7af3d4f313d36e8cdc09d631f9199e948eca29
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445736
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes A signed CLA is on file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants