Small win here.
Created attachment 187429 [details] Patch
Comment on attachment 187429 [details] Patch Attachment 187429 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16470772 New failing tests: compositing/checkerboard.html accessibility/anchor-linked-anonymous-block-crash.html http/tests/cache/cancel-multiple-post-xhrs.html animations/3d/state-at-end-event-transform.html animations/3d/replace-filling-transform.html http/tests/cache/history-only-cached-subresource-loads.html compositing/bounds-in-flipped-writing-mode.html accessibility/accessibility-node-reparent.html animations/animation-border-overflow.html accessibility/accessibility-object-detached.html accessibility/anonymous-render-block-in-continuation-causes-crash.html animations/animation-controller-drt-api.html compositing/absolute-position-changed-with-composited-parent-layer.html compositing/absolute-position-changed-in-composited-layer.html http/tests/cache/iframe-304-crash.html animations/3d/transform-perspective.html http/tests/cache/cancel-during-failure-crash.html canvas/philip/tests/2d.clearRect+fillRect.alpha0.5.html canvas/philip/tests/2d.clearRect.basic.html animations/3d/transform-origin-vs-functions.html animations/animation-css-rule-types.html canvas/philip/tests/2d.clearRect+fillRect.basic.html canvas/philip/tests/2d.clearRect+fillRect.alpha0.html
Comment on attachment 187429 [details] Patch Fantastic. Thank you. Mac-WK2 looks sad, but I'm sure you'll check before landing.
Comment on attachment 187429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187429&action=review > Source/WebCore/rendering/RenderText.cpp:1193 > + for (unsigned i = 0; i < textLength(); i++) { > + if (!style()->isCollapsibleWhiteSpace(characterAt(i))) I would store textLength() in a temporary variable. You may want to split this in 2 cases, 8bits and 16bits, instead of doing that in the loop.
Comment on attachment 187429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187429&action=review >> Source/WebCore/rendering/RenderText.cpp:1193 >> + if (!style()->isCollapsibleWhiteSpace(characterAt(i))) > > I would store textLength() in a temporary variable. > > You may want to split this in 2 cases, 8bits and 16bits, instead of doing that in the loop. Ben has a good point about the two cases. The new code is going to be considerably slower than two separate loops. You could prove we need only a single loop with performance testing, or you could write the two loops.
Created attachment 187681 [details] Patch for landing Hoisted the 8/16 bit checks out of the loop as requested.
Comment on attachment 187681 [details] Patch for landing Clearing flags on attachment: 187681 Committed r142529: <http://trac.webkit.org/changeset/142529>
All reviewed patches have been landed. Closing bug.