Skip to content

Commit 1c9457c

Browse files
authored
Revert "Only allow mappings for ICU initialization. (#8656)" (#8682)
This reverts commit bd8c5b1. Reverts flutter/engine#8656 Reason: flutter/engine#8656 seems to break the framework windows tests and the engine roll (see https://cirrus-ci.com/task/4704667236827136 and #31330). The failure has been consistent for 7 consecutive engine-to-framework auto-rolls. TBR: @chinmaygarde
1 parent b4ed303 commit 1c9457c

File tree

12 files changed

+125
-61
lines changed

12 files changed

+125
-61
lines changed

benchmarking/benchmarking.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ namespace benchmarking {
1010

1111
int Main(int argc, char** argv) {
1212
benchmark::Initialize(&argc, argv);
13-
fml::icu::InitializeICU(fml::FileMapping::CreateReadOnly(
14-
fml::OpenDirectoryOfExecutable(), "icudtl.dat"));
13+
fml::icu::InitializeICU("icudtl.dat");
1514
::benchmark::RunSpecifiedBenchmarks();
1615
return 0;
1716
}

common/settings.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,12 @@ struct Settings {
134134
bool verbose_logging = false;
135135
std::string log_tag = "flutter";
136136

137+
// The icu_initialization_required setting does not have a corresponding
138+
// switch because it is intended to be decided during build time, not runtime.
139+
// Some companies apply source modification here because their build system
140+
// brings its own ICU data files.
141+
bool icu_initialization_required = true;
142+
std::string icu_data_path;
137143
MappingCallback icu_mapper;
138144

139145
// Assets settings

fml/file.cc

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
#include "flutter/fml/file.h"
66

77
#include "flutter/fml/logging.h"
8-
#include "flutter/fml/paths.h"
98

109
namespace fml {
1110

@@ -59,14 +58,4 @@ ScopedTemporaryDirectory::~ScopedTemporaryDirectory() {
5958
}
6059
}
6160

62-
fml::UniqueFD OpenDirectoryOfExecutable() {
63-
auto result = paths::GetExecutableDirectoryPath();
64-
65-
if (!result.first) {
66-
return {};
67-
}
68-
69-
return OpenFile(result.second.c_str(), false, FilePermission::kRead);
70-
}
71-
7261
} // namespace fml

fml/file.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ fml::UniqueFD OpenDirectory(const fml::UniqueFD& base_directory,
4646
bool create_if_necessary,
4747
FilePermission permission);
4848

49-
fml::UniqueFD OpenDirectoryOfExecutable();
50-
5149
fml::UniqueFD Duplicate(fml::UniqueFD::element_type descriptor);
5250

5351
bool IsDirectory(const fml::UniqueFD& directory);

fml/icu_util.cc

Lines changed: 99 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,115 @@
44

55
#include "flutter/fml/icu_util.h"
66

7+
#include <memory>
78
#include <mutex>
89

10+
#include "flutter/fml/build_config.h"
911
#include "flutter/fml/logging.h"
12+
#include "flutter/fml/mapping.h"
13+
#include "flutter/fml/native_library.h"
14+
#include "flutter/fml/paths.h"
1015
#include "third_party/icu/source/common/unicode/udata.h"
1116

1217
namespace fml {
1318
namespace icu {
1419

15-
void InitializeICU(std::unique_ptr<const Mapping> mapping) {
16-
if (mapping == nullptr || mapping->GetSize() == 0) {
17-
return;
20+
class ICUContext {
21+
public:
22+
ICUContext(const std::string& icu_data_path) : valid_(false) {
23+
valid_ = SetupMapping(icu_data_path) && SetupICU();
1824
}
19-
static std::once_flag g_icu_init_flag;
20-
std::call_once(g_icu_init_flag, [mapping = std::move(mapping)]() mutable {
21-
static auto icu_mapping = std::move(mapping);
25+
26+
ICUContext(std::unique_ptr<Mapping> mapping) : mapping_(std::move(mapping)) {
27+
valid_ = SetupICU();
28+
}
29+
30+
~ICUContext() = default;
31+
32+
bool SetupMapping(const std::string& icu_data_path) {
33+
// Check if the path exists and it readable directly.
34+
auto fd =
35+
fml::OpenFile(icu_data_path.c_str(), false, fml::FilePermission::kRead);
36+
37+
// Check the path relative to the current executable.
38+
if (!fd.is_valid()) {
39+
auto directory = fml::paths::GetExecutableDirectoryPath();
40+
41+
if (!directory.first) {
42+
return false;
43+
}
44+
45+
std::string path_relative_to_executable =
46+
paths::JoinPaths({directory.second, icu_data_path});
47+
48+
fd = fml::OpenFile(path_relative_to_executable.c_str(), false,
49+
fml::FilePermission::kRead);
50+
}
51+
52+
if (!fd.is_valid()) {
53+
return false;
54+
}
55+
56+
std::initializer_list<FileMapping::Protection> protection = {
57+
fml::FileMapping::Protection::kRead};
58+
59+
auto file_mapping =
60+
std::make_unique<FileMapping>(fd, std::move(protection));
61+
62+
if (file_mapping->GetSize() != 0) {
63+
mapping_ = std::move(file_mapping);
64+
return true;
65+
}
66+
67+
return false;
68+
}
69+
70+
bool SetupICU() {
71+
if (GetSize() == 0) {
72+
return false;
73+
}
74+
2275
UErrorCode err_code = U_ZERO_ERROR;
23-
udata_setCommonData(icu_mapping->GetMapping(), &err_code);
24-
FML_CHECK(err_code == U_ZERO_ERROR) << "Must be able to initialize ICU.";
76+
udata_setCommonData(GetMapping(), &err_code);
77+
return (err_code == U_ZERO_ERROR);
78+
}
79+
80+
const uint8_t* GetMapping() const {
81+
return mapping_ ? mapping_->GetMapping() : nullptr;
82+
}
83+
84+
size_t GetSize() const { return mapping_ ? mapping_->GetSize() : 0; }
85+
86+
bool IsValid() const { return valid_; }
87+
88+
private:
89+
bool valid_;
90+
std::unique_ptr<Mapping> mapping_;
91+
92+
FML_DISALLOW_COPY_AND_ASSIGN(ICUContext);
93+
};
94+
95+
void InitializeICUOnce(const std::string& icu_data_path) {
96+
static ICUContext* context = new ICUContext(icu_data_path);
97+
FML_CHECK(context->IsValid())
98+
<< "Must be able to initialize the ICU context. Tried: " << icu_data_path;
99+
}
100+
101+
std::once_flag g_icu_init_flag;
102+
void InitializeICU(const std::string& icu_data_path) {
103+
std::call_once(g_icu_init_flag,
104+
[&icu_data_path]() { InitializeICUOnce(icu_data_path); });
105+
}
106+
107+
void InitializeICUFromMappingOnce(std::unique_ptr<Mapping> mapping) {
108+
static ICUContext* context = new ICUContext(std::move(mapping));
109+
FML_CHECK(context->IsValid())
110+
<< "Unable to initialize the ICU context from a mapping.";
111+
}
112+
113+
void InitializeICUFromMapping(std::unique_ptr<Mapping> mapping) {
114+
std::call_once(g_icu_init_flag, [mapping = std::move(mapping)]() mutable {
115+
InitializeICUFromMappingOnce(std::move(mapping));
25116
});
26117
}
27118

fml/icu_util.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
namespace fml {
1414
namespace icu {
1515

16-
void InitializeICU(std::unique_ptr<const Mapping> mapping);
16+
void InitializeICU(const std::string& icu_data_path = "");
17+
18+
void InitializeICUFromMapping(std::unique_ptr<Mapping> mapping);
1719

1820
} // namespace icu
1921
} // namespace fml

shell/common/shell.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,14 @@ static void PerformInitializationTasks(const Settings& settings) {
193193
FML_DLOG(INFO) << "Skia deterministic rendering is enabled.";
194194
}
195195

196-
if (settings.icu_mapper) {
197-
fml::icu::InitializeICU(settings.icu_mapper());
196+
if (settings.icu_initialization_required) {
197+
if (settings.icu_data_path.size() != 0) {
198+
fml::icu::InitializeICU(settings.icu_data_path);
199+
} else if (settings.icu_mapper) {
200+
fml::icu::InitializeICUFromMapping(settings.icu_mapper());
201+
} else {
202+
FML_DLOG(WARNING) << "Skipping ICU initialization in the shell.";
203+
}
198204
}
199205
});
200206
}

shell/common/switches.cc

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -242,20 +242,9 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) {
242242
command_line.GetOptionValue(FlagForSwitch(Switch::CacheDirPath),
243243
&settings.temp_directory_path);
244244

245-
{
246-
// ICU from a data file.
247-
std::string icu_data_path;
245+
if (settings.icu_initialization_required) {
248246
command_line.GetOptionValue(FlagForSwitch(Switch::ICUDataFilePath),
249-
&icu_data_path);
250-
if (icu_data_path.size() > 0) {
251-
settings.icu_mapper = [icu_data_path]() {
252-
return fml::FileMapping::CreateReadOnly(icu_data_path);
253-
};
254-
}
255-
}
256-
257-
{
258-
// ICU from a symbol in a dynamic library
247+
&settings.icu_data_path);
259248
if (command_line.HasOption(FlagForSwitch(Switch::ICUSymbolPrefix))) {
260249
std::string icu_symbol_prefix, native_lib_path;
261250
command_line.GetOptionValue(FlagForSwitch(Switch::ICUSymbolPrefix),

shell/platform/darwin/ios/framework/Source/FlutterDartProject.mm

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,10 @@
5959
// defaults.
6060

6161
// Flutter ships the ICU data file in the the bundle of the engine. Look for it there.
62-
if (!settings.icu_mapper) {
62+
if (settings.icu_data_path.size() == 0) {
6363
NSString* icuDataPath = [engineBundle pathForResource:@"icudtl" ofType:@"dat"];
6464
if (icuDataPath.length > 0) {
65-
auto icu_data_path = std::string{icuDataPath.UTF8String};
66-
settings.icu_mapper = [icu_data_path]() {
67-
return fml::FileMapping::CreateReadOnly(icu_data_path);
68-
};
65+
settings.icu_data_path = icuDataPath.UTF8String;
6966
}
7067
}
7168

shell/platform/embedder/embedder.cc

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -370,12 +370,7 @@ FlutterEngineResult FlutterEngineRun(size_t version,
370370

371371
PopulateSnapshotMappingCallbacks(args, settings);
372372

373-
if (!settings.icu_mapper) {
374-
settings.icu_mapper = [icu_data_path]() {
375-
return fml::FileMapping::CreateReadOnly(icu_data_path);
376-
};
377-
}
378-
373+
settings.icu_data_path = icu_data_path;
379374
settings.assets_path = args->assets_path;
380375

381376
if (!flutter::DartVM::IsRunningPrecompiledCode()) {

shell/testing/tester_main.cc

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -251,14 +251,8 @@ int main(int argc, char* argv[]) {
251251
return EXIT_FAILURE;
252252
}
253253

254-
// Using command line arguments, the user can specify the ICU data as being
255-
// present in either a file or a dynamic library. If no such specification has
256-
// been, default to icudtl.dat.
257-
if (!settings.icu_mapper) {
258-
settings.icu_mapper = []() {
259-
return fml::FileMapping::CreateReadOnly(fml::OpenDirectoryOfExecutable(),
260-
"icudtl.dat");
261-
};
254+
if (settings.icu_data_path.size() == 0) {
255+
settings.icu_data_path = "icudtl.dat";
262256
}
263257

264258
// The tools that read logs get confused if there is a log tag specified.

third_party/txt/tests/txt_run_all_unittests.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@ int main(int argc, char** argv) {
3636
}
3737
FML_DCHECK(txt::GetFontDir().length() > 0);
3838

39-
fml::icu::InitializeICU(fml::FileMapping::CreateReadOnly(
40-
fml::OpenDirectoryOfExecutable(), "icudtl.dat"));
41-
39+
fml::icu::InitializeICU("icudtl.dat");
4240
SkGraphics::Init();
4341
testing::InitGoogleTest(&argc, argv);
4442
return RUN_ALL_TESTS();

0 commit comments

Comments
 (0)