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.

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

Jump to bottom

[browser] reduce use of wrap_error_root #100233

Merged
merged 3 commits into from
Mar 27, 2024
Merged

[browser] reduce use of wrap_error_root #100233

merged 3 commits into from
Mar 27, 2024

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Mar 25, 2024 •
edited
Loading

  • renamed BindJSImport to BindJSImportST and changed signature
    • it now returns null pointer to UTF16 buffer when there is no error
    • or zero terminated UTF16 buffer with error message, freed by receiver
  • renamed mono_wasm_bind_js_import to mono_wasm_bind_js_import_ST
  • implemented stringToUTF16Ptr
  • moved wrap_error_root to hybrid-globalization
  • makes failed binding JSException rather than abort

Contributes to #98365

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm labels Mar 25, 2024
@pavelsavara pavelsavara added this to the 9.0.0 milestone Mar 25, 2024
@pavelsavara pavelsavara self-assigned this Mar 25, 2024
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

@pavelsavara pavelsavara marked this pull request as ready for review March 25, 2024 14:09
ilonatommy
ilonatommy approved these changes Mar 25, 2024
View reviewed changes
src/mono/browser/runtime/strings.ts Outdated Show resolved Hide resolved
src/mono/browser/runtime/strings.ts Show resolved Hide resolved
@build-analysis build-analysis bot mentioned this pull request Mar 25, 2024
Closed
@ilonatommy ilonatommy mentioned this pull request Mar 26, 2024
Merged
@pavelsavara
Copy link
Member Author

I will wait with merging of this until #100271 is merged first

@pavelsavara pavelsavara added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Mar 26, 2024
@build-analysis build-analysis bot mentioned this pull request Mar 26, 2024
Closed
@pavelsavara pavelsavara removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Mar 27, 2024
@ilonatommy ilonatommy mentioned this pull request Mar 27, 2024
Closed
@ilonatommy ilonatommy merged commit 40a6b1d into dotnet:main Mar 27, 2024
95 of 98 checks passed
@jkotas jkotas mentioned this pull request Mar 27, 2024
Open
@pavelsavara pavelsavara mentioned this pull request Mar 28, 2024
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ilonatommy ilonatommy ilonatommy approved these changes

@lewing lewing Awaiting requested review from lewing lewing is a code owner

Assignees

@pavelsavara pavelsavara

Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm
Projects
None yet
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@pavelsavara @ilonatommy