Unreviewed. Rollout of r156536. ~5% regression in inline layout performance.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Oct 2013 14:39:39 +0000 (14:39 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Oct 2013 14:39:39 +0000 (14:39 +0000)
Source/WebCore:

* rendering/LineWidth.cpp:
* rendering/LineWidth.h:
(WebCore::LineWidth::addUncommittedWidth):
* rendering/RenderBlockLineLayout.cpp:
(WebCore::LineBreaker::nextSegmentBreak):

LayoutTests:

* fast/css/unexpected-word-wrapping-with-non-empty-spans-expected.html: Removed.
* fast/css/unexpected-word-wrapping-with-non-empty-spans.html: Removed.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@157100 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css/unexpected-word-wrapping-with-non-empty-spans-expected.html [deleted file]
LayoutTests/fast/css/unexpected-word-wrapping-with-non-empty-spans.html [deleted file]
Source/WebCore/ChangeLog
Source/WebCore/rendering/LineWidth.cpp
Source/WebCore/rendering/LineWidth.h
Source/WebCore/rendering/RenderBlockLineLayout.cpp

index 1804948..78e8934 100644 (file)
@@ -1,3 +1,10 @@
+2013-10-08  Zalan Bujtas  <zalan@apple.com>
+
+        Unreviewed. Rollout of r156536. ~5% regression in inline layout performance.
+
+        * fast/css/unexpected-word-wrapping-with-non-empty-spans-expected.html: Removed.
+        * fast/css/unexpected-word-wrapping-with-non-empty-spans.html: Removed.
+
 2013-10-08  Michał Pakuła vel Rutka  <m.pakula@samsung.com>
 
         Unreviewed EFL gardening
diff --git a/LayoutTests/fast/css/unexpected-word-wrapping-with-non-empty-spans-expected.html b/LayoutTests/fast/css/unexpected-word-wrapping-with-non-empty-spans-expected.html
deleted file mode 100644 (file)
index d12596f..0000000
+++ /dev/null
@@ -1,23 +0,0 @@
-<html>
-<head>
-  <style>
-    div {
-      border: 1px solid red;
-      width: 16px;
-      word-wrap: break-word;
-    }
-  </style>
-</head>
-<body>
-<p>This tests if non-empty spans changes word wrapping.</p>
-<div>
-    1234
-</div>
-<div>
-    1234
-</div>
-<div>
-    12345678
-    12345678
-</div>
-</html>
diff --git a/LayoutTests/fast/css/unexpected-word-wrapping-with-non-empty-spans.html b/LayoutTests/fast/css/unexpected-word-wrapping-with-non-empty-spans.html
deleted file mode 100644 (file)
index 43a24cf..0000000
+++ /dev/null
@@ -1,23 +0,0 @@
-<html>
-<head>
-  <style>
-    div {
-      border: 1px solid red;
-      width: 16px;
-      word-wrap: break-word;
-    }
-  </style>
-</head>
-<body>
-<p>This tests if non-empty spans changes word wrapping.</p>
-<div>
-    <span>123</span><span></span><span>4</span>
-</div>
-<div>
-    <span></span><span>123</span><span>4</span>
-</div>
-<div>
-    12345678
-    <span>1</span><span>2</span><span>3</span><span>4</span><span>5</span><span>6</span><span>7</span><span>8</span>
-</div>
-</html>
index cea5a48..b2bafcc 100644 (file)
@@ -1,3 +1,13 @@
+2013-10-08  Zalan Bujtas  <zalan@apple.com>
+
+        Unreviewed. Rollout of r156536. ~5% regression in inline layout performance.
+
+        * rendering/LineWidth.cpp:
+        * rendering/LineWidth.h:
+        (WebCore::LineWidth::addUncommittedWidth):
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::LineBreaker::nextSegmentBreak):
+
 2013-10-08  Gyuyoung Kim  <gyuyoung.kim@samsung.com>
 
         Use toCSSFooValue() instead of using static_cast<const CSSFooValue*>
index 54f4b86..80a7ac6 100644 (file)
@@ -150,23 +150,6 @@ void LineWidth::shrinkAvailableWidthForNewFloatIfNeeded(FloatingObject* newFloat
     computeAvailableWidthFromLeftAndRight();
 }
 
-float LineWidth::uncommittedWidthForObject(const RenderObject& object) const
-{
-    auto result = m_uncommittedWidthMap.find(&object);
-    if (result != m_uncommittedWidthMap.end())
-        return result->value;
-    return -1;
-}
-
-void LineWidth::addUncommittedWidth(float delta, const RenderObject& current)
-{
-    m_uncommittedWidth += delta;
-
-    auto result = m_uncommittedWidthMap.add(&current, delta);
-    if (!result.isNewEntry)
-        result.iterator->value += delta;
-}
-
 void LineWidth::commit()
 {
     m_committedWidth += m_uncommittedWidth;
index ba5a6e9..f88c2ab 100644 (file)
@@ -31,7 +31,6 @@
 #define LineWidth_h
 
 #include "LayoutUnit.h"
-#include <wtf/HashMap.h>
 
 namespace WebCore {
 
@@ -55,13 +54,12 @@ public:
     float currentWidth() const { return m_committedWidth + m_uncommittedWidth; }
     // FIXME: We should eventually replace these three functions by ones that work on a higher abstraction.
     float uncommittedWidth() const { return m_uncommittedWidth; }
-    float uncommittedWidthForObject(const RenderObject&) const;
     float committedWidth() const { return m_committedWidth; }
     float availableWidth() const { return m_availableWidth; }
 
     void updateAvailableWidth(LayoutUnit minimumHeight = 0);
     void shrinkAvailableWidthForNewFloatIfNeeded(FloatingObject*);
-    void addUncommittedWidth(float delta, const RenderObject&);
+    void addUncommittedWidth(float delta) { m_uncommittedWidth += delta; }
     void commit();
     void applyOverhang(RenderRubyRun*, RenderObject* startRenderer, RenderObject* endRenderer);
     void fitBelowFloats();
@@ -91,7 +89,6 @@ private:
 #endif
     bool m_isFirstLine;
     IndentTextOrNot m_shouldIndentText;
-    HashMap<const RenderObject*, float> m_uncommittedWidthMap;
 };
 
 }
index cabf01e..4ef79e1 100644 (file)
@@ -2825,7 +2825,7 @@ InlineIterator LineBreaker::nextSegmentBreak(InlineBidiResolver& resolver, LineI
                 trailingObjects.appendBoxIfNeeded(box);
             } else
                 m_positionedObjects.append(box);
-            width.addUncommittedWidth(inlineLogicalWidth(current.m_obj), *current.m_obj);
+            width.addUncommittedWidth(inlineLogicalWidth(current.m_obj));
             // Reset prior line break context characters.
             renderTextInfo.m_lineBreakIterator.resetPriorContext();
         } else if (current.m_obj->isFloating()) {
@@ -2874,7 +2874,7 @@ InlineIterator LineBreaker::nextSegmentBreak(InlineBidiResolver& resolver, LineI
                 }
             }
 
-            width.addUncommittedWidth(inlineLogicalWidth(current.m_obj) + borderPaddingMarginStart(flowBox) + borderPaddingMarginEnd(flowBox), *current.m_obj);
+            width.addUncommittedWidth(inlineLogicalWidth(current.m_obj) + borderPaddingMarginStart(flowBox) + borderPaddingMarginEnd(flowBox));
         } else if (current.m_obj->isReplaced()) {
             RenderBox* replacedBox = toRenderBox(current.m_obj);
 
@@ -2906,9 +2906,9 @@ InlineIterator LineBreaker::nextSegmentBreak(InlineBidiResolver& resolver, LineI
                     ignoringSpaces = true;
                 }
                 if (toRenderListMarker(*current.m_obj).isInside())
-                    width.addUncommittedWidth(replacedLogicalWidth, *current.m_obj);
+                    width.addUncommittedWidth(replacedLogicalWidth);
             } else
-                width.addUncommittedWidth(replacedLogicalWidth, *current.m_obj);
+                width.addUncommittedWidth(replacedLogicalWidth);
             if (current.m_obj->isRubyRun())
                 width.applyOverhang(toRenderRubyRun(current.m_obj), last, next);
             // Update prior line break context characters, using U+FFFD (OBJECT REPLACEMENT CHARACTER) for replaced element.
@@ -2996,7 +2996,7 @@ InlineIterator LineBreaker::nextSegmentBreak(InlineBidiResolver& resolver, LineI
 
                 if (c == softHyphen && autoWrap && !hyphenWidth && style.hyphens() != HyphensNone) {
                     hyphenWidth = measureHyphenWidth(t, f, &fallbackFonts);
-                    width.addUncommittedWidth(hyphenWidth, *current.m_obj);
+                    width.addUncommittedWidth(hyphenWidth);
                 }
 
                 bool applyWordSpacing = false;
@@ -3050,13 +3050,13 @@ InlineIterator LineBreaker::nextSegmentBreak(InlineBidiResolver& resolver, LineI
 
                     wordMeasurement.width = additionalTempWidth + wordSpacingForWordMeasurement;
                     additionalTempWidth += lastSpaceWordSpacing;
-                    width.addUncommittedWidth(additionalTempWidth, *current.m_obj);
+                    width.addUncommittedWidth(additionalTempWidth);
 
                     if (collapseWhiteSpace && previousCharacterIsSpace && currentCharacterIsSpace && additionalTempWidth)
                         width.setTrailingWhitespaceWidth(additionalTempWidth);
 
                     if (!appliedStartWidth) {
-                        width.addUncommittedWidth(inlineLogicalWidth(current.m_obj, true, false), *current.m_obj);
+                        width.addUncommittedWidth(inlineLogicalWidth(current.m_obj, true, false));
                         appliedStartWidth = true;
                     }
 
@@ -3112,10 +3112,10 @@ InlineIterator LineBreaker::nextSegmentBreak(InlineBidiResolver& resolver, LineI
                                 goto end;
                         } else {
                             if (!betweenWords || (midWordBreak && !autoWrap))
-                                width.addUncommittedWidth(-additionalTempWidth, *current.m_obj);
+                                width.addUncommittedWidth(-additionalTempWidth);
                             if (hyphenWidth) {
                                 // Subtract the width of the soft hyphen out since we fit on a line.
-                                width.addUncommittedWidth(-hyphenWidth, *current.m_obj);
+                                width.addUncommittedWidth(-hyphenWidth);
                                 hyphenWidth = 0;
                             }
                         }
@@ -3227,7 +3227,7 @@ InlineIterator LineBreaker::nextSegmentBreak(InlineBidiResolver& resolver, LineI
             additionalTempWidth += lastSpaceWordSpacing;
 
             float inlineLogicalTempWidth = inlineLogicalWidth(current.m_obj, !appliedStartWidth, includeEndWidth);
-            width.addUncommittedWidth(additionalTempWidth + inlineLogicalTempWidth, *current.m_obj);
+            width.addUncommittedWidth(additionalTempWidth + inlineLogicalTempWidth);
 
             if (wordMeasurement.fallbackFonts.isEmpty() && !fallbackFonts.isEmpty())
                 wordMeasurement.fallbackFonts.swap(fallbackFonts);
@@ -3317,7 +3317,7 @@ InlineIterator LineBreaker::nextSegmentBreak(InlineBidiResolver& resolver, LineI
             // make sure we consume at least one char/object.
             if (lBreak == resolver.position())
                 lBreak.increment();
-        } else if (!current.m_pos && !width.committedWidth() && current.m_obj && width.uncommittedWidthForObject(*current.m_obj) == width.uncommittedWidth()) {
+        } else if (!width.committedWidth() && (!current.m_obj || !current.m_obj->isBR()) && !current.m_pos) {
             // Do not push the current object to the next line, when this line has some content, but it is still considered empty.
             // Empty inline elements like <span></span> can produce such lines and now we just ignore these break opportunities
             // at the start of a line, if no width has been committed yet.