linux: set env FC_FONTATIONS and EGL_PLATFORM early [chromium/src : main]
Wanchang Ryu (Gerrit)
Wanchang Ryu voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu added 1 comment![]( "Open in Gerrit")
Hi.
I have crash issue on our device sometimes and I found that it was becasue set/getenv race condition with multi-thread.
I think calling setenv before creating threads would fix the issue.
Could you take a look ?
Related details
- Arthur Sonzogni
- Maksim Sisov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maksim Sisov (Gerrit)
Maksim Sisov added 5 comments![]( "Open in Gerrit")
"+ui/ozone",nit: limit to `"ui/ozone/platform_selection.h"` ?
#if BUILDFLAG(IS_LINUX)The original `FC_FONTATIONS` setenv lived in `fontconfig_util.cc`, which builds for `IS_LINUX || IS_CHROMEOS` (afaik). The new code uses `#if BUILDFLAG(IS_LINUX)` only. Can you please fix that?
environment->SetVar("EGL_PLATFORM", "wayland");`base::Environment::SetVar()` hardcodes setenv(name, value, 1) and always overwrites, doesn't it? If a user sets EGL_PLATFORM to something else before running Chrome, the original code preserved that choice. The new code steamrolls it.
```suggestion
if (!environment->HasVar("EGL_PLATFORM")) {
environment->SetVar("EGL_PLATFORM", "wayland");
}
```
// TODO: It may not be necessary to set this environment variable when using
// swiftshader.can you move this comment to the new place as well? you can use `40083534` for the todo
Related details
- Arthur Sonzogni
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu voted and added 6 comments![]( "Open in Gerrit")
Votes added by Wanchang Ryu
| Commit-Queue | +1 |
6 comments
Thanks for review.
I addressed comments in the patchset2.
Please take a look.
nit: limit to `"ui/ozone/platform_selection.h"` ?
Done
The original `FC_FONTATIONS` setenv lived in `fontconfig_util.cc`, which builds for `IS_LINUX || IS_CHROMEOS` (afaik). The new code uses `#if BUILDFLAG(IS_LINUX)` only. Can you please fix that?
Thanks! I missed the point.
I checked https://chromium.googlesource.com/chromium/src/+/08400dc072cbffd20d7e95d0b877071350a48a90/ui/gfx/BUILD.gn#234
I will fixed with `IS_LINUX || IS_CHROMEOS`
`base::Environment::SetVar()` hardcodes setenv(name, value, 1) and always overwrites, doesn't it? If a user sets EGL_PLATFORM to something else before running Chrome, the original code preserved that choice. The new code steamrolls it.
```suggestion
if (!environment->HasVar("EGL_PLATFORM")) {
environment->SetVar("EGL_PLATFORM", "wayland");
}
```
Thanks! I checked and you're right.
I will fix with your suggestion.
// TODO: It may not be necessary to set this environment variable when using
// swiftshader.can you move this comment to the new place as well? you can use `40083534` for the todo
I see. I will move this comment to the new place with TODO(40083534):
Related details
- Arthur Sonzogni
- Maksim Sisov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu added 1 comment![]( "Open in Gerrit")
- Maksim Sisov
- Nasko Oskov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maksim Sisov (Gerrit)
Maksim Sisov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Maksim Sisov
| Code-Review | +1 |
1 comment
- Nasko Oskov
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu voted and added 1 comment![]( "Open in Gerrit")
Votes added by Wanchang Ryu
| Commit-Queue | +1 |
Related details
- Maksim Sisov
- Nasko Oskov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maksim Sisov (Gerrit)
Maksim Sisov added 1 comment![]( "Open in Gerrit")
if (ui::GetOzonePlatformId() == ui::kPlatformWayland) {The ChromeOS compile failure is because `kPlatformWayland` is a generated constant that only exists when Wayland is in the platform list.
Instead of adding more ifdefs, consider moving the `EGL_PLATFORM` setenv into the ozone layer itself. `OzonePlatform::PreEarlyInitialize()` is a virtual method called from `PreEarlyInitialization()` during `BasicStartupComplete()`, well before any threads are created. The Wayland platform can just override it:
```cpp
// In ozone_platform_wayland.cc
void OzonePlatformWayland::PreEarlyInitialize() override {
// TODO(40083534): It may not be necessary to set this environment
// variable when using swiftshader.
setenv("EGL_PLATFORM", "wayland", 0);
}
```
This eliminates the `//ui/ozone` dependency in `content/app`, the nested ifdefs, and the compile failure.
Related details
- Nasko Oskov
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu added 1 comment![]( "Open in Gerrit")
Looks build failed on ozone_platform_wayland=false.
I need to check how to take account into the value on the source code.
Related details
- Nasko Oskov
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu added 1 comment![]( "Open in Gerrit")
if (ui::GetOzonePlatformId() == ui::kPlatformWayland) {The ChromeOS compile failure is because `kPlatformWayland` is a generated constant that only exists when Wayland is in the platform list.
Instead of adding more ifdefs, consider moving the `EGL_PLATFORM` setenv into the ozone layer itself. `OzonePlatform::PreEarlyInitialize()` is a virtual method called from `PreEarlyInitialization()` during `BasicStartupComplete()`, well before any threads are created. The Wayland platform can just override it:
```cpp
// In ozone_platform_wayland.cc
void OzonePlatformWayland::PreEarlyInitialize() override {
// TODO(40083534): It may not be necessary to set this environment
// variable when using swiftshader.
setenv("EGL_PLATFORM", "wayland", 0);
}
```This eliminates the `//ui/ozone` dependency in `content/app`, the nested ifdefs, and the compile failure.
- Maksim Sisov
- Nasko Oskov
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu added 1 comment![]( "Open in Gerrit")
if (ui::GetOzonePlatformId() == ui::kPlatformWayland) {Wanchang RyuThe ChromeOS compile failure is because `kPlatformWayland` is a generated constant that only exists when Wayland is in the platform list.
Instead of adding more ifdefs, consider moving the `EGL_PLATFORM` setenv into the ozone layer itself. `OzonePlatform::PreEarlyInitialize()` is a virtual method called from `PreEarlyInitialization()` during `BasicStartupComplete()`, well before any threads are created. The Wayland platform can just override it:
```cpp
// In ozone_platform_wayland.cc
void OzonePlatformWayland::PreEarlyInitialize() override {
// TODO(40083534): It may not be necessary to set this environment
// variable when using swiftshader.
setenv("EGL_PLATFORM", "wayland", 0);
}
```This eliminates the `//ui/ozone` dependency in `content/app`, the nested ifdefs, and the compile failure.
Thanks for the comment, I will check.
I checked when PreEarlyInitialize is called and it is called after creating two more threads.
Here is callstack/threads of before calling OzonePlatformWayland::PreEarlyInitialize().
>Thread 1 "chrome" hit Breakpoint 2, 0x000055556d12b560 in ui::OzonePlatform::PreEarlyInitialization()@plt ()
(gdb) bt
#0 0x000055556d12b560 in ui::OzonePlatform::PreEarlyInitialization()@plt ()
#1 0x000055555d8d1c3d in ChromeMainDelegate::PostEarlyInitialization(std::__Cr::variant<content::ContentMainDelegate::InvokedInBrowserProcess, content::ContentMainDelegate::InvokedInChildProcess>) (this=0x7fffffffd928, invoked_in=...) at ../../chrome/app/chrome_main_delegate.cc:837
#2 0x00007fffdade8534 in content::ContentMainRunnerImpl::RunBrowser(content::MainFunctionParams, bool) (this=0x29a8000d8000, main_params=..., start_minimal_browser=false)
at ../../content/app/content_main_runner_impl.cc:1246
#3 0x00007fffdade80a7 in content::ContentMainRunnerImpl::Run() (this=0x29a8000d8000) at ../../content/app/content_main_runner_impl.cc:1178
#4 0x00007fffdade3f5d in content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) (params=..., content_main_runner=0x29a8000d8000)
at ../../content/app/content_main.cc:358
#5 0x00007fffdade4466 in content::ContentMain(content::ContentMainParams) (params=...) at ../../content/app/content_main.cc:371
#6 0x000055555d8d0060 in ChromeMain(int, char const**) (argc=1, argv=0x7fffffffdb08) at ../../chrome/app/chrome_main.cc:191
#7 0x000055555d8cfd12 in main(int, char const**) (argc=1, argv=0x7fffffffdb08) at ../../chrome/app/chrome_exe_main_aura.cc:17
(gdb) info thread
Id Target Id Frame
* 1 Thread 0x7fff55f72ec0 (LWP 3925024) "chrome" 0x000055556d12b560 in ui::OzonePlatform::PreEarlyInitialization()@plt ()
2 Thread 0x7fff543ff640 (LWP 3926045) "sandbox_ipc_thr" 0x00007fff79b18c3f in __GI___poll (fds=0x7fff543f45d0, nfds=2, timeout=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
3 Thread 0x7fff53bfe640 (LWP 3926051) "chrome" 0x00007fff79aea42f in __GI___wait4 (pid=3926048, stat_loc=0x7fff53bf352c, options=0, usage=0x0)
at ../sysdeps/unix/sysv/linux/wait4.c:30
(gdb)
Created threads are related to zygote (created during InitializeZygoteSandboxForBrowserProcess() call)
So I prefer to keep current location to call setenv to ensure that we don't have any other threads.
I'd like to replace ui::GetOzonePlatformId() with ui::GetOzonePlatformName() to use string compare.
Related details
- Maksim Sisov
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu added 2 comments![]( "Open in Gerrit")
Looks build failed on ozone_platform_wayland=false.
I need to check how to take account into the value on the source code.
- Maksim Sisov
- Nasko Oskov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maksim Sisov (Gerrit)
Maksim Sisov added 1 comment![]( "Open in Gerrit")
Regarding the `EGL_PLATFORM` move: `wayland_surface_factory.cc` usually runs in the GPU process, while the new `setenv` in `content_main_runner_impl.cc` runs in the browser process. Can you verify that the GPU process actually gets `EGL_PLATFORM` set?
The GPU process seems to be forked from the zygote, which is created after the proposed `setenv` location, so fork inheritance might cover it — but it would be good to confirm (e.g., break in `LoadGLES2Bindings` in a GPU process and check `getenv("EGL_PLATFORM")`).
If it doesn't propagate, the safest approach would be to keep the existing setenv in wayland_surface_factory.cc as-is and only add the early setenv for the in-process-gpu race fix.
Also, I'd prefer that `ozone`-related env vars stay inside `//ui/ozone` rather than being set from `//content`. Exposing backend-specific details like `EGL_PLATFORM=wayland` in c`ontent_main_runner_impl.cc` is a layering concern — content shouldn't need to know which ozone backend is in use. I understand `PreEarlyInitialize()` runs too late (after zygote threads), but perhaps there's a better hook within ozone that runs earlier, or the `setenv` could stay in `wayland_surface_factory.cc` with an additional early call somewhere in the ozone init path.
Related details
- Nasko Oskov
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu added 1 comment![]( "Open in Gerrit")
Can you verify that the GPU process actually gets EGL_PLATFORM set?
Yes, env is set before creating zygote so it propagates to child processes.
I confirmed by adding/checking logs.
Also, I'd prefer that ozone-related env vars stay inside //ui/ozone rather than being set from //content
Yes, I also agree with this preference. There is PreSandboxStartup hook which is called right before the location I put calling setenv.
Changing current ui::OzonePlatform::PreEarlyInitialize name to ui::OzonePlatform::PreSandboxStartup and add hook on PreSandboxStartup with checking browser process would work.
I will make this change, if this is acceptable.
Related details
- Maksim Sisov
- Nasko Oskov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maksim Sisov (Gerrit)
Maksim Sisov added 1 comment![]( "Open in Gerrit")
Thanks for checking. It seems `PreEarlyInitialize` isn't even used. Must be safe to rename and use for this case then.
Related details
- Nasko Oskov
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu added 1 comment![]( "Open in Gerrit")
Could you take a look ?
- Gabriel Charette
- Greg Thompson
- Kramer Ge
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu added 1 comment![]( "Open in Gerrit")
- Avi Drissman
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Anderson (Gerrit)
Thomas Anderson voted and added 2 comments![]( "Open in Gerrit")
Votes added by Thomas Anderson
| Code-Review | +1 |
2 comments
#include "ui/gfx/linux/fontconfig_util.h"is this include needed?
// Set environment variables for fontconfig fontations indexing and Wayland
// EGL platform before creating threads.remove
Related details
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kramer Ge (Gerrit)
Kramer Ge voted and added 2 comments![]( "Open in Gerrit")
Votes added by Kramer Ge
| Code-Review | +1 |
2 comments
// TODO(40083534): It may not be necessary to set this environment variable // PreSandboxStartup, PreEarlyInitialization, PostCreateMainMessageLoop,nit: this signature is removed.
Related details
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maksim Sisov (Gerrit)
Maksim Sisov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Maksim Sisov
| Code-Review | +1 |
1 comment
lgtm with nits from thomasanderson@ and fangzhoug@
Wanchang Ryu (Gerrit)
Wanchang Ryu voted and added 5 comments![]( "Open in Gerrit")
Votes added by Wanchang Ryu
| Commit-Queue | +1 |
5 comments
#include "ui/gfx/linux/fontconfig_util.h"Wanchang Ryuis this include needed?
Right, It can be removed.
// Set environment variables for fontconfig fontations indexing and Wayland
// EGL platform before creating threads.Wanchang Ryuremove
Done
// TODO(40083534): It may not be necessary to set this environment variable // PreSandboxStartup, PreEarlyInitialization, PostCreateMainMessageLoop,nit: this signature is removed.
- Gabriel Charette
- Kramer Ge
- Maksim Sisov
- Nico Weber
- Thomas Anderson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gabriel Charette (Gerrit)
Gabriel Charette voted and added 1 comment![]( "Open in Gerrit")
Votes added by Gabriel Charette
| Code-Review | +1 |
1 comment
- Kramer Ge
- Maksim Sisov
- Nico Weber
- Thomas Anderson
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kramer Ge (Gerrit)
Kramer Ge voted and added 1 comment![]( "Open in Gerrit")
Votes added by Kramer Ge
| Code-Review | +1 |
1 comment
- Maksim Sisov
- Nico Weber
- Thomas Anderson
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nico Weber (Gerrit)
Nico Weber added 1 comment![]( "Open in Gerrit")
Can we get someone who's familiar with the linux sandbox review this?
(I know this would cause issues with the mac sandbox, and that permanently allows things that happen pre-warmup post-warmup.)
Related details
- Maksim Sisov
- Thomas Anderson
- Wanchang Ryu
Wanchang Ryu (Gerrit)
Wanchang Ryu added 1 comment![]( "Open in Gerrit")
Can we get someone who's familiar with the linux sandbox review this?
(I know this would cause issues with the mac sandbox, and that permanently allows things that happen pre-warmup post-warmup.)
Hi.
Thanks for the comment.
Changed code is under browser process by checking `if (process_type.empty())`
The browser process is not entering sandbox so I think it is safe for your concern.
Please let me know if I missed your point.
Related details
- Maksim Sisov
- Nico Weber
- Thomas Anderson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nico Weber (Gerrit)
Nico Weber added 3 comments![]( "Open in Gerrit")
This probably looks good now.
You care about "pre thread creation" and not so much about "pre sandbox", yes? Or did I get that wrong?
pre-sandbox phase in the browser process.If you only care about the browser process, you don't care about this being pre-sandbox but only about it being pre-thread-creation, yes? Could you reword the CL description?
setenv/getenv.Can you say _why_ we have to do this before sandbox initialization? (edit: I think you probably don't care about that, see above.)
Can you also mention that this now longer initializes ozone at all for most process types (afaiu)?
Related details
- Maksim Sisov
- Thomas Anderson
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu voted and added 2 comments![]( "Open in Gerrit")
Votes added by Wanchang Ryu
| Commit-Queue | +1 |
2 comments
If you only care about the browser process, you don't care about this being pre-sandbox but only about it being pre-thread-creation, yes? Could you reword the CL description?
Right, I care about pre-thread-creation on browser process.
I will reword.
Can you say _why_ we have to do this before sandbox initialization? (edit: I think you probably don't care about that, see above.)
Can you also mention that this now longer initializes ozone at all for most process types (afaiu)?
ozone initialization was done by ChromeMainDelegate::PostEarlyInitialization when invoked_in_browser is true.
(I checked https://chromium-review.googlesource.com/c/chromium/src/+/7725187/10/chrome/app/chrome_main_delegate.cc#b751 and https://chromium-review.googlesource.com/c/chromium/src/+/7725187/10/chrome/app/chrome_main_delegate.cc#b831)
So I think it was done only under browser process previously as well.
Also I would better remove `before sandboxing` here because required condition is only before-thread-creation.
Related details
- Maksim Sisov
- Nico Weber
- Thomas Anderson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu added 2 comments![]( "Open in Gerrit")
Ping @tha...@chromium.org
Could you let me know if I need to address something more ?
setenv/getenv.Wanchang RyuCan you say _why_ we have to do this before sandbox initialization? (edit: I think you probably don't care about that, see above.)
Can you also mention that this now longer initializes ozone at all for most process types (afaiu)?
ozone initialization was done by ChromeMainDelegate::PostEarlyInitialization when invoked_in_browser is true.
(I checked https://chromium-review.googlesource.com/c/chromium/src/+/7725187/10/chrome/app/chrome_main_delegate.cc#b751 and https://chromium-review.googlesource.com/c/chromium/src/+/7725187/10/chrome/app/chrome_main_delegate.cc#b831)So I think it was done only under browser process previously as well.
Also I would better remove `before sandboxing` here because required condition is only before-thread-creation.
Can you also mention that this now longer initializes ozone at all for most process types (afaiu)?
I think this change doesn't increase initialization ozone since it changes only phase to call initialization and it is used only under browser process.
Related details
- Maksim Sisov
- Nico Weber
- Thomas Anderson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nico Weber (Gerrit)
Nico Weber voted and added 1 comment![]( "Open in Gerrit")
Votes added by Nico Weber
| Code-Review | +1 |
1 comment
// sandbox and threads have not been crated at the moment. It is useful fornit: typo "crated"
Related details
- Maksim Sisov
- Thomas Anderson
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Röttsches (Gerrit)
Dominik Röttsches added 1 comment![]( "Open in Gerrit")
// Use Fontations, instead of FreeType, indexing in FontConfig.Can we perhaps add a comment here that this needs the environment variable to be set and point to where that is done in the new init function?
Related details
- Maksim Sisov
- Thomas Anderson
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maksim Sisov (Gerrit)
Maksim Sisov voted Code-Review+1![]( "Open in Gerrit")
- Thomas Anderson
- Wanchang Ryu
Wanchang Ryu (Gerrit)
Wanchang Ryu voted and added 3 comments![]( "Open in Gerrit")
Votes added by Wanchang Ryu
| Commit-Queue | +1 |
3 comments
// Use Fontations, instead of FreeType, indexing in FontConfig.Can we perhaps add a comment here that this needs the environment variable to be set and point to where that is done in the new init function?
Thanks for the comment.
Done.
// sandbox and threads have not been crated at the moment. It is useful fornit: typo "crated"
- Dominik Röttsches
- Gabriel Charette
- Kramer Ge
- Maksim Sisov
- Nico Weber
- Thomas Anderson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Röttsches (Gerrit)
Dominik Röttsches voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dominik Röttsches
| Code-Review | +1 |
1 comment
// Environment variable FC_FONTATIONS=1 is required here to use Fontations,Comment LGTM, thanks for adding that and the explanation!
Related details
- Gabriel Charette
- Kramer Ge
- Maksim Sisov
- Nico Weber
- Thomas Anderson
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Wanchang Ryu (Gerrit)
Wanchang Ryu voted and added 1 comment![]( "Open in Gerrit")
Votes added by Wanchang Ryu
| Commit-Queue | +2 |
1 comment
// Environment variable FC_FONTATIONS=1 is required here to use Fontations,Comment LGTM, thanks for adding that and the explanation!
Thanks for confirmation!
Related details
- Gabriel Charette
- Kramer Ge
- Maksim Sisov
- Nico Weber
- Thomas Anderson
Wanchang Ryu (Gerrit)
Wanchang Ryu added 1 comment![]( "Open in Gerrit")
I think this change is all set. Could any reviewer please vote Review+1 to commit ?
Steinar H Gunderson (Gerrit)
Steinar H Gunderson voted Code-Review+1![]( "Open in Gerrit")
- Gabriel Charette
- Kramer Ge
- Maksim Sisov
- Nico Weber
- Thomas Anderson
- Wanchang Ryu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dominik Röttsches (Gerrit)
Dominik Röttsches voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
linux: set env FC_FONTATIONS and EGL_PLATFORM early
Move Linux/ChromeOS environment setup and Ozone pre-init work to the
pre-thread-creation phase in the browser process.
- Rename OzonePlatform::PreEarlyInitialization() to
OzonePlatform::PreSandboxStartup().
- Rename virtual hook PreEarlyInitialize() to OnPreSandboxStartup().
- Call Ozone pre-sandbox startup from
ChromeMainDelegate::PreSandboxStartup() (browser process only).
- Set FC_FONTATIONS=1 in ContentMainRunnerImpl::Initialize() before
threads are created, instead of doing it from GlobalFontConfig
construction.
- Move Wayland EGL_PLATFORM=wayland setup from GLES binding load time to
OzonePlatformWayland::OnPreSandboxStartup().
This ensures required environment/platform setup happens before thread
creation, and avoids race conditions issues of setenv/getenv.
- M chrome/app/chrome_main_delegate.cc
- M content/app/content_main_runner_impl.cc
- M ui/base/clipboard/clipboard_non_backed_unittest.cc
- M ui/gfx/linux/fontconfig_util.cc
- M ui/ozone/platform/wayland/gpu/wayland_surface_factory.cc
- M ui/ozone/platform/wayland/ozone_platform_wayland.cc
- M ui/ozone/public/ozone_platform.cc
- M ui/ozone/public/ozone_platform.h
Code-Review: +1 by Steinar H Gunderson, +1 by Dominik Röttsches
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |