Skip to content

Commit 11bf86a

Browse files
cbrackencbracken
authored
Migrate string encoding conversions to FML (flutter#31334)
We've implemented UTF-8/UTF-16 string encoding conversions in multiple places, from FML to //flutter/shell/platform/common, to the individual embedders. This migrates these conversions to FML and adds tests. Windows APIs use wchar_t-based strings and as a result, we continue to keep Windows-specific functions in fml/platform/win/wstring_conversion.h. We break out string_conversions into its own source set since FML brings with it some Dart dependencies (e.g. dart_timestamp_provider.cc) that are unused by some targets such as uwptool.exe in the Windows UWP embedding. Issue: flutter#98061
1 parent 24fe585 commit 11bf86a

26 files changed

+277
-249
lines changed

‎ci/licenses_golden/licenses_flutter

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,9 @@ FILE: ../../../flutter/fml/platform/win/message_loop_win.h
285285
FILE: ../../../flutter/fml/platform/win/native_library_win.cc
286286
FILE: ../../../flutter/fml/platform/win/paths_win.cc
287287
FILE: ../../../flutter/fml/platform/win/posix_wrappers_win.cc
288+
FILE: ../../../flutter/fml/platform/win/wstring_conversion.cc
288289
FILE: ../../../flutter/fml/platform/win/wstring_conversion.h
290+
FILE: ../../../flutter/fml/platform/win/wstring_conversion_unittests.cc
289291
FILE: ../../../flutter/fml/posix_wrappers.h
290292
FILE: ../../../flutter/fml/raster_thread_merger.cc
291293
FILE: ../../../flutter/fml/raster_thread_merger.h
@@ -294,6 +296,9 @@ FILE: ../../../flutter/fml/shared_thread_merger.cc
294296
FILE: ../../../flutter/fml/shared_thread_merger.h
295297
FILE: ../../../flutter/fml/size.h
296298
FILE: ../../../flutter/fml/status.h
299+
FILE: ../../../flutter/fml/string_conversion.cc
300+
FILE: ../../../flutter/fml/string_conversion.h
301+
FILE: ../../../flutter/fml/string_conversion_unittests.cc
297302
FILE: ../../../flutter/fml/synchronization/atomic_object.h
298303
FILE: ../../../flutter/fml/synchronization/count_down_latch.cc
299304
FILE: ../../../flutter/fml/synchronization/count_down_latch.h
@@ -2049,9 +2054,6 @@ FILE: ../../../flutter/shell/platform/windows/public/flutter_windows.h
20492054
FILE: ../../../flutter/shell/platform/windows/sequential_id_generator.cc
20502055
FILE: ../../../flutter/shell/platform/windows/sequential_id_generator.h
20512056
FILE: ../../../flutter/shell/platform/windows/sequential_id_generator_unittests.cc
2052-
FILE: ../../../flutter/shell/platform/windows/string_conversion.cc
2053-
FILE: ../../../flutter/shell/platform/windows/string_conversion.h
2054-
FILE: ../../../flutter/shell/platform/windows/string_conversion_unittests.cc
20552057
FILE: ../../../flutter/shell/platform/windows/system_utils.h
20562058
FILE: ../../../flutter/shell/platform/windows/system_utils_unittests.cc
20572059
FILE: ../../../flutter/shell/platform/windows/system_utils_win32.cc

‎fml/BUILD.gn

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,11 @@ source_set("fml") {
104104
sources += [ "backtrace_stub.cc" ]
105105
}
106106

107-
public_deps = [ ":command_line" ]
107+
public_deps = [
108+
":build_config",
109+
":command_line",
110+
":string_conversion",
111+
]
108112

109113
deps = [
110114
"//third_party/abseil-cpp/absl/debugging:symbolize",
@@ -217,11 +221,7 @@ source_set("fml") {
217221
"platform/win/native_library_win.cc",
218222
"platform/win/paths_win.cc",
219223
"platform/win/posix_wrappers_win.cc",
220-
"platform/win/wstring_conversion.h",
221224
]
222-
223-
# For wstring_conversion. See issue #50053.
224-
defines = [ "_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING" ]
225225
} else {
226226
sources += [
227227
"platform/posix/file_posix.cc",
@@ -233,6 +233,15 @@ source_set("fml") {
233233
}
234234
}
235235

236+
source_set("build_config") {
237+
sources = [ "build_config.h" ]
238+
239+
public_configs = [
240+
"//flutter:config",
241+
"//flutter/common:flutter_config",
242+
]
243+
}
244+
236245
source_set("command_line") {
237246
sources = [
238247
"command_line.cc",
@@ -245,6 +254,30 @@ source_set("command_line") {
245254
]
246255
}
247256

257+
source_set("string_conversion") {
258+
sources = [
259+
"string_conversion.cc",
260+
"string_conversion.h",
261+
]
262+
263+
if (is_win) {
264+
sources += [
265+
"platform/win/wstring_conversion.cc",
266+
"platform/win/wstring_conversion.h",
267+
]
268+
269+
# TODO(cbracken): https://github.com/flutter/flutter/issues/50053
270+
defines = [ "_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING" ]
271+
}
272+
273+
deps = [ ":build_config" ]
274+
275+
public_configs = [
276+
"//flutter:config",
277+
"//flutter/common:flutter_config",
278+
]
279+
}
280+
248281
if (enable_unittests) {
249282
test_fixtures("fml_fixtures") {
250283
fixtures = []
@@ -286,6 +319,7 @@ if (enable_unittests) {
286319
"message_loop_unittests.cc",
287320
"paths_unittests.cc",
288321
"raster_thread_merger_unittests.cc",
322+
"string_conversion_unittests.cc",
289323
"synchronization/count_down_latch_unittests.cc",
290324
"synchronization/semaphore_unittest.cc",
291325
"synchronization/sync_switch_unittest.cc",
@@ -307,6 +341,10 @@ if (enable_unittests) {
307341
]
308342
}
309343

344+
if (is_win) {
345+
sources += [ "platform/win/wstring_conversion_unittests.cc" ]
346+
}
347+
310348
deps = [
311349
":fml_fixtures",
312350
"//flutter/fml",

‎fml/platform/win/errors_win.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ std::string GetLastErrorMessage() {
4040
std::wstringstream stream;
4141
stream << message << " (" << last_error << ").";
4242

43-
return WideStringToString(stream.str());
43+
return WideStringToUtf8(stream.str());
4444
}
4545

4646
} // namespace fml

‎fml/platform/win/file_win.cc

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ static std::string GetFullHandlePath(const fml::UniqueFD& handle) {
4545
// was cached
4646
if (result && memcmp(found.value().id.Identifier, info.FileId.Identifier,
4747
sizeof(FILE_ID_INFO))) {
48-
return WideStringToString(found.value().filename);
48+
return WideStringToUtf8(found.value().filename);
4949
} else {
5050
fml::internal::os_win::UniqueFDTraits::RemoveCacheEntry(handle.get());
5151
}
@@ -59,7 +59,7 @@ static std::string GetFullHandlePath(const fml::UniqueFD& handle) {
5959
if (buffer_size == 0) {
6060
return {};
6161
}
62-
return WideStringToString({buffer, buffer_size});
62+
return WideStringToUtf8({buffer, buffer_size});
6363
#endif
6464
}
6565

@@ -106,7 +106,7 @@ static DWORD GetShareFlags(FilePermission permission) {
106106
}
107107

108108
static DWORD GetFileAttributesForUtf8Path(const char* absolute_path) {
109-
return ::GetFileAttributes(StringToWideString(absolute_path).c_str());
109+
return ::GetFileAttributes(Utf8ToWideString(absolute_path).c_str());
110110
}
111111

112112
static DWORD GetFileAttributesForUtf8Path(const fml::UniqueFD& base_directory,
@@ -147,15 +147,15 @@ std::string CreateTemporaryDirectory() {
147147
stream << temp_dir_container << "\\" << uuid_str;
148148
auto temp_dir = stream.str();
149149

150-
auto dir_fd = OpenDirectory(WideStringToString(temp_dir).c_str(), true,
150+
auto dir_fd = OpenDirectory(WideStringToUtf8(temp_dir).c_str(), true,
151151
FilePermission::kReadWrite);
152152
if (!dir_fd.is_valid()) {
153153
FML_DLOG(ERROR) << "Could not get temporary directory file descriptor. "
154154
<< GetLastErrorMessage();
155155
return {};
156156
}
157157

158-
return WideStringToString(std::move(temp_dir));
158+
return WideStringToUtf8(std::move(temp_dir));
159159
}
160160

161161
fml::UniqueFD OpenFile(const fml::UniqueFD& base_directory,
@@ -173,7 +173,7 @@ fml::UniqueFD OpenFile(const char* path,
173173
return {};
174174
}
175175

176-
auto file_name = StringToWideString({path});
176+
auto file_name = Utf8ToWideString({path});
177177

178178
if (file_name.size() == 0) {
179179
return {};
@@ -216,7 +216,7 @@ fml::UniqueFD OpenDirectory(const char* path,
216216
return {};
217217
}
218218

219-
auto file_name = StringToWideString({path});
219+
auto file_name = Utf8ToWideString({path});
220220

221221
if (file_name.size() == 0) {
222222
return {};
@@ -314,7 +314,7 @@ bool IsFile(const std::string& path) {
314314
}
315315

316316
bool UnlinkDirectory(const char* path) {
317-
if (!::RemoveDirectory(StringToWideString(path).c_str())) {
317+
if (!::RemoveDirectory(Utf8ToWideString(path).c_str())) {
318318
FML_DLOG(ERROR) << "Could not remove directory: '" << path << "'. "
319319
<< GetLastErrorMessage();
320320
return false;
@@ -324,7 +324,7 @@ bool UnlinkDirectory(const char* path) {
324324

325325
bool UnlinkDirectory(const fml::UniqueFD& base_directory, const char* path) {
326326
if (!::RemoveDirectory(
327-
StringToWideString(GetAbsolutePath(base_directory, path)).c_str())) {
327+
Utf8ToWideString(GetAbsolutePath(base_directory, path)).c_str())) {
328328
FML_DLOG(ERROR) << "Could not remove directory: '" << path << "'. "
329329
<< GetLastErrorMessage();
330330
return false;
@@ -333,7 +333,7 @@ bool UnlinkDirectory(const fml::UniqueFD& base_directory, const char* path) {
333333
}
334334

335335
bool UnlinkFile(const char* path) {
336-
if (!::DeleteFile(StringToWideString(path).c_str())) {
336+
if (!::DeleteFile(Utf8ToWideString(path).c_str())) {
337337
FML_DLOG(ERROR) << "Could not remove file: '" << path << "'. "
338338
<< GetLastErrorMessage();
339339
return false;
@@ -343,7 +343,7 @@ bool UnlinkFile(const char* path) {
343343

344344
bool UnlinkFile(const fml::UniqueFD& base_directory, const char* path) {
345345
if (!::DeleteFile(
346-
StringToWideString(GetAbsolutePath(base_directory, path)).c_str())) {
346+
Utf8ToWideString(GetAbsolutePath(base_directory, path)).c_str())) {
347347
FML_DLOG(ERROR) << "Could not remove file: '" << path << "'. "
348348
<< GetLastErrorMessage();
349349
return false;
@@ -436,8 +436,8 @@ bool WriteAtomically(const fml::UniqueFD& base_directory,
436436

437437
temp_file.reset();
438438

439-
if (!::MoveFile(StringToWideString(temp_file_path).c_str(),
440-
StringToWideString(file_path).c_str())) {
439+
if (!::MoveFile(Utf8ToWideString(temp_file_path).c_str(),
440+
Utf8ToWideString(file_path).c_str())) {
441441
FML_DLOG(ERROR)
442442
<< "Could not replace temp file at correct path. File path: "
443443
<< file_path << ". Temp file path: " << temp_file_path << " "
@@ -451,8 +451,8 @@ bool WriteAtomically(const fml::UniqueFD& base_directory,
451451
bool VisitFiles(const fml::UniqueFD& directory, const FileVisitor& visitor) {
452452
std::string search_pattern = GetFullHandlePath(directory) + "\\*";
453453
WIN32_FIND_DATA find_file_data;
454-
HANDLE find_handle = ::FindFirstFile(
455-
StringToWideString(search_pattern).c_str(), &find_file_data);
454+
HANDLE find_handle = ::FindFirstFile(Utf8ToWideString(search_pattern).c_str(),
455+
&find_file_data);
456456

457457
if (find_handle == INVALID_HANDLE_VALUE) {
458458
FML_DLOG(ERROR) << "Can't open the directory. Error: "
@@ -461,7 +461,7 @@ bool VisitFiles(const fml::UniqueFD& directory, const FileVisitor& visitor) {
461461
}
462462

463463
do {
464-
std::string filename = WideStringToString(find_file_data.cFileName);
464+
std::string filename = WideStringToUtf8(find_file_data.cFileName);
465465
if (filename != "." && filename != "..") {
466466
if (!visitor(directory, filename)) {
467467
::FindClose(find_handle);

‎fml/platform/win/native_library_win.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ NativeLibrary::NativeLibrary(const char* path)
1616
return;
1717
}
1818

19-
handle_ = ::LoadLibrary(StringToWideString(path).c_str());
19+
handle_ = ::LoadLibrary(Utf8ToWideString(path).c_str());
2020
}
2121

2222
NativeLibrary::NativeLibrary(Handle handle, bool close_handle)

‎fml/platform/win/wstring_conversion.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "flutter/fml/platform/win/wstring_conversion.h"
6+
7+
#include <codecvt>
8+
#include <locale>
9+
#include <string>
10+
11+
namespace fml {
12+
13+
using WideStringConverter =
14+
std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>, wchar_t>;
15+
16+
std::string WideStringToUtf8(const std::wstring_view str) {
17+
WideStringConverter converter;
18+
return converter.to_bytes(str.data());
19+
}
20+
21+
std::wstring Utf8ToWideString(const std::string_view str) {
22+
WideStringConverter converter;
23+
return converter.from_bytes(str.data());
24+
}
25+
26+
} // namespace fml

‎fml/platform/win/wstring_conversion.h

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,16 @@
55
#ifndef FLUTTER_FML_PLATFORM_WIN_WSTRING_CONVERSION_H_
66
#define FLUTTER_FML_PLATFORM_WIN_WSTRING_CONVERSION_H_
77

8-
#include <codecvt>
9-
#include <locale>
108
#include <string>
119

1210
namespace fml {
1311

14-
using WideStringConvertor =
15-
std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>, wchar_t>;
12+
// Returns a UTF-8 encoded equivalent of a UTF-16 encoded input wide string.
13+
std::string WideStringToUtf8(const std::wstring_view str);
1614

17-
inline std::wstring StringToWideString(const std::string& str) {
18-
WideStringConvertor converter;
19-
return converter.from_bytes(str);
20-
}
21-
22-
inline std::string WideStringToString(const std::wstring& wstr) {
23-
WideStringConvertor converter;
24-
return converter.to_bytes(wstr);
25-
}
15+
// Returns a UTF-16 encoded wide string equivalent of a UTF-8 encoded input
16+
// string.
17+
std::wstring Utf8ToWideString(const std::string_view str);
2618

2719
} // namespace fml
2820

‎fml/platform/win/wstring_conversion_unittests.cc

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "flutter/fml/platform/win/wstring_conversion.h"
6+
7+
#include "gtest/gtest.h"
8+
9+
namespace fml {
10+
namespace testing {
11+
12+
TEST(StringConversion, Utf16ToWideStringEmpty) {
13+
EXPECT_EQ(Utf8ToWideString(""), L"");
14+
}
15+
16+
TEST(StringConversion, Utf8ToWideStringAscii) {
17+
EXPECT_EQ(Utf8ToWideString("abc123"), L"abc123");
18+
}
19+
20+
TEST(StringConversion, Utf8ToWideStringUnicode) {
21+
EXPECT_EQ(Utf8ToWideString("\xe2\x98\x83"), L"\x2603");
22+
}
23+
24+
TEST(StringConversion, WideStringToUtf8Empty) {
25+
EXPECT_EQ(WideStringToUtf8(L""), "");
26+
}
27+
28+
TEST(StringConversion, WideStringToUtf8Ascii) {
29+
EXPECT_EQ(WideStringToUtf8(L"abc123"), "abc123");
30+
}
31+
32+
TEST(StringConversion, WideStringToUtf8Unicode) {
33+
EXPECT_EQ(WideStringToUtf8(L"\x2603"), "\xe2\x98\x83");
34+
}
35+
36+
} // namespace testing
37+
} // namespace fml

‎fml/string_conversion.cc

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "flutter/fml/string_conversion.h"
6+
7+
#include <codecvt>
8+
#include <locale>
9+
#include <string>
10+
11+
#include "flutter/fml/build_config.h"
12+
13+
#if defined(FML_OS_WIN)
14+
// TODO(naifu): https://github.com/flutter/flutter/issues/98074
15+
// Eliminate this workaround for a link error on Windows when the underlying
16+
// bug is fixed.
17+
std::locale::id std::codecvt<char16_t, char, _Mbstatet>::id;
18+
#endif // defined(FML_OS_WIN)
19+
20+
namespace fml {
21+
22+
using Utf16StringConverter =
23+
std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t>;
24+
25+
std::string Utf16ToUtf8(const std::u16string_view string) {
26+
Utf16StringConverter converter;
27+
return converter.to_bytes(string.data());
28+
}
29+
30+
std::u16string Utf8ToUtf16(const std::string_view string) {
31+
Utf16StringConverter converter;
32+
return converter.from_bytes(string.data());
33+
}
34+
35+
} // namespace fml

0 commit comments

Comments
 (0)