h5vcc: Use base::span for data transport in H5vccPlatformService#9288
Open
rakuco wants to merge 4 commits intoyoutube:mainfrom
Open
h5vcc: Use base::span for data transport in H5vccPlatformService#9288rakuco wants to merge 4 commits intoyoutube:mainfrom
rakuco wants to merge 4 commits intoyoutube:mainfrom
Conversation
Use a ReadOnlyBuffer mojom type to represent data rather than an array, as this makes the C++ bindings use base::span instead of std::vector. Doing so allows for less duplication and fewer manual conversions, as DOMArrayBuffer can deal with spans natively, and some simplification on the browser side as well. While here, const-ify and use `std::unique_ptr` with `base::FreeDeleter` on `response_ptr` to avoid manual memory management. Tested with the softmic extension on Linux. Fixed: 487712437
Contributor
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the H5vccPlatformService to use base::span for data transport over Mojom by switching from array<uint8> to mojo_base.mojom.ReadOnlyBuffer. This is a good improvement that reduces data copying. The change also introduces std::unique_ptr with base::FreeDeleter to manage memory returned from the Starboard service, which improves memory safety by replacing manual free() calls. The changes are well-implemented across the C++ and Mojom files. I have one suggestion to ensure type safety during a numeric conversion.
Collaborator
Author
|
/gemini review |
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use a ReadOnlyBuffer mojom type to represent data rather than an array, as this makes the C++ bindings use base::span instead of std::vector.
Doing so allows for less duplication and fewer manual conversions, as DOMArrayBuffer can deal with spans natively, and some simplification on the browser side as well. While here, const-ify and use
std::unique_ptrwithbase::FreeDeleteronresponse_ptrto avoid manual memory management.Tested with the softmic extension on Linux.
Fixed: 487712437