Bug 71420

Summary: Stop abusing RenderTableSection::needsRecalcCells logic
Product: WebKit Reporter: Julien Chaffraix <jchaffraix>
Component: TablesAssignee: Julien Chaffraix <jchaffraix>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Provide some alternative to needsRecalcCells when it is not appropriate to call it.
none
Added proper rebaseline for the progressing test.
none
Fixed test_expectations.txt that were wrong.
none
Patch for landing none

Julien Chaffraix
Reported 2011-11-02 16:47:54 PDT
The current code badly misuses the needsRecalcCells logic. Cells recalculation is an expensive operation that requires throwing off our internal representation and some layout information. It is meant for changes in the section's structure that mandates such a safe approach. Some part of the code forces this path for logical height changes, which does not make sense at all. Let's remove those bad usage.
Attachments
Provide some alternative to needsRecalcCells when it is not appropriate to call it. (7.48 KB, patch)
2011-11-02 17:26 PDT, Julien Chaffraix
no flags
Added proper rebaseline for the progressing test. (41.55 KB, patch)
2011-11-02 18:55 PDT, Julien Chaffraix
no flags
Fixed test_expectations.txt that were wrong. (41.56 KB, patch)
2011-11-03 09:19 PDT, Julien Chaffraix
no flags
Patch for landing (41.57 KB, patch)
2011-11-03 10:03 PDT, Julien Chaffraix
no flags
Julien Chaffraix
Comment 1 2011-11-02 17:26:07 PDT
Created attachment 113406 [details] Provide some alternative to needsRecalcCells when it is not appropriate to call it.
WebKit Review Bot
Comment 2 2011-11-02 18:10:07 PDT
Comment on attachment 113406 [details] Provide some alternative to needsRecalcCells when it is not appropriate to call it. Attachment 113406 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10128777 New failing tests: fast/repaint/table-extra-bottom-grow.html
Julien Chaffraix
Comment 3 2011-11-02 18:55:03 PDT
Created attachment 113416 [details] Added proper rebaseline for the progressing test.
Julien Chaffraix
Comment 4 2011-11-03 09:19:51 PDT
Created attachment 113503 [details] Fixed test_expectations.txt that were wrong.
Julien Chaffraix
Comment 5 2011-11-03 10:03:27 PDT
Created attachment 113512 [details] Patch for landing
WebKit Review Bot
Comment 6 2011-11-03 10:20:18 PDT
Comment on attachment 113512 [details] Patch for landing Clearing flags on attachment: 113512 Committed r99212: <http://trac.webkit.org/changeset/99212>
WebKit Review Bot
Comment 7 2011-11-03 10:20:23 PDT
All reviewed patches have been landed. Closing bug.
Csaba Osztrogonác
Comment 8 2011-11-03 10:41:48 PDT
(In reply to comment #6) > (From update of attachment 113512 [details]) > Clearing flags on attachment: 113512 > > Committed r99212: <http://trac.webkit.org/changeset/99212> FYI: It broke layout testing on qt-mac platform, because fast/repaint/table-extra-bottom-grow.html is in qt-mac/Skipped file too and NRWT doesn't like duplicated entries. 2011-11-03 10:38:35,835 16718 test_expectations.py:791 ERROR FAILURES FOR <mac, x86, release, cpu> in LayoutTests/platform/qt/test_expectations.txt 2011-11-03 10:38:35,836 16718 test_expectations.py:794 ERROR Line:5530 Duplicate or ambiguous expectation. fast/repaint/table-extra-bottom-grow.html I removed this entry from test_expectations.txt.
Julien Chaffraix
Comment 9 2011-11-03 11:02:24 PDT
(In reply to comment #8) > (In reply to comment #6) > > (From update of attachment 113512 [details] [details]) > > Clearing flags on attachment: 113512 > > > > Committed r99212: <http://trac.webkit.org/changeset/99212> > > FYI: It broke layout testing on qt-mac platform, because fast/repaint/table-extra-bottom-grow.html is in qt-mac/Skipped file too and NRWT doesn't like duplicated entries. > > 2011-11-03 10:38:35,835 16718 test_expectations.py:791 ERROR FAILURES FOR <mac, x86, release, cpu> in LayoutTests/platform/qt/test_expectations.txt > 2011-11-03 10:38:35,836 16718 test_expectations.py:794 ERROR Line:5530 Duplicate or ambiguous expectation. fast/repaint/table-extra-bottom-grow.html > > I removed this entry from test_expectations.txt. Thanks for fixing, Ossy! It's too bad --lint-test-files only runs on the current platform and there is no way of checking all platforms.
Note You need to log in before you can comment on or make changes to this bug.