linux: Explicitly call FcInit() in ToolkitInitialized(). (issue 497523002 by derat@chromium.org)

7 views
Skip to first unread message

de...@chromium.org

unread,
Aug 21, 2014, 1:31:17 PM8/21/14
to s...@chromium.org, chromium...@chromium.org
Reviewers: sky,

Description:
linux: Explicitly call FcInit() in ToolkitInitialized().

This hopefully prevents implicit FontConfig initialization
from occurring later when queries are first made from
gfx::FontRenderParams/Skia/Blink/etc.

BUG=404311

Please review this at https://codereview.chromium.org/497523002/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files (+14, -2 lines):
M chrome/browser/chrome_browser_main_linux.h
M chrome/browser/chrome_browser_main_linux.cc
M ui/gfx/font_render_params_linux.cc


Index: chrome/browser/chrome_browser_main_linux.cc
diff --git a/chrome/browser/chrome_browser_main_linux.cc
b/chrome/browser/chrome_browser_main_linux.cc
index
b7967f86914bf14b20cc1d70e23606f69cb51925..227045351a73303e5170a212cf786511c7ac8e5e
100644
--- a/chrome/browser/chrome_browser_main_linux.cc
+++ b/chrome/browser/chrome_browser_main_linux.cc
@@ -4,6 +4,8 @@

#include "chrome/browser/chrome_browser_main_linux.h"

+#include <fontconfig/fontconfig.h>
+
#include "chrome/browser/browser_process.h"
#include "components/breakpad/app/breakpad_linux.h"
#include "components/metrics/metrics_service.h"
@@ -22,6 +24,15 @@ ChromeBrowserMainPartsLinux::ChromeBrowserMainPartsLinux(
ChromeBrowserMainPartsLinux::~ChromeBrowserMainPartsLinux() {
}

+void ChromeBrowserMainPartsLinux::ToolkitInitialized() {
+ // Explicitly initialize FontConfig early on to prevent races later due
to
+ // implicit initialization in respons to threads' first calls to
FontConfig:
+ // http://crbug.com/404311
+ FcInit();
+
+ ChromeBrowserMainPartsPosix::ToolkitInitialized();
+}
+
void ChromeBrowserMainPartsLinux::PreProfileInit() {
#if !defined(OS_CHROMEOS)
// Needs to be called after we have chrome::DIR_USER_DATA and
Index: chrome/browser/chrome_browser_main_linux.h
diff --git a/chrome/browser/chrome_browser_main_linux.h
b/chrome/browser/chrome_browser_main_linux.h
index
37f35ce523f5e67f6ef1e29056feb59ce1f72ef1..9d9a92aab8e5f574f4b022a631232016b3f694c1
100644
--- a/chrome/browser/chrome_browser_main_linux.h
+++ b/chrome/browser/chrome_browser_main_linux.h
@@ -17,6 +17,7 @@ class ChromeBrowserMainPartsLinux : public
ChromeBrowserMainPartsPosix {
virtual ~ChromeBrowserMainPartsLinux();

// ChromeBrowserMainParts overrides.
+ virtual void ToolkitInitialized() OVERRIDE;
virtual void PreProfileInit() OVERRIDE;
virtual void PostProfileInit() OVERRIDE;

Index: ui/gfx/font_render_params_linux.cc
diff --git a/ui/gfx/font_render_params_linux.cc
b/ui/gfx/font_render_params_linux.cc
index
f02636e8050c2da9466648995aada7952e1f09f6..55a39ef6c00e25b68c852d67245eebfeee4896c3
100644
--- a/ui/gfx/font_render_params_linux.cc
+++ b/ui/gfx/font_render_params_linux.cc
@@ -4,6 +4,8 @@

#include "ui/gfx/font_render_params.h"

+#include <fontconfig/fontconfig.h>
+
#include "base/command_line.h"
#include "base/containers/mru_cache.h"
#include "base/hash.h"
@@ -17,8 +19,6 @@
#include "ui/gfx/linux_font_delegate.h"
#include "ui/gfx/switches.h"

-#include <fontconfig/fontconfig.h>
-
namespace gfx {

namespace {


s...@chromium.org

unread,
Aug 21, 2014, 1:38:21 PM8/21/14
to de...@chromium.org, chromium...@chromium.org
LGTM


https://codereview.chromium.org/497523002/diff/40001/chrome/browser/chrome_browser_main_linux.cc
File chrome/browser/chrome_browser_main_linux.cc (right):

https://codereview.chromium.org/497523002/diff/40001/chrome/browser/chrome_browser_main_linux.cc#newcode29
chrome/browser/chrome_browser_main_linux.cc:29: // implicit
initialization in respons to threads' first calls to Fontconfig:
respons->response

https://codereview.chromium.org/497523002/

beh...@google.com

unread,
Aug 21, 2014, 1:43:39 PM8/21/14
to de...@chromium.org, s...@chromium.org, chromium...@chromium.org

de...@chromium.org

unread,
Aug 21, 2014, 1:56:14 PM8/21/14
to s...@chromium.org, beh...@google.com, chromium...@chromium.org

https://codereview.chromium.org/497523002/diff/40001/chrome/browser/chrome_browser_main_linux.cc
File chrome/browser/chrome_browser_main_linux.cc (right):

https://codereview.chromium.org/497523002/diff/40001/chrome/browser/chrome_browser_main_linux.cc#newcode29
chrome/browser/chrome_browser_main_linux.cc:29: // implicit
initialization in respons to threads' first calls to Fontconfig:
On 2014/08/21 17:38:19, sky wrote:
> respons->response

Done.

https://codereview.chromium.org/497523002/

de...@chromium.org

unread,
Aug 21, 2014, 6:07:52 PM8/21/14
to s...@chromium.org, beh...@google.com, chromium...@chromium.org
I've confirmed under gdb that FcInit() is getting called first by
ToolkitInitialized() with this change.

https://codereview.chromium.org/497523002/

commi...@chromium.org

unread,
Aug 21, 2014, 6:12:33 PM8/21/14
to de...@chromium.org, s...@chromium.org, beh...@google.com, chromium...@chromium.org

commi...@chromium.org

unread,
Aug 21, 2014, 7:21:10 PM8/21/14
to de...@chromium.org, s...@chromium.org, beh...@google.com, chromium...@chromium.org
FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tests_recipe/builds/1125)

https://codereview.chromium.org/497523002/

commi...@chromium.org

unread,
Aug 21, 2014, 9:28:59 PM8/21/14
to de...@chromium.org, s...@chromium.org, beh...@google.com, chromium...@chromium.org
Committed patchset #4 (60001) as 291298

https://codereview.chromium.org/497523002/
Reply all
Reply to author
Forward
0 new messages