Bug 113364 – Selection code spends a lot of time in InlineTextBox::localSelectionRect

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryosuke Niwa]

As far as I've sampled in the bug 113358, we're spending 5+% of time in InlineTextBox::localSelectionRect computing font width.
But there is no need to do this when we're selecting the entire line box.

[Ryosuke Niwa]

Created attachment 195214 [details]
Work in progress 1

[Ryosuke Niwa]

Created attachment 195215 [details]
A/B test result with WIP1

[Radar WebKit Bug Importer]

<rdar://problem/13513153>

[WebKit Review Bot]

Attachment 195214 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/rendering/InlineTextBox.cpp']" exit_code: 1
Source/WebCore/rendering/InlineTextBox.cpp:211:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 1 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Enrica Casucci]

Comment on attachment 195214 [details]
Work in progress 1

View in context: https://bugs.webkit.org/attachment.cgi?id=195214&action=review

Missing ChangeLog file

>> Source/WebCore/rendering/InlineTextBox.cpp:211
>> +    if (sPos != 0 || ePos != static_cast<int>(m_len))
> 
> Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]

Please fix the style issue and add a comment.

[Ryosuke Niwa]

(In reply to comment #5)
> (From update of attachment 195214 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195214&action=review
> 
> Missing ChangeLog file
> 
> >> Source/WebCore/rendering/InlineTextBox.cpp:211
> >> +    if (sPos != 0 || ePos != static_cast<int>(m_len))
> > 
> > Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
> 
> Please fix the style issue and add a comment.

Oh, oops. This wasn't up for review.  The one on https://bugs.webkit.org/show_bug.cgi?id=113358 is. I'm trying to add the performance test first so that people can verify my assertion that my patch improves the performance.

[Ryosuke Niwa]

Created attachment 195358 [details]
Ready for review

[Enrica Casucci]

Comment on attachment 195358 [details]
Ready for review

View in context: https://bugs.webkit.org/attachment.cgi?id=195358&action=review

> Source/WebCore/rendering/InlineTextBox.cpp:209
> +    FloatPoint startingPoint = FloatPoint(logicalLeft(), selTop);

I would add a comment here about the optimization.

[Ryosuke Niwa]

Committed r147008: <http://trac.webkit.org/changeset/147008>