"Bootstrap" a toolkit-views Typography spec.

ui/views/style/typography_unittest.cc

Index: ui/views/style/typography_unittest.cc
diff --git a/ui/views/style/typography_unittest.cc b/ui/views/style/typography_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c02fe7091fd1a97324cf88ae0005465160681616
--- /dev/null
+++ b/ui/views/style/typography_unittest.cc
@@ -0,0 +1,270 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/views/style/typography.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/default_style.h"
+#include "ui/base/resource/resource_bundle.h"
+
+#if defined(OS_MACOSX)
+#include "base/mac/mac_util.h"
+#endif
+
+namespace views {
+
+// Check legacy font sizes. No new code should be using these constants, but if
+// these tests ever fail it probably means something in the old UI will have
+// changed by mistake.
+// Disabled since this relies on machine configuration. http://crbug.com/701241.
+TEST(TypographyTest, DISABLED_LegacyFontSizeConstants) {
+  ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  gfx::FontList label_font = rb.GetFontListWithDelta(ui::kLabelFontSizeDelta);
+
+#if defined(OS_WIN)
+  // On Windows, some bots say 11 and some bots say 12. This is a bad thing.

[Peter Kasting, 2017/03/14 23:53:34]

OS versions differ?  Might check OSInfo::version().

[tapted, 2017/03/15 08:26:25]

I'll need to smoke out the bot - might have been a one-off (or possibly me
getting lost in error output)

+  if (label_font.GetFontSize() == 11) {
+    EXPECT_EQ(11, label_font.GetFontSize());
+    EXPECT_EQ(13, label_font.GetHeight());
+    EXPECT_EQ(11, label_font.GetBaseline());
+  } else if (label_font.GetFontSize() == 12) {
+    EXPECT_EQ(12, label_font.GetFontSize());
+    EXPECT_EQ(15, label_font.GetHeight());
+    EXPECT_EQ(12, label_font.GetBaseline());
+  } else {
+    ADD_FAILURE() << "Label font size is not 11 or 12 on Windows.";
+  }
+  EXPECT_EQ(9, label_font.GetCapHeight());
+#else
+  EXPECT_EQ(12, label_font.GetFontSize());
+  EXPECT_EQ(15, label_font.GetHeight());
+  EXPECT_EQ(12, label_font.GetBaseline());
+  EXPECT_EQ(9, label_font.GetCapHeight());
+#endif
+
+#if defined(OS_MACOSX)
+  if (base::mac::IsOS10_9()) {
+    EXPECT_EQ(6, label_font.GetExpectedTextWidth(1));
+  } else {
+    EXPECT_EQ(10, label_font.GetExpectedTextWidth(1));
+  }
+#elif defined(OS_WIN)
+  int label_font_width = label_font.GetExpectedTextWidth(1);
+  EXPECT_TRUE(label_font_width == 5 || label_font_width == 6);
+  EXPECT_EQ(6, label_font.GetExpectedTextWidth(1));
+#else
+  EXPECT_EQ(6, label_font.GetExpectedTextWidth(1));
+#endif
+
+  int windows_fudge_delta = 0;
+#if defined(OS_WIN)
+  windows_fudge_delta = label_font.GetFontSize() == 11 ? 1 : 0;
+#endif
+
+  gfx::FontList title_font =
+      rb.GetFontListWithDelta(ui::kTitleFontSizeDelta + windows_fudge_delta);
+
+#if defined(OS_WIN)
+  EXPECT_EQ(15, title_font.GetFontSize());
+  EXPECT_EQ(20, title_font.GetHeight());
+  EXPECT_EQ(16, title_font.GetBaseline());
+  EXPECT_EQ(11, title_font.GetCapHeight());
+#elif defined(OS_MACOSX)
+  EXPECT_EQ(14, title_font.GetFontSize());
+  EXPECT_EQ(17, title_font.GetHeight());
+  EXPECT_EQ(14, title_font.GetBaseline());
+  if (base::mac::IsOS10_9()) {
+    EXPECT_EQ(11, title_font.GetCapHeight());
+  } else {
+    EXPECT_EQ(10, title_font.GetCapHeight());
+  }
+#else
+  EXPECT_EQ(15, title_font.GetFontSize());
+  EXPECT_EQ(18, title_font.GetHeight());
+  EXPECT_EQ(14, title_font.GetBaseline());
+  EXPECT_EQ(11, title_font.GetCapHeight());
+#endif
+
+#if defined(OS_MACOSX)
+  if (base::mac::IsOS10_9()) {
+    EXPECT_EQ(7, title_font.GetExpectedTextWidth(1));
+  } else {
+    EXPECT_EQ(12, title_font.GetExpectedTextWidth(1));
+  }
+#elif defined(OS_WIN)
+  int title_font_width = title_font.GetExpectedTextWidth(1);
+  EXPECT_TRUE(title_font_width == 6 || title_font_width == 8);  // Weird.

[Peter Kasting, 2017/03/14 23:53:34]

What.

[tapted, 2017/03/15 08:26:25]

Yeah - there was one bot run that was different to all the rest :/. I want to
smoke it out. Simplified these for now (plan is to land these change-detectors
DISABLED_ and smoke them out -- http://crbug.com/701241

Also moved these to layout_delegate_unittest.cc - there's a lot of stuff from
the spec, and we want to test the "extended" enums for the stuff below.

+  EXPECT_EQ(8, title_font.GetExpectedTextWidth(1));

[Peter Kasting, 2017/03/14 23:53:34]

This line kinda contradicts the one just above

[tapted, 2017/03/15 08:26:25]

Acknowledged.

+#else
+  EXPECT_EQ(8, title_font.GetExpectedTextWidth(1));
+#endif
+
+  gfx::FontList small_font = rb.GetFontList(ResourceBundle::SmallFont);
+  gfx::FontList base_font = rb.GetFontList(ResourceBundle::BaseFont);
+  gfx::FontList bold_font = rb.GetFontList(ResourceBundle::BoldFont);
+  gfx::FontList medium_font = rb.GetFontList(ResourceBundle::MediumFont);
+  gfx::FontList medium_bold_font =
+      rb.GetFontList(ResourceBundle::MediumBoldFont);
+  gfx::FontList large_font = rb.GetFontList(ResourceBundle::LargeFont);
+
+#if defined(OS_MACOSX)
+  EXPECT_EQ(12, small_font.GetFontSize());
+  EXPECT_EQ(13, base_font.GetFontSize());
+  EXPECT_EQ(13, bold_font.GetFontSize());
+  EXPECT_EQ(16, medium_font.GetFontSize());
+  EXPECT_EQ(16, medium_bold_font.GetFontSize());
+  EXPECT_EQ(21, large_font.GetFontSize());
+#else
+  EXPECT_EQ(11 - windows_fudge_delta, small_font.GetFontSize());
+  EXPECT_EQ(12 - windows_fudge_delta, base_font.GetFontSize());
+  EXPECT_EQ(12 - windows_fudge_delta, bold_font.GetFontSize());
+  EXPECT_EQ(15 - windows_fudge_delta, medium_font.GetFontSize());
+  EXPECT_EQ(15 - windows_fudge_delta, medium_bold_font.GetFontSize());
+  EXPECT_EQ(20 - windows_fudge_delta, large_font.GetFontSize());
+#endif
+}
+
+// Check that asking for fonts of a given size match the Harmony spec.
+// Disabled since this relies on machine configuration. http://crbug.com/701241.
+TEST(TypographyTest, DISABLED_RequestFontBySize) {
+#if defined(OS_MACOSX)
+  constexpr int kBase = 13;
+#else
+  constexpr int kBase = 12;
+#endif
+  // Harmony spec.
+  constexpr int kHeadline = 20;
+  constexpr int kTitle = 15;  // Leading 22.
+  constexpr int kBody1 = 13;  // Leading 20.
+  constexpr int kBody2 = 12;  // Leading 20.
+  constexpr int kButton = 12;
+
+  // Cater for bots that report 11 as the base font size rather than 12.
+  int windows_fudge_delta = 0;
+
+#if defined(OS_WIN)
+  constexpr gfx::Font::Weight kButtonWeight = gfx::Font::Weight::BOLD;
+  windows_fudge_delta = gfx::FontList().GetFontSize() == 11 ? 1 : 0;
+#else
+  constexpr gfx::Font::Weight kButtonWeight = gfx::Font::Weight::MEDIUM;
+#endif
+
+  ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+
+  gfx::FontList headline_font =
+      rb.GetFontListWithDelta(kHeadline - kBase + windows_fudge_delta);
+  gfx::FontList title_font =
+      rb.GetFontListWithDelta(kTitle - kBase + windows_fudge_delta);
+  gfx::FontList body1_font =
+      rb.GetFontListWithDelta(kBody1 - kBase + windows_fudge_delta);
+  gfx::FontList body2_font =
+      rb.GetFontListWithDelta(kBody2 - kBase + windows_fudge_delta);
+  gfx::FontList button_font = rb.GetFontListWithDelta(
+      kButton - kBase + windows_fudge_delta, gfx::Font::NORMAL, kButtonWeight);
+
+  // The following checks on leading don't need to match the spec. Instead, it
+  // means Label::SetLineHeight() needs to be used to increase it. But what we
+  // are really interested in is the delta between GetFontSize() and GetHeight()
+  // since that (plus a fixed constant) determines how the leading should change
+  // when a larger font is configured in the OS.
+
+  EXPECT_EQ(kHeadline, headline_font.GetFontSize());
+
+// Headline leading not specified (multiline should be rare).
+#if defined(OS_MACOSX)
+  EXPECT_EQ(25, headline_font.GetHeight());
+#elif defined(OS_WIN)
+  EXPECT_EQ(28, headline_font.GetHeight());
+#else
+  EXPECT_EQ(24, headline_font.GetHeight());
+#endif
+
+  EXPECT_EQ(kTitle, title_font.GetFontSize());
+
+// Title font leading should be 22.
+#if defined(OS_MACOSX)
+  EXPECT_EQ(19, title_font.GetHeight());  // i.e. Add 3 to obtain line height.
+#elif defined(OS_WIN)
+  EXPECT_EQ(20, title_font.GetHeight());  // Add 2.
+#else
+  EXPECT_EQ(18, title_font.GetHeight());  // Add 4.
+#endif
+
+  EXPECT_EQ(kBody1, body1_font.GetFontSize());
+
+// Body1 font leading should be 20.
+#if defined(OS_MACOSX)
+  EXPECT_EQ(16, body1_font.GetHeight());  // Add 4.
+#else  // Win and Linux.
+  EXPECT_EQ(17, body1_font.GetHeight());  // Add 3.
+#endif
+
+  EXPECT_EQ(kBody2, body2_font.GetFontSize());
+
+  // Body2 font leading should be 20.
+  EXPECT_EQ(15, body2_font.GetHeight());  // All platforms: Add 5.
+
+  EXPECT_EQ(kButton, button_font.GetFontSize());
+
+  // Button leading not specified (shouldn't be needed: no multiline buttons).
+  EXPECT_EQ(15, button_font.GetHeight());
+}

[tapted, 2017/03/14 09:56:18]

The tests above were a means to explore how "leading"/LineSpacing should be
calculated in the TypographyProvider. The default provider can just use 0 to
indicate that it should use Font::GetHeight().

But the harmony provider needs to account for how a leading of "20" should
change when the size of the base font changes. There's a discussion about this
at https://bugs.chromium.org/p/chromium/issues/detail?id=691891#c5  (i.e. how
HarmonyProvider will implement GetLineSpacing()).

Anyway, this test probably doesn't belong here - it should go with
HarmonyProvider. Not sure about LegacyFontSizeConstants - maybe we don't need it
- or maybe it's useful having "reality" documented somewhere - currently the
mapping between things like ResourceBundle::BaseFont to a font size and other
metrics is very unclear.

+
+// Test that the default TypographyProivder correctly maps TextContexts relative
+// to the "base" font in the manner that legacy toolkit-views code expects. This
+// reads the base font configuration at runtime, and only tests font sizes, so
+// should be robust against platform changes.
+TEST(TypographyTest, FontSizeRelativeToBase) {
+  constexpr TextStyle kStyle = TextStyle::PRIMARY;
+
+// Legacy code measures everything relative to a default-constructed FontList.
+// On Mac, subtract one since that is 13pt instead of 12pt.
+#if defined(OS_MACOSX)
+  const int kTwelve = gfx::FontList().GetFontSize() - 1;
+#else
+  const int kTwelve = gfx::FontList().GetFontSize();
+#endif
+
+  EXPECT_EQ(kTwelve, Typography::GetFont(TextContext::DIALOG_TEXT_SMALL, kStyle)
+                         .GetFontSize());
+  EXPECT_EQ(
+      kTwelve,
+      Typography::GetFont(TextContext::CONTROL_LABEL, kStyle).GetFontSize());
+  EXPECT_EQ(kTwelve,
+            Typography::GetFont(TextContext::FIELD, kStyle).GetFontSize());
+  EXPECT_EQ(
+      kTwelve,
+      Typography::GetFont(TextContext::BUTTON_TEXT, kStyle).GetFontSize());
+
+#if defined(OS_MACOSX)
+  // We never exposed UI on Mac using these constants so it doesn't matter that
+  // they are different. They only need to match under Harmony.
+  EXPECT_EQ(kTwelve + 9,
+            Typography::GetFont(TextContext::HEADLINE, kStyle).GetFontSize());
+  EXPECT_EQ(
+      kTwelve + 2,
+      Typography::GetFont(TextContext::DIALOG_TITLE, kStyle).GetFontSize());
+  EXPECT_EQ(
+      kTwelve + 2,
+      Typography::GetFont(TextContext::DIALOG_MESSAGE, kStyle).GetFontSize());
+  EXPECT_EQ(
+      kTwelve,
+      Typography::GetFont(TextContext::DEPRECATED_SMALL, kStyle).GetFontSize());
+#else
+  // E.g. Headline should give a 20pt font.
+  EXPECT_EQ(kTwelve + 8,
+            Typography::GetFont(TextContext::HEADLINE, kStyle).GetFontSize());
+  // Titles should be 15pt. Etc.
+  EXPECT_EQ(
+      kTwelve + 3,
+      Typography::GetFont(TextContext::DIALOG_TITLE, kStyle).GetFontSize());
+  EXPECT_EQ(
+      kTwelve + 1,
+      Typography::GetFont(TextContext::DIALOG_MESSAGE, kStyle).GetFontSize());
+  EXPECT_EQ(
+      kTwelve - 1,
+      Typography::GetFont(TextContext::DEPRECATED_SMALL, kStyle).GetFontSize());
+#endif
+}
+
+}  // namespace views