Commit 84ba5199 authored by Nick Diego Yamane's avatar Nick Diego Yamane Committed by Commit Bot

x11, ozone/linux: Initialize GTK after Ozone

GtkUi is currently already used in Ozone X11. Ozone platform
initialization code calls XInitThreads() function to init Xlib in
threaded mode under certain conditions**, which according to the docs
should be called before any other Xlib call [1].  However, GTK is
initialized before Ozone platform, which makes Gtk to create/init X11
connection object before calling XInitThreads(), that's wrong and causes
crashes when Gtk file dialogs are instantiated/shown (among other
potential issues not observed so far).

Present CL simplifies GtkUi initialization, moving it entirely to
ToolkitInitialized() override, which happens just after aura::Env's
initialization (which actually init OzonePlatform global object) and
before initializing Views framework, so that code previously in
PreCreateThreads() could be moved and unified under in a single method.

As follow-up, crrev.com/c/1863940 fixes the remaining issues that were
preventing file dialogs to work in Ozone X11.

** Actually current Ozone X11 calls it always, but that must be changed
soon to do so only in some cases. E.g: in-process-gpu, single-process,
etc. Issue tracked under https://crbug.com/1024477.

[1] https://www.x.org/releases/X11R7.5/doc/man/man3/XInitThreads.3.html

Bug: 1008755
Change-Id: Idc81e94dc236257bd7693bfe13833239bbb0fa87
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1899796
Commit-Queue: Nick Yamane <nickdiego@igalia.com>
Reviewed-by: default avatarThomas Anderson <thomasanderson@chromium.org>
Reviewed-by: default avatarMichael Spang <spang@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716908}
parent 4fb5e9ec
...@@ -15,7 +15,9 @@ ChromeBrowserMainExtraPartsViewsLinux::ChromeBrowserMainExtraPartsViewsLinux() = ...@@ -15,7 +15,9 @@ ChromeBrowserMainExtraPartsViewsLinux::ChromeBrowserMainExtraPartsViewsLinux() =
ChromeBrowserMainExtraPartsViewsLinux:: ChromeBrowserMainExtraPartsViewsLinux::
~ChromeBrowserMainExtraPartsViewsLinux() = default; ~ChromeBrowserMainExtraPartsViewsLinux() = default;
void ChromeBrowserMainExtraPartsViewsLinux::PreEarlyInitialization() { void ChromeBrowserMainExtraPartsViewsLinux::ToolkitInitialized() {
ChromeBrowserMainExtraPartsViews::ToolkitInitialized();
views::LinuxUI* linux_ui = views::BuildLinuxUI(); views::LinuxUI* linux_ui = views::BuildLinuxUI();
if (!linux_ui) if (!linux_ui)
return; return;
...@@ -27,21 +29,11 @@ void ChromeBrowserMainExtraPartsViewsLinux::PreEarlyInitialization() { ...@@ -27,21 +29,11 @@ void ChromeBrowserMainExtraPartsViewsLinux::PreEarlyInitialization() {
return ThemeServiceAuraLinux::ShouldUseSystemThemeForProfile( return ThemeServiceAuraLinux::ShouldUseSystemThemeForProfile(
GetThemeProfileForWindow(window)); GetThemeProfileForWindow(window));
})); }));
views::LinuxUI::SetInstance(linux_ui);
}
void ChromeBrowserMainExtraPartsViewsLinux::ToolkitInitialized() {
ChromeBrowserMainExtraPartsViews::ToolkitInitialized();
auto* instance = views::LinuxUI::instance();
if (instance)
instance->Initialize();
}
void ChromeBrowserMainExtraPartsViewsLinux::PreCreateThreads() {
// Update the device scale factor before initializing views // Update the device scale factor before initializing views
// because its display::Screen instance depends on it. // because its display::Screen instance depends on it.
auto* instance = views::LinuxUI::instance(); linux_ui->UpdateDeviceScaleFactor();
if (instance)
instance->UpdateDeviceScaleFactor(); views::LinuxUI::SetInstance(linux_ui);
ChromeBrowserMainExtraPartsViews::PreCreateThreads(); linux_ui->Initialize();
} }
...@@ -21,9 +21,7 @@ class ChromeBrowserMainExtraPartsViewsLinux ...@@ -21,9 +21,7 @@ class ChromeBrowserMainExtraPartsViewsLinux
~ChromeBrowserMainExtraPartsViewsLinux() override; ~ChromeBrowserMainExtraPartsViewsLinux() override;
// Overridden from ChromeBrowserMainExtraParts: // Overridden from ChromeBrowserMainExtraParts:
void PreEarlyInitialization() override;
void ToolkitInitialized() override; void ToolkitInitialized() override;
void PreCreateThreads() override;
private: private:
DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainExtraPartsViewsLinux); DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainExtraPartsViewsLinux);
......
...@@ -13,7 +13,8 @@ ...@@ -13,7 +13,8 @@
#include "ui/display/fake/fake_display_delegate.h" #include "ui/display/fake/fake_display_delegate.h"
#include "ui/events/devices/x11/touch_factory_x11.h" #include "ui/events/devices/x11/touch_factory_x11.h"
#include "ui/events/platform/x11/x11_event_source_default.h" #include "ui/events/platform/x11/x11_event_source_default.h"
#include "ui/gfx/x/x11.h" #include "ui/gfx/x/x11_connection.h"
#include "ui/gfx/x/x11_types.h"
#include "ui/ozone/common/stub_overlay_manager.h" #include "ui/ozone/common/stub_overlay_manager.h"
#include "ui/ozone/platform/x11/x11_clipboard_ozone.h" #include "ui/ozone/platform/x11/x11_clipboard_ozone.h"
#include "ui/ozone/platform/x11/x11_cursor_factory_ozone.h" #include "ui/ozone/platform/x11/x11_cursor_factory_ozone.h"
...@@ -148,7 +149,8 @@ class OzonePlatformX11 : public OzonePlatform { ...@@ -148,7 +149,8 @@ class OzonePlatformX11 : public OzonePlatform {
// Always initialize in multi-thread mode, since this is used only during // Always initialize in multi-thread mode, since this is used only during
// development. // development.
XInitThreads(); // TODO(crbug.com/1024477): Initialize Xlib threads only when required
gfx::InitializeThreadedX11();
// If XOpenDisplay() failed there is nothing we can do. Crash here instead // If XOpenDisplay() failed there is nothing we can do. Crash here instead
// of crashing later. If you are crashing here, make sure there is an X // of crashing later. If you are crashing here, make sure there is an X
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment