Let both minimum font-size settings apply to CSS px units.

Applying the minimum font-size to the device pixels would not give the
expected result, either for using zoom for DSF, or applying page zoom.

I added the documentation for the settings based on what I found on
stackoverflow:

https://stackoverflow.com/questions/52943595/cannot-understand-android-webview-documentation

The behavior changes for both the non-"logical" setting in that we will
still apply a minimum font-size even if the zoomed font-size would have
become larger than the minimum font-size in device-pixels. That is, if
we have a minimum font-size of 20px and two elements, one with a 10px
font-size and the other with a 20px font-size, they will both be
rendered at 40 device pixel size at 200% zoom.

This changes the behavior for the logical setting when we have a page
zoom factor which is lower than 100%, where we also now just adjust the
CSS px font-size before zooming is applied.

The logical setting is only exposed via the Android WebView API or via
a hand-edited config file. The minimum font-size setting the
chrome://settings is the non-"logical" one.

If the settings object is null, return the font-size without minimum
sizes applied instead of the not-so-useful 1px return we had.

Bug: 1082127
Change-Id: Idc03bf37fb7d39206ad4d4dc50eef01b56a69ddb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2199134Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770661}

third_party/blink/renderer/core/css/BUILD.gn

@@ -631,6 +631,7 @@ blink_core_tests("unit_tests") {
     "cssom/style_property_map_test.cc",
     "drag_update_test.cc",
     "font_face_cache_test.cc",
+    "font_size_functions_test.cc",
     "font_update_invalidation_test.cc",
     "invalidation/invalidation_set_test.cc",
     "invalidation/pending_invalidations_test.cc",

third_party/blink/renderer/core/css/font_size_functions.cc

@@ -49,44 +49,37 @@ float FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
   if (fabsf(specified_size) < std::numeric_limits<float>::epsilon())
     return 0.0f;
 
-  // We support two types of minimum font size. The first is a hard override
-  // that applies to all fonts. This is "minSize." The second type of minimum
-  // font size is a "smart minimum" that is applied only when the Web page can't
-  // know what size it really asked for, e.g., when it uses logical sizes like
-  // "small" or expresses the font-size as a percentage of the user's default
-  // font setting.
-
-  // With the smart minimum, we never want to get smaller than the minimum font
-  // size to keep fonts readable. However we always allow the page to set an
-  // explicit pixel size that is smaller, since sites will mis-render otherwise
-  // (e.g., http://www.gamespot.com with a 9px minimum).
-
   Settings* settings = document->GetSettings();
-  if (!settings)
-    return 1.0f;
+  if (apply_minimum_font_size && settings) {
+    // We support two types of minimum font size. The first is a hard override
+    // that applies to all fonts. This is "min_size." The second type of minimum
+    // font size is a "smart minimum" that is applied only when the Web page
+    // can't know what size it really asked for, e.g., when it uses logical
+    // sizes like "small" or expresses the font-size as a percentage of the
+    // user's default font setting.
+
+    // With the smart minimum, we never want to get smaller than the minimum
+    // font size to keep fonts readable. However we always allow the page to set
+    // an explicit pixel size that is smaller, since sites will mis-render
+    // otherwise (e.g., http://www.gamespot.com with a 9px minimum).
 
-  float zoomed_size = specified_size * zoom_factor;
-  if (apply_minimum_font_size) {
     int min_size = settings->GetMinimumFontSize();
     int min_logical_size = settings->GetMinimumLogicalFontSize();
 
-    // Apply the hard minimum first. We only apply the hard minimum if after
-    // zooming we're still too small.
-    if (zoomed_size < min_size)
-      zoomed_size = min_size;
+    // Apply the hard minimum first.
+    if (specified_size < min_size)
+      specified_size = min_size;
 
-    // Now apply the "smart minimum." This minimum is also only applied if we're
-    // still too small after zooming. The font size must either be relative to
+    // Now apply the "smart minimum". The font size must either be relative to
     // the user default or the original size must have been acceptable. In other
     // words, we only apply the smart minimum whenever we're positive doing so
     // won't disrupt the layout.
-    if (zoomed_size < min_logical_size &&
-        (specified_size >= min_logical_size || !is_absolute_size))
-      zoomed_size = min_logical_size;
+    if (specified_size < min_logical_size && !is_absolute_size)
+      specified_size = min_logical_size;
   }
   // Also clamp to a reasonable maximum to prevent insane font sizes from
   // causing crashes on various platforms (I'm looking at you, Windows.)
-  return std::min(kMaximumAllowedFontSize, zoomed_size);
+  return std::min(kMaximumAllowedFontSize, specified_size * zoom_factor);
 }
 
 const int kFontSizeTableMax = 16;

third_party/blink/renderer/core/css/font_size_functions.h

@@ -23,6 +23,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_FONT_SIZE_FUNCTIONS_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_FONT_SIZE_FUNCTIONS_H_
 
+#include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/css_value_keywords.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
 
@@ -35,7 +36,7 @@ enum ApplyMinimumFontSize {
   kApplyMinimumForFontSize
 };
 
-class FontSizeFunctions {
+class CORE_EXPORT FontSizeFunctions {
   STATIC_ONLY(FontSizeFunctions);
 
  public:

third_party/blink/renderer/core/css/font_size_functions_test.cc

+// Copyright 2020 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 "third_party/blink/renderer/core/css/font_size_functions.h"
+
+#include "third_party/blink/renderer/core/dom/document.h"
+#include "third_party/blink/renderer/core/frame/settings.h"
+#include "third_party/blink/renderer/core/testing/page_test_base.h"
+
+namespace blink {
+
+using FontSizeFunctionsTest = PageTestBase;
+
+TEST_F(FontSizeFunctionsTest, GetComputedSizeFromSpecifiedSize_NoMinFontSize) {
+  constexpr float zoom_factor = 2;
+  constexpr int min_font_size = 100;
+  constexpr bool is_absolute = true;
+  constexpr bool is_logical = false;
+
+  GetDocument().GetSettings()->SetMinimumFontSize(min_font_size);
+  GetDocument().GetSettings()->SetMinimumLogicalFontSize(min_font_size);
+
+  for (const int& font_size : {1, 10, 40, 120}) {
+    EXPECT_EQ(font_size * zoom_factor,
+              FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
+                  &GetDocument(), zoom_factor, is_absolute, font_size,
+                  kDoNotApplyMinimumForFontSize));
+    EXPECT_EQ(font_size * zoom_factor,
+              FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
+                  &GetDocument(), zoom_factor, is_logical, font_size,
+                  kDoNotApplyMinimumForFontSize));
+  }
+}
+
+TEST_F(FontSizeFunctionsTest, GetComputedSizeFromSpecifiedSize_MinFontSize) {
+  constexpr float zoom_factor = 2;
+  constexpr int min_font_size = 100;
+  constexpr bool is_absolute = true;
+  constexpr bool is_logical = false;
+
+  GetDocument().GetSettings()->SetMinimumFontSize(min_font_size);
+  GetDocument().GetSettings()->SetMinimumLogicalFontSize(0);
+
+  int test_cases[][2] = {
+      {1, min_font_size}, {10, min_font_size}, {40, min_font_size}, {120, 120}};
+  for (const auto* font_sizes : test_cases) {
+    EXPECT_EQ(font_sizes[1] * zoom_factor,
+              FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
+                  &GetDocument(), zoom_factor, is_absolute, font_sizes[0]));
+    EXPECT_EQ(font_sizes[1] * zoom_factor,
+              FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
+                  &GetDocument(), zoom_factor, is_logical, font_sizes[0]));
+  }
+}
+
+TEST_F(FontSizeFunctionsTest,
+       GetComputedSizeFromSpecifiedSize_MinLogicalFontSize) {
+  constexpr float zoom_factor = 2;
+  constexpr int min_font_size = 100;
+  constexpr bool is_absolute = true;
+  constexpr bool is_logical = false;
+
+  GetDocument().GetSettings()->SetMinimumFontSize(0);
+  GetDocument().GetSettings()->SetMinimumLogicalFontSize(min_font_size);
+
+  int test_cases[][2] = {
+      {1, min_font_size}, {10, min_font_size}, {40, min_font_size}, {120, 120}};
+  for (const auto* font_sizes : test_cases) {
+    EXPECT_EQ(font_sizes[0] * zoom_factor,
+              FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
+                  &GetDocument(), zoom_factor, is_absolute, font_sizes[0]));
+    EXPECT_EQ(font_sizes[1] * zoom_factor,
+              FontSizeFunctions::GetComputedSizeFromSpecifiedSize(
+                  &GetDocument(), zoom_factor, is_logical, font_sizes[0]));
+  }
+}
+
+}  // namespace blink

third_party/blink/renderer/core/frame/settings.json5

@@ -65,12 +65,17 @@
       type: "int",
     },
 
+    // Sets the minimum font-size in CSS px. That means the minimum font-size is
+    // applied before applying the device scale factor or page zoom.
     {
       name: "minimumFontSize",
       initial: 0,
       invalidate: "Style",
       type: "int",
     },
+
+    // Sets the minimum font-size in CSS px, but only applies to relative font
+    // units like 'small', em, and percentage sizes.
     {
       name: "minimumLogicalFontSize",
       initial: 0,

third_party/blink/web_tests/wpt_internal/css/css-fonts/font-size-ex-ch-with-zoom-ref.html

@@ -3,6 +3,6 @@
 <title>Maintain correct font-size in ex and ch with zoom</title>
 
 <body style="margin: 0; padding: 0;">
-  <div style="font-size: 3ex">size should be 3ex</div>
-  <div style="font-size: 3ch">size should be 3ch</div>
+  <div style="font-size: 6ex">size should be 6ex</div>
+  <div style="font-size: 6ch">size should be 6ch</div>
 </body>

third_party/blink/web_tests/wpt_internal/css/css-fonts/font-size-ex-ch-with-zoom.html

@@ -7,8 +7,8 @@
 <title>Maintain correct font-size in ex and ch with zoom</title>
 
 <body style="margin: 0; padding: 0;">
-  <div style="font-size: 1ex">size should be 3ex</div>
-  <div style="font-size: 1ch">size should be 3ch</div>
+  <div style="font-size: 2ex">size should be 6ex</div>
+  <div style="font-size: 2ch">size should be 6ch</div>
 </body>
 
 <script>