Bug 124273

Summary: [CSS Regions] Redundant computation of flowThreadPortionOverflowRect
Product: WebKit Reporter: Manuel Rego Casasnovas <rego>
Component: CSSAssignee: Manuel Rego Casasnovas <rego>
Status: RESOLVED INVALID    
Severity: Normal CC: buildbot, commit-queue, eflews.bot, esprehn+autocc, glenn, gyuyoung.kim, jfernandez, kondapallykalyan, mihnea, rniwa, WebkitBugTracker
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 118665, 119230    
Bug Blocks: 118668    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
none
Patch none

Description Manuel Rego Casasnovas 2013-11-13 03:22:52 PST
During the selection in documents with CSS Regions RenderSelectionInfo::repaint() is called several times. Which ends up calling RenderRegion::repaintFlowThreadContent():
void RenderRegion::repaintFlowThreadContent(const LayoutRect& repaintRect, bool immediate) const         
{                                                                                                        
    repaintFlowThreadContentRectangle(repaintRect, immediate, flowThreadPortionRect(), flowThreadPortionOverflowRect(), contentBoxRect().location());
}

flowThreadPortionRect() is already stored in a data member of RenderRengion. However flowThreadPortionOverflowRect() is computed again every time calling overflowRectForFlowThreadPortion().
Comment 1 Manuel Rego Casasnovas 2013-11-13 03:33:29 PST
Created attachment 216789 [details]
Patch
Comment 2 EFL EWS Bot 2013-11-13 03:47:38 PST
Comment on attachment 216789 [details]
Patch

Attachment 216789 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/22589900
Comment 3 Manuel Rego Casasnovas 2013-11-13 03:54:34 PST
(In reply to comment #1)
> Created an attachment (id=216789) [details]
> Patch

Just for reference in my machine these are the results of running the performance test Layout/RegionsSelection.html (uploaded to bug #119230) with and without this patch:
* Without this patch: 5204.433 ms
* With this patch: 4696.187 ms

Also you can do a quick test with the following HTML example and check the time required to do the selection in the JavaScript console:
http://people.igalia.com/mrego/css/perf/test-regions.html

Again the results in my machine are:
* Without this patch: 186.957 ms
* With this patch: 163.778 ms
Comment 4 Build Bot 2013-11-13 06:06:09 PST
Comment on attachment 216789 [details]
Patch

Attachment 216789 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/22539954

New failing tests:
fast/multicol/newmulticol/layers-split-across-columns.html
fast/multicol/transform-inside-opacity.html
fast/multicol/newmulticol/float-paginate.html
fast/multicol/newmulticol/float-multicol.html
fast/multicol/newmulticol/cell-shrinkback.html
fast/multicol/newmulticol/hide-box-vertical-rl.html
fast/multicol/newmulticol/layers-in-multicol.html
fast/multicol/mixed-positioning-stacking-order.html
fast/multicol/newmulticol/clipping.html
fast/multicol/newmulticol/fixed-height-fill-auto.html
fast/multicol/newmulticol/float-avoidance.html
fast/multicol/newmulticol/single-line.html
fast/multicol/positioned-outside-of-columns.html
fast/multicol/newmulticol/float-paginate-complex.html
fast/multicol/mixed-opacity-test.html
fast/multicol/newmulticol/float-paginate-empty-lines.html
fast/multicol/mixed-opacity-fixed-test.html
fast/multicol/newmulticol/direct-child-column-span-all.html
fast/multicol/newmulticol/hide-box-vertical-lr.html
fast/multicol/newmulticol/columns-shorthand-parsing.html
fast/regions/region-styling/region-style-in-columns.html
fast/multicol/newmulticol/positioned-split.html
fast/multicol/newmulticol/column-rules-fixed-height.html
fast/multicol/newmulticol/positioned-with-constrained-height.html
Comment 5 Build Bot 2013-11-13 06:06:11 PST
Created attachment 216794 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 6 Build Bot 2013-11-13 07:04:43 PST
Comment on attachment 216789 [details]
Patch

Attachment 216789 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/22640020

New failing tests:
fast/multicol/newmulticol/layers-split-across-columns.html
fast/multicol/transform-inside-opacity.html
fast/multicol/newmulticol/float-paginate.html
fast/multicol/newmulticol/float-multicol.html
fast/multicol/newmulticol/cell-shrinkback.html
fast/multicol/newmulticol/hide-box-vertical-rl.html
fast/multicol/newmulticol/layers-in-multicol.html
fast/multicol/mixed-positioning-stacking-order.html
fast/multicol/newmulticol/clipping.html
fast/multicol/newmulticol/fixed-height-fill-auto.html
fast/multicol/newmulticol/float-avoidance.html
fast/multicol/newmulticol/single-line.html
fast/multicol/positioned-outside-of-columns.html
fast/multicol/newmulticol/float-paginate-complex.html
fast/multicol/mixed-opacity-test.html
fast/multicol/newmulticol/float-paginate-empty-lines.html
fast/multicol/mixed-opacity-fixed-test.html
fast/multicol/newmulticol/direct-child-column-span-all.html
fast/multicol/newmulticol/hide-box-vertical-lr.html
fast/multicol/newmulticol/columns-shorthand-parsing.html
fast/regions/region-styling/region-style-in-columns.html
fast/multicol/newmulticol/positioned-split.html
fast/multicol/newmulticol/column-rules-fixed-height.html
fast/multicol/newmulticol/positioned-with-constrained-height.html
Comment 7 Build Bot 2013-11-13 07:04:45 PST
Created attachment 216797 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 8 Build Bot 2013-11-13 13:28:29 PST
Comment on attachment 216789 [details]
Patch

Attachment 216789 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/22779999

New failing tests:
fast/multicol/newmulticol/layers-split-across-columns.html
fast/multicol/transform-inside-opacity.html
fast/multicol/newmulticol/float-multicol.html
fast/multicol/newmulticol/float-paginate.html
fast/multicol/newmulticol/cell-shrinkback.html
fast/multicol/newmulticol/hide-box-vertical-rl.html
fast/multicol/newmulticol/layers-in-multicol.html
fast/multicol/mixed-positioning-stacking-order.html
fast/multicol/newmulticol/clipping.html
fast/multicol/newmulticol/fixed-height-fill-auto.html
fast/multicol/newmulticol/float-avoidance.html
fast/multicol/newmulticol/single-line.html
fast/multicol/positioned-outside-of-columns.html
fast/multicol/newmulticol/float-paginate-complex.html
fast/multicol/mixed-opacity-test.html
fast/multicol/newmulticol/float-paginate-empty-lines.html
fast/multicol/newmulticol/direct-child-column-span-all.html
fast/multicol/newmulticol/hide-box-vertical-lr.html
fast/multicol/newmulticol/columns-shorthand-parsing.html
fast/regions/region-styling/region-style-in-columns.html
fast/multicol/newmulticol/positioned-split.html
fast/multicol/newmulticol/column-rules-fixed-height.html
fast/multicol/newmulticol/positioned-with-constrained-height.html
Comment 9 Build Bot 2013-11-13 13:28:31 PST
Created attachment 216848 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 10 Manuel Rego Casasnovas 2013-11-14 08:12:01 PST
Created attachment 216937 [details]
Patch

Code related to this patch is going to change in bug #118665, so this patch would need to be updated once it lands.
Comment 11 Manuel Rego Casasnovas 2013-11-21 14:10:54 PST
After patch for bug #118665 has landed (r159609), RenderRegion::flowThreadPortionOverflowRect() is not used anymore in RenderRegion::repaintFlowThreadContent() so this patch is invalid.