str_buffer contains a multi-string delimited by nulls, so I believe we would need a for loop here no matter what. We could call this function on each substring of the multi-string and increment the counter by its length instead of iterating over each character, but I do not see any benefit to this.

S_OK is technically correct but ERROR_SUCCESS would be more idiomatic: https://learn.microsoft.com/windows/win32/api/winreg/nf-winreg-reggetvaluea#return-value

Microsoft docs are localized only if you remove the language code:

Is using RegGetValueA instead of RegGetValue intentional? If no, prefer RegGetValue. That might have the added benefit of removing the str_buffer to buffer conversion below.

Is there any way to get the length of locales programmatically?

https://stackoverflow.com/questions/2853615/get-length-of-wchar-t-in-c

The issue here is that it's not just a string, it's a multistring, which means it contains null delimiters in the middle that string length methods will treat as a terminator and return a bad value for. We could write our own method that counts until it reaches a double null, though I don't know how much use it would get outside of this test.

Is the incoming data a string of locales delimited by \0?

Yes. Each locale is delimited by \0 with a double \0 at the end.

Lol, I wonder why they would do that 🤷‍♂️ . Anyway, the change LGTM now 👍 .

That's how the Windows registry stores all multi strings, or at least how it represents them when you request the data.

You should be able to do this as follows:

using namespace std::string_literals;
static const std::wstring locales = L"en-US\0zh-Hans-CN\0ja\0zh-Hant-TW\0he\0\0"s;
static const DWORD locales_len = locales.size();

That said -- the documentation for the pcbData param to GetRegValue function states this value should be in bytes, so we should really return:

static const DWORD locales_len = locales.size() * sizeof(wchar_t);

which in this case works out to 140.

Looks like you're already doing that multiplication below.

Looks like these strings gets referenced twice

L"Control panel\International\User Profile",
L"Languages"

Should we make these constants? It might help improve readability 🤷‍♂️

Even briefer, and a bit more correct, since technically this is a pointer and not a handle (which, while being a type of void* has a very specific meaning in Windows):

Are the changes in this file necessary or can they be reverted?

Are these includes necessary?

Is this used? If no, please removing it as it's dead code. If yes, should this be a std::unique_ptr that's moved by the caller to avoid a copy?

explicit isn't needed here since there's no parameter to be eligible for implicit conversion (and because copy-initialization is already prevented by the disallow copy & assign macro, which already prevents something like WindowsRegistry r = {}).

You can delete this if you apply the default suggestion below. Sadly I'm not GitHub-suggestion-savvy enough to know how to delete two lines in a suggested change :/

Prefix as std::memcpy and make sure to #include <cstring>, where it's defined.

Same name fix here, but I have no idea how to do multiline suggestions in github :(

In unit tests, I think you'll want to inject the registry, so you'll want to add a std::unique_ptr<WindowsRegistry> registry param, then move the unique the the construction site rather than in the initialiser list.

This was my rationale for including the SetWindowsRegistry method originally. Can parameters have default arguments that are not constant expressions, or would we need to include this new argument anywhere a FlutterWindowsEngine is constructed?

You'd need to include this wherever it's constructed, though other than unit tests, I suspect that should be very rare, possibly just one place. An alternative would be to add a second constructor that takes the registry and use that one in unit tests where it matters.

Since semantically, this isn't something we ever expect to mutate on an existing engine, I think constructor injection is preferrable.