bug 1384701 get system fonts in EnsureInit() which is on main thread even with servo r=manishearth
authorKarl Tomlinson <karlt+@karlt.net>
Mon, 04 Sep 2017 18:29:04 +1200
changeset 428636 e711937ff6fa74fd4da55382d7df24647aaa4462
parent 428635 143ef903d8f6310a4405b456a63cdd7cca858a54
child 428637 37c74cd56457e7c7067ebeea2b92e44380eb4868
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmanishearth
bugs1384701
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1384701 get system fonts in EnsureInit() which is on main thread even with servo r=manishearth GTK should be used only from the main thread, but the specific symptom before this change was that creating a GtkEntry causes pango_cairo_font_map_get_default() to be called. This function returns a different font map on each thread. The font map is leaked when StyleThreads are leaked at exit. Font caches are usually expensive and so using the existing font map on the main thread is preferable. A GtkEntry already exists on the main thread, as well as style contexts for most other system fonts, and so it is more efficient to create these on the main thread while the style contexts exist. Doing this also avoids the need for Gecko_nsFont_InitSystem() to hold a global lock to avoid concurrently calling into GTK through LookAndFeel::GetColor(). MozReview-Commit-ID: DSOwEUeYmtV
caps/tests/mochitest/chrome.ini
widget/gtk/nsLookAndFeel.cpp
--- a/caps/tests/mochitest/chrome.ini
+++ b/caps/tests/mochitest/chrome.ini
@@ -2,11 +2,10 @@
 skip-if = os == 'android'
 support-files =
   file_data.txt
   file_disableScript.html
   !/caps/tests/mochitest/file_data.txt
   !/caps/tests/mochitest/file_disableScript.html
 
 [test_bug995943.xul]
-skip-if = stylo && debug && os == 'linux' # bug 1384701
 [test_addonMayLoad.html]
 [test_disableScript.xul]
--- a/widget/gtk/nsLookAndFeel.cpp
+++ b/widget/gtk/nsLookAndFeel.cpp
@@ -702,27 +702,26 @@ nsLookAndFeel::GetFloatImpl(FloatID aID,
     default:
         aResult = -1.0;
         res = NS_ERROR_FAILURE;
     }
     return res;
 }
 
 static void
-GetSystemFontInfo(GtkWidget *aWidget,
+GetSystemFontInfo(GtkStyleContext *aStyle,
                   nsString *aFontName,
                   gfxFontStyle *aFontStyle)
 {
     aFontStyle->style       = NS_FONT_STYLE_NORMAL;
 
     // As in
     // https://git.gnome.org/browse/gtk+/tree/gtk/gtkwidget.c?h=3.22.19#n10333
-    GtkStyleContext *style = gtk_widget_get_style_context(aWidget);
     PangoFontDescription *desc;
-    gtk_style_context_get(style, gtk_style_context_get_state(style),
+    gtk_style_context_get(aStyle, gtk_style_context_get_state(aStyle),
                           "font", &desc, nullptr);
 
     aFontStyle->systemFont = true;
 
     NS_NAMED_LITERAL_STRING(quote, "\"");
     NS_ConvertUTF8toUTF16 family(pango_font_description_get_family(desc));
     *aFontName = quote + family + quote;
 
@@ -747,135 +746,57 @@ GetSystemFontInfo(GtkWidget *aWidget,
 
     // |size| is now pixels
 
     aFontStyle->size = size;
 
     pango_font_description_free(desc);
 }
 
-static void
-GetSystemFontInfo(LookAndFeel::FontID aID,
-                  nsString *aFontName,
-                  gfxFontStyle *aFontStyle)
-{
-    if (aID == LookAndFeel::eFont_Widget) {
-        GtkWidget *label = gtk_label_new("M");
-        GtkWidget *parent = gtk_fixed_new();
-        GtkWidget *window = gtk_window_new(GTK_WINDOW_POPUP);
-
-        gtk_container_add(GTK_CONTAINER(parent), label);
-        gtk_container_add(GTK_CONTAINER(window), parent);
-
-        gtk_widget_ensure_style(label);
-        GetSystemFontInfo(label, aFontName, aFontStyle);
-        gtk_widget_destroy(window);  // no unref, windows are different
-
-    } else if (aID == LookAndFeel::eFont_Button) {
-        GtkWidget *label = gtk_label_new("M");
-        GtkWidget *parent = gtk_fixed_new();
-        GtkWidget *button = gtk_button_new();
-        GtkWidget *window = gtk_window_new(GTK_WINDOW_POPUP);
-
-        gtk_container_add(GTK_CONTAINER(button), label);
-        gtk_container_add(GTK_CONTAINER(parent), button);
-        gtk_container_add(GTK_CONTAINER(window), parent);
-
-        gtk_widget_ensure_style(label);
-        GetSystemFontInfo(label, aFontName, aFontStyle);
-        gtk_widget_destroy(window);  // no unref, windows are different
-
-    } else if (aID == LookAndFeel::eFont_Field) {
-        GtkWidget *entry = gtk_entry_new();
-        GtkWidget *parent = gtk_fixed_new();
-        GtkWidget *window = gtk_window_new(GTK_WINDOW_POPUP);
-
-        gtk_container_add(GTK_CONTAINER(parent), entry);
-        gtk_container_add(GTK_CONTAINER(window), parent);
-
-        gtk_widget_ensure_style(entry);
-        GetSystemFontInfo(entry, aFontName, aFontStyle);
-        gtk_widget_destroy(window);  // no unref, windows are different
-
-    } else {
-        MOZ_ASSERT(aID == LookAndFeel::eFont_Menu, "unexpected font ID");
-        GtkWidget *accel_label = gtk_accel_label_new("M");
-        GtkWidget *menuitem = gtk_menu_item_new();
-        GtkWidget *menu = gtk_menu_new();
-        g_object_ref_sink(menu);
-
-        gtk_container_add(GTK_CONTAINER(menuitem), accel_label);
-        gtk_menu_shell_append((GtkMenuShell *)GTK_MENU(menu), menuitem);
-
-        gtk_widget_ensure_style(accel_label);
-        GetSystemFontInfo(accel_label, aFontName, aFontStyle);
-        g_object_unref(menu);
-    }
-}
-
 bool
 nsLookAndFeel::GetFontImpl(FontID aID, nsString& aFontName,
                            gfxFontStyle& aFontStyle,
                            float aDevPixPerCSSPixel)
 {
-  nsString *cachedFontName = nullptr;
-  gfxFontStyle *cachedFontStyle = nullptr;
-  bool *isCached = nullptr;
-
   switch (aID) {
     case eFont_Menu:         // css2
     case eFont_PullDownMenu: // css3
-      cachedFontName = &mMenuFontName;
-      cachedFontStyle = &mMenuFontStyle;
-      isCached = &mMenuFontCached;
-      aID = eFont_Menu;
-      break;
+      aFontName = mMenuFontName;
+      aFontStyle = mMenuFontStyle;
+      return true;
 
     case eFont_Field:        // css3
     case eFont_List:         // css3
-      cachedFontName = &mFieldFontName;
-      cachedFontStyle = &mFieldFontStyle;
-      isCached = &mFieldFontCached;
-      aID = eFont_Field;
-      break;
+      aFontName = mFieldFontName;
+      aFontStyle = mFieldFontStyle;
+      return true;
 
     case eFont_Button:       // css3
-      cachedFontName = &mButtonFontName;
-      cachedFontStyle = &mButtonFontStyle;
-      isCached = &mButtonFontCached;
-      break;
+      aFontName = mButtonFontName;
+      aFontStyle = mButtonFontStyle;
+      return true;
 
     case eFont_Caption:      // css2
     case eFont_Icon:         // css2
     case eFont_MessageBox:   // css2
     case eFont_SmallCaption: // css2
     case eFont_StatusBar:    // css2
     case eFont_Window:       // css3
     case eFont_Document:     // css3
     case eFont_Workspace:    // css3
     case eFont_Desktop:      // css3
     case eFont_Info:         // css3
     case eFont_Dialog:       // css3
     case eFont_Tooltips:     // moz
     case eFont_Widget:       // moz
-      cachedFontName = &mDefaultFontName;
-      cachedFontStyle = &mDefaultFontStyle;
-      isCached = &mDefaultFontCached;
-      aID = eFont_Widget;
-      break;
+    default:
+      aFontName = mDefaultFontName;
+      aFontStyle = mDefaultFontStyle;
+      return true;
   }
-
-  if (!*isCached) {
-    GetSystemFontInfo(aID, cachedFontName, cachedFontStyle);
-    *isCached = true;
-  }
-
-  aFontName = *cachedFontName;
-  aFontStyle = *cachedFontStyle;
-  return true;
 }
 
 void
 nsLookAndFeel::EnsureInit()
 {
     GdkColor colorValue;
     GdkColor *colorValuePtr;
 
@@ -925,45 +846,61 @@ nsLookAndFeel::EnsureInit()
         nsAutoCString contentThemeName;
         mozilla::Preferences::GetCString("widget.content.gtk-theme-override",
                                          contentThemeName);
         if (!contentThemeName.IsEmpty()) {
             g_object_set(settings, "gtk-theme-name", contentThemeName.get(), nullptr);
         }
     }
 
+    // The label is not added to a parent widget, but shared for constructing
+    // different style contexts.  The node hierarchy is constructed only on
+    // the label style context.
+    GtkWidget *labelWidget = gtk_label_new("M");
+    g_object_ref_sink(labelWidget);
+
     // Scrollbar colors
     style = ClaimStyleContext(MOZ_GTK_SCROLLBAR_TROUGH_VERTICAL);
     gtk_style_context_get_background_color(style, GTK_STATE_FLAG_NORMAL, &color);
     sMozScrollbar = GDK_RGBA_TO_NS_RGBA(color);
     ReleaseStyleContext(style);
 
     // Window colors
     style = ClaimStyleContext(MOZ_GTK_WINDOW);
     gtk_style_context_get_background_color(style, GTK_STATE_FLAG_NORMAL, &color);
     sMozWindowBackground = GDK_RGBA_TO_NS_RGBA(color);
     gtk_style_context_get_color(style, GTK_STATE_FLAG_NORMAL, &color);
     sMozWindowText = GDK_RGBA_TO_NS_RGBA(color);
     ReleaseStyleContext(style);
+    style = ClaimStyleContext(MOZ_GTK_WINDOW_CONTAINER);
+    {
+        GtkStyleContext* labelStyle = CreateStyleForWidget(labelWidget, style);
+        GetSystemFontInfo(labelStyle, &mDefaultFontName, &mDefaultFontStyle);
+        g_object_unref(labelStyle);
+    }
+    ReleaseStyleContext(style);
 
     // tooltip foreground and background
     style = ClaimStyleContext(MOZ_GTK_TOOLTIP);
     gtk_style_context_get_background_color(style, GTK_STATE_FLAG_NORMAL, &color);
     sInfoBackground = GDK_RGBA_TO_NS_RGBA(color);
     ReleaseStyleContext(style);
 
     style = ClaimStyleContext(MOZ_GTK_TOOLTIP_BOX_LABEL);
     gtk_style_context_get_color(style, GTK_STATE_FLAG_NORMAL, &color);
     sInfoText = GDK_RGBA_TO_NS_RGBA(color);
     ReleaseStyleContext(style);
 
     style = ClaimStyleContext(MOZ_GTK_MENUITEM);
     {
         GtkStyleContext* accelStyle =
             CreateStyleForWidget(gtk_accel_label_new("M"), style);
+
+        GetSystemFontInfo(accelStyle, &mMenuFontName, &mMenuFontStyle);
+
         gtk_style_context_get_color(accelStyle, GTK_STATE_FLAG_NORMAL, &color);
         sMenuText = GDK_RGBA_TO_NS_RGBA(color);
         gtk_style_context_get_color(accelStyle, GTK_STATE_FLAG_INSENSITIVE, &color);
         sMenuTextInactive = GDK_RGBA_TO_NS_RGBA(color);
         g_object_unref(accelStyle);
     }
     ReleaseStyleContext(style);
 
@@ -974,30 +911,25 @@ nsLookAndFeel::EnsureInit()
 
     style = ClaimStyleContext(MOZ_GTK_MENUITEM);
     gtk_style_context_get_background_color(style, GTK_STATE_FLAG_PRELIGHT, &color);
     sMenuHover = GDK_RGBA_TO_NS_RGBA(color);
     gtk_style_context_get_color(style, GTK_STATE_FLAG_PRELIGHT, &color);
     sMenuHoverText = GDK_RGBA_TO_NS_RGBA(color);
     ReleaseStyleContext(style);
 
-    // button styles
     GtkWidget *parent = gtk_fixed_new();
-    GtkWidget *button = gtk_button_new();
-    GtkWidget *label = gtk_label_new("M");
     GtkWidget *window = gtk_window_new(GTK_WINDOW_POPUP);
     GtkWidget *treeView = gtk_tree_view_new();
     GtkWidget *linkButton = gtk_link_button_new("http://example.com/");
     GtkWidget *menuBar = gtk_menu_bar_new();
     GtkWidget *menuBarItem = gtk_menu_item_new();
     GtkWidget *entry = gtk_entry_new();
     GtkWidget *textView = gtk_text_view_new();
 
-    gtk_container_add(GTK_CONTAINER(button), label);
-    gtk_container_add(GTK_CONTAINER(parent), button);
     gtk_container_add(GTK_CONTAINER(parent), treeView);
     gtk_container_add(GTK_CONTAINER(parent), linkButton);
     gtk_container_add(GTK_CONTAINER(parent), menuBar);
     gtk_menu_shell_append(GTK_MENU_SHELL(menuBar), menuBarItem);
     gtk_container_add(GTK_CONTAINER(window), parent);
     gtk_container_add(GTK_CONTAINER(parent), entry);
     gtk_container_add(GTK_CONTAINER(parent), textView);
     
@@ -1027,18 +959,20 @@ nsLookAndFeel::EnsureInit()
         static_cast<GtkStateFlags>(GTK_STATE_FLAG_FOCUSED|GTK_STATE_FLAG_SELECTED),
         &color);
     sTextSelectedText = GDK_RGBA_TO_NS_RGBA(color);
     ReleaseStyleContext(style);
 
     // Button text color
     style = ClaimStyleContext(MOZ_GTK_BUTTON);
     {
-        GtkStyleContext* labelStyle =
-            CreateStyleForWidget(gtk_label_new("M"), style);
+        GtkStyleContext* labelStyle = CreateStyleForWidget(labelWidget, style);
+
+        GetSystemFontInfo(labelStyle, &mButtonFontName, &mButtonFontStyle);
+
         gtk_style_context_get_color(labelStyle, GTK_STATE_FLAG_NORMAL, &color);
         sButtonText = GDK_RGBA_TO_NS_RGBA(color);
         gtk_style_context_get_color(labelStyle, GTK_STATE_FLAG_PRELIGHT, &color);
         sButtonHoverText = GDK_RGBA_TO_NS_RGBA(color);
         g_object_unref(labelStyle);
     }
     ReleaseStyleContext(style);
 
@@ -1135,17 +1069,21 @@ nsLookAndFeel::EnsureInit()
     g_object_get (entry, "invisible-char", &value, nullptr);
     sInvisibleCharacter = char16_t(value);
 
     // caret styles
     gtk_widget_style_get(entry,
                          "cursor-aspect-ratio", &sCaretRatio,
                          nullptr);
 
+    GetSystemFontInfo(gtk_widget_get_style_context(entry),
+                      &mFieldFontName, &mFieldFontStyle);
+
     gtk_widget_destroy(window);
+    g_object_unref(labelWidget);
 }
 
 // virtual
 char16_t
 nsLookAndFeel::GetPasswordCharacterImpl()
 {
     EnsureInit();
     return sInvisibleCharacter;