Created attachment 277484 [details] Test reduction See test reduction.
rdar://problem/24389168
Created attachment 277486 [details] Patch
Comment on attachment 277486 [details] Patch Attachment 277486 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1229698 New failing tests: fast/text/text-node-remains-dirty-after-calling-surroundContents.html
Created attachment 277490 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 277486 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277486&action=review > Source/WebCore/ChangeLog:10 > + The remove operation marks the ancestor tree dirty (and this newly constructed line is supposed to be clean) Missing period at the end. > Source/WebCore/rendering/RenderBlockLineLayout.cpp:973 > + // removeFromParent() unnecessarily dirties the ancestor subtree. But do we know that these dirty bits came from the removeFromParent(), or could they have been dirty already? > Source/WebCore/rendering/RenderText.cpp:1271 > - // FIXME: should not be needed!!! > - if (!textBox.len()) { > - // We want the box to be destroyed. > - textBox.removeFromParent(); > - m_lineBoxes.remove(textBox); > - delete &textBox; > - return; > - } > - > m_containsReversedText |= !textBox.isLeftToRightDirection(); This seems like a behavior change. Previously, you would have never hit the last statement if len was 0. Now you do.
Comment on attachment 277486 [details] Patch Attachment 277486 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1229705 New failing tests: fast/text/text-node-remains-dirty-after-calling-surroundContents.html
Created attachment 277492 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4
Comment on attachment 277486 [details] Patch Attachment 277486 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1229710 New failing tests: fast/text/text-node-remains-dirty-after-calling-surroundContents.html
Created attachment 277494 [details] Archive of layout-test-results from ews115 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 277622 [details] Patch
(In reply to comment #5) > Comment on attachment 277486 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=277486&action=review > > > Source/WebCore/ChangeLog:10 > > + The remove operation marks the ancestor tree dirty (and this newly constructed line is supposed to be clean) > > Missing period at the end. > > > Source/WebCore/rendering/RenderBlockLineLayout.cpp:973 > > + // removeFromParent() unnecessarily dirties the ancestor subtree. > > But do we know that these dirty bits came from the removeFromParent(), or > could they have been dirty already? It's a newly constructed line, it's not supposed to be dirty. (fixed) > > > Source/WebCore/rendering/RenderText.cpp:1271 > > - // FIXME: should not be needed!!! > > - if (!textBox.len()) { > > - // We want the box to be destroyed. > > - textBox.removeFromParent(); > > - m_lineBoxes.remove(textBox); > > - delete &textBox; > > - return; > > - } > > - > > m_containsReversedText |= !textBox.isLeftToRightDirection(); > > This seems like a behavior change. Previously, you would have never hit the > last statement if len was 0. Now you do. I assumed a zero length string has no impact on this. (fixed)
Created attachment 277623 [details] Patch
Created attachment 277644 [details] Patch
Comment on attachment 277644 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277644&action=review > Source/WebCore/rendering/RenderBlockLineLayout.cpp:967 > +#ifndef NDEBUG Use !ASSERT_DISABLED
Created attachment 277655 [details] Patch
Comment on attachment 277655 [details] Patch Clearing flags on attachment: 277655 Committed r200220: <http://trac.webkit.org/changeset/200220>
All reviewed patches have been landed. Closing bug.