Text inside input field gets misaligned when relaying out

Source/core/layout/LayoutTextControlSingleLine.cpp

 /**
  * Copyright (C) 2006, 2007, 2010 Apple Inc. All rights reserved.
  *           (C) 2008 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2010 Google Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
@@ skipping 76 matching lines @@
 
 LayoutUnit LayoutTextControlSingleLine::computeLogicalHeightLimit() const
 {
     return containerElement() ? contentLogicalHeight() : logicalHeight();
 }
 
 void LayoutTextControlSingleLine::layout()
 {
     SubtreeLayoutScope layoutScope(*this);
 
+    // FIXME: This code is madness (https://crbug.com/461117)

[leviw_travelin_and_unemployed, 2015/02/24 18:38:25]

Well said, sir!

     // FIXME: We should remove the height-related hacks in layout() and
     // styleDidChange(). We need them because
     // - Center the inner elements vertically if the input height is taller than
     //   the intrinsic height of the inner elements.
     // - Shrink the inner elment heights if the input height is samller than the
     //   intrinsic heights of the inner elements.
 
     // We don't honor paddings and borders for textfields without decorations
     // and type=search if the text height is taller than the contentHeight()
     // because of compability.
 
     RenderBox* innerEditorRenderer = innerEditorElement()->renderBox();
+    bool innerEditorRendererHadLayout = innerEditorRenderer && innerEditorRenderer->needsLayout();

[leviw_travelin_and_unemployed, 2015/02/24 18:38:25]

innerEditorRendererHadNeedsLayout?

[Julien - ping for review, 2015/02/24 18:48:01]

I prefer HadLayout as it corresponds to what's happening as we call layout
below.

     RenderBox* viewPortRenderer = editingViewPortElement() ? editingViewPortElement()->renderBox() : 0;
 
     // To ensure consistency between layouts, we need to reset any conditionally overriden height.
     if (innerEditorRenderer && !innerEditorRenderer->style()->logicalHeight().isAuto()) {
         innerEditorRenderer->style()->setLogicalHeight(Length(Auto));
         layoutScope.setNeedsLayout(innerEditorRenderer);
         HTMLElement* placeholderElement = inputElement()->placeholderElement();
         if (RenderBox* placeholderBox = placeholderElement ? placeholderElement->renderBox() : 0)
             layoutScope.setNeedsLayout(placeholderBox);
     }
@@ skipping 10 matching lines @@
     // Set the text block height
     LayoutUnit desiredLogicalHeight = textBlockLogicalHeight();
     LayoutUnit logicalHeightLimit = computeLogicalHeightLimit();
     if (innerEditorRenderer && innerEditorRenderer->logicalHeight() > logicalHeightLimit) {
         if (desiredLogicalHeight != innerEditorRenderer->logicalHeight())
             layoutScope.setNeedsLayout(this);
 
         m_desiredInnerEditorLogicalHeight = desiredLogicalHeight;
 
         innerEditorRenderer->style()->setLogicalHeight(Length(desiredLogicalHeight, Fixed));
         layoutScope.setNeedsLayout(innerEditorRenderer);

[leviw_travelin_and_unemployed, 2015/02/24 18:38:25]

Do we still need this?

[Julien - ping for review, 2015/02/24 18:48:02]

Yes. We won't force a relayout if you had a layout in the first pass above and
this requires an extra one.

         if (viewPortRenderer) {
             viewPortRenderer->style()->setLogicalHeight(Length(desiredLogicalHeight, Fixed));
             layoutScope.setNeedsLayout(viewPortRenderer);
         }
     }
     // The container might be taller because of decoration elements.
     if (containerRenderer) {
         containerRenderer->layoutIfNeeded();
         LayoutUnit containerLogicalHeight = containerRenderer->logicalHeight();
         if (containerLogicalHeight > logicalHeightLimit) {
             containerRenderer->style()->setLogicalHeight(Length(logicalHeightLimit, Fixed));
             layoutScope.setNeedsLayout(this);
         } else if (containerRenderer->logicalHeight() < contentLogicalHeight()) {
             containerRenderer->style()->setLogicalHeight(Length(contentLogicalHeight(), Fixed));
             layoutScope.setNeedsLayout(this);
         } else {
             containerRenderer->style()->setLogicalHeight(Length(containerLogicalHeight, Fixed));
         }
     }
 
+    // We ensure that the inner editor renderer is laid out at least once. This is
+    // required as the logic below assumes that we don't carry over previous layout values.
+    if (innerEditorRenderer && !innerEditorRendererHadLayout)
+        layoutScope.setNeedsLayout(innerEditorRenderer);
+
     // If we need another layout pass, we have changed one of children's height so we need to relayout them.
     if (needsLayout())
         RenderBlockFlow::layoutBlock(true);
 
     // Center the child block in the block progression direction (vertical centering for horizontal text fields).
     if (!container && innerEditorRenderer && innerEditorRenderer->size().height() != contentLogicalHeight()) {
         LayoutUnit logicalHeightDiff = innerEditorRenderer->logicalHeight() - contentLogicalHeight();
         innerEditorRenderer->setLogicalTop(innerEditorRenderer->logicalTop() - (logicalHeightDiff / 2 + layoutMod(logicalHeightDiff, 2)));
     } else {
         centerContainerIfNeeded(containerRenderer);
@@ skipping 252 matching lines @@
     if (innerEditorElement())
         innerEditorElement()->setScrollTop(newTop);
 }
 
 HTMLInputElement* LayoutTextControlSingleLine::inputElement() const
 {
     return toHTMLInputElement(node());
 }
 
 }