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
Extract WideToUTF16String/UTF16StringToWide to FML #39006
Conversation
5ec7530
to
d91e80a
Compare
In third_party/accessibility, for string conversion, we use a mix of: * FML * third_party/accessibility base string utility functions * static functions local to the translation unit itself This moves all conversions between UTF16 and wide strings to FML. Note that this implementation is only safe on platforms where: * the size of wchar_t and char16_t are the same * the encoding of wchar_t and char16_t are both UTF-16 which is the case for Windows, hence why these functions are implemented in a Windows-specific translation unit (wstring_conversion). Issue: flutter/flutter#118811
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still LGTM :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I was thinking about this too
The windows failures do look related. |
Yep - two of the tests were wrong; serves me right for adding Windows-specific tests from a Mac. |
Looking good now :) |
Reverting as it broke the tree https://ci.chromium.org/p/flutter/builders/prod/Windows%20Host%20Engine/19925 |
This reverts commit 7bbe79e.
Oh wow. I'm curious how that wasn't caught in presubmit! Thanks for reverting. We should definitely not be using //third_party/accessibility:base outside of the accessibility tree. I'll fix that code and see if I can add a visibility restriction so people don't re-add code like that in future. |
Ah I see why it wasn't caught - a new usage was introduced in a separate PR between the time presubmits ran and the patch landed. |
…118869) * 7bbe79e10 Extract WideToUTF16String/UTF16StringToWide to FML (flutter/engine#39006) * acca56ce0 Revert "Extract WideToUTF16String/UTF16StringToWide to FML (#39006)" (flutter/engine#39019) * 730e88fb6 [Impeller] Check the correct stencil coverage when deciding whether to elide a restore (flutter/engine#39023)
* Extract WideToUTF16String/UTF16StringToWide to FML In third_party/accessibility, for string conversion, we use a mix of: * FML * third_party/accessibility base string utility functions * static functions local to the translation unit itself This moves all conversions between UTF16 and wide strings to FML. Note that this implementation is only safe on platforms where: * the size of wchar_t and char16_t are the same * the encoding of wchar_t and char16_t are both UTF-16 which is the case for Windows, hence why these functions are implemented in a Windows-specific translation unit (wstring_conversion). Issue: flutter/flutter#118811 * Migrate UTF16ToWide as well
…9006)" (flutter#39019) This reverts commit 7bbe79e.
In third_party/accessibility, for string conversion, we use a mix of:
This moves all conversions between UTF16 and wide strings to FML. Note that this implementation is only safe on platforms where:
Issue: flutter/flutter#118811
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.