Summary: | Selection code spends a lot of time in InlineTextBox::localSelectionRect | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||
Component: | HTML Editing | Assignee: | Ryosuke Niwa <rniwa> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | darin, enrica, eric, esprehn+autocc, esprehn, leviw, ojan.autocc, ojan, tony, webkit-bug-importer, webkit.review.bot | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 113358 | ||||||||||
Bug Blocks: | 113478 | ||||||||||
Attachments: |
|
Description
Ryosuke Niwa
2013-03-26 21:01:11 PDT
Created attachment 195214 [details]
Work in progress 1
Created attachment 195215 [details]
A/B test result with WIP1
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.
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. (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. Created attachment 195358 [details]
Ready for review
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. Committed r147008: <http://trac.webkit.org/changeset/147008> |