Reserve enough vertical space for the menuList layout element setting its logical height to the max…

third_party/WebKit/Source/core/layout/LayoutMenuList.cpp

Index: third_party/WebKit/Source/core/layout/LayoutMenuList.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutMenuList.cpp b/third_party/WebKit/Source/core/layout/LayoutMenuList.cpp
index 44c8bd0bfafb2a38ab59ec356980a1c4381ef988..7638c263e6adbf7391ce3099e7506b5619687518 100644
--- a/third_party/WebKit/Source/core/layout/LayoutMenuList.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutMenuList.cpp
@@ -35,6 +35,10 @@
 
 namespace blink {
 
+// Vertical padding that is added to the element's logical height.
+// This is needed so that the select's layout is consistent with other browsers, e.g. Gecko.
+const int cInnerBlockVerticalPaddingEm = 1;
+
 LayoutMenuList::LayoutMenuList(Element* element)
     : LayoutFlexibleBox(element)
     , m_buttonText(nullptr)
@@ -42,6 +46,7 @@ LayoutMenuList::LayoutMenuList(Element* element)
     , m_optionsChanged(true)
     , m_isEmpty(false)
     , m_hasUpdatedActiveOption(false)
+    , m_optionsHeight(0)
     , m_optionsWidth(0)
     , m_lastActiveIndex(-1)
 {
@@ -148,11 +153,12 @@ void LayoutMenuList::styleDidChange(StyleDifference diff, const ComputedStyle* o
 
     bool fontChanged = !oldStyle || oldStyle->font() != style()->font();
     if (fontChanged)
-        updateOptionsWidth();
+        updateOptionsHeightWidth();
 }
 
-void LayoutMenuList::updateOptionsWidth()
+void LayoutMenuList::updateOptionsHeightWidth()
 {
+    float maxOptionHeight = 0;
     float maxOptionWidth = 0;
     const HeapVector<Member<HTMLElement>>& listItems = selectElement()->listItems();
     int size = listItems.size();
@@ -163,38 +169,64 @@ void LayoutMenuList::updateOptionsWidth()
             continue;
 
         String text = toHTMLOptionElement(element)->textIndentedToRespectGroupLabel();
-        applyTextTransform(style(), text, ' ');
+
+        const ComputedStyle* itemStyle = element->computedStyle() ? element->computedStyle() : style();
+        applyTextTransform(itemStyle, text, ' ');
+        TextRun textRun = constructTextRun(itemStyle->font(), text, *itemStyle);
+
+        maxOptionHeight = std::max(maxOptionHeight, computeTextHeight(textRun, *itemStyle));
+
         if (LayoutTheme::theme().popupOptionSupportsTextIndent()) {
             // Add in the option's text indent.  We can't calculate percentage values for now.
             float optionWidth = 0;
             if (const ComputedStyle* optionStyle = element->computedStyle())
                 optionWidth += minimumValueForLength(optionStyle->textIndent(), LayoutUnit());
-            if (!text.isEmpty())
-                optionWidth += computeTextWidth(text);
+            if (!text.isEmpty()) {
+                TextRun textRun = constructTextRun(itemStyle->font(), text, styleRef());
+                optionWidth += computeTextWidth(textRun, *itemStyle);
+            }
             maxOptionWidth = std::max(maxOptionWidth, optionWidth);
         } else if (!text.isEmpty()) {
-            maxOptionWidth = std::max(maxOptionWidth, computeTextWidth(text));
+            maxOptionWidth = std::max(maxOptionWidth, computeTextWidth(textRun, *itemStyle));
         }
     }
 
+    int height = static_cast<int>(ceilf(maxOptionHeight));
     int width = static_cast<int>(ceilf(maxOptionWidth));
-    if (m_optionsWidth == width)
+    if (m_optionsWidth == width && m_optionsHeight == height)
         return;
 
-    m_optionsWidth = width;
+    auto invalidationReason = LayoutInvalidationReason::Unknown;
+    if (m_optionsWidth != width) {
+        m_optionsWidth = width;
+        invalidationReason = LayoutInvalidationReason::MenuWidthChanged;
+    }
+
+    if (m_optionsHeight != height) {
+        m_optionsHeight = height;
+        invalidationReason = LayoutInvalidationReason::MenuHeightChanged;
+    }
+
     if (parent())
-        setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation(LayoutInvalidationReason::MenuWidthChanged);
+        setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation(invalidationReason);
+}
+
+float LayoutMenuList::computeTextHeight(const TextRun& textRun, const ComputedStyle& computedStyle) const
+{
+    FloatRect glyphBounds;
+    computedStyle.font().width(textRun, nullptr /* fallbackFonts */, &glyphBounds);
+    return glyphBounds.height();
 }
 
-float LayoutMenuList::computeTextWidth(const String& text) const
+float LayoutMenuList::computeTextWidth(const TextRun& textRun, const ComputedStyle& computedStyle) const
 {
-    return style()->font().width(constructTextRun(style()->font(), text, styleRef()));
+    return computedStyle.font().width(textRun);
 }
 
 void LayoutMenuList::updateFromElement()
 {
     if (m_optionsChanged) {
-        updateOptionsWidth();
+        updateOptionsHeightWidth();
         m_optionsChanged = false;
     }
 
@@ -302,6 +334,14 @@ void LayoutMenuList::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth,
         minLogicalWidth = maxLogicalWidth;
 }
 
+void LayoutMenuList::computeLogicalHeight(LayoutUnit logicalHeight, LayoutUnit logicalTop,
+    LogicalExtentComputedValues& computedValues) const
+{
+    logicalHeight = LayoutUnit(m_optionsHeight) + borderAndPaddingHeight()
+        + LayoutUnit(cInnerBlockVerticalPaddingEm * style()->computedFontSize());

[tkent, 2016/06/20 23:57:19]

Do we really need to add this?
I'm afraid this caused a regression.  crbug.com/620613

+    LayoutBox::computeLogicalHeight(logicalHeight, logicalTop, computedValues);
+}
+
 void LayoutMenuList::didSetSelectedIndex(int optionIndex)
 {
     didUpdateActiveOption(optionIndex);