Bug 71420 - Stop abusing RenderTableSection::needsRecalcCells logic
Summary: Stop abusing RenderTableSection::needsRecalcCells logic
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tables (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Julien Chaffraix
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-02 16:47 PDT by Julien Chaffraix
Modified: 2011-11-03 11:02 PDT (History)
3 users (show)

See Also:


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 Details | Formatted Diff | Diff
Added proper rebaseline for the progressing test. (41.55 KB, patch)
2011-11-02 18:55 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Fixed test_expectations.txt that were wrong. (41.56 KB, patch)
2011-11-03 09:19 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Patch for landing (41.57 KB, patch)
2011-11-03 10:03 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Chaffraix 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.
Comment 1 Julien Chaffraix 2011-11-02 17:26:07 PDT
Created attachment 113406 [details]
Provide some alternative to needsRecalcCells when it is not appropriate to call it.
Comment 2 WebKit Review Bot 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
Comment 3 Julien Chaffraix 2011-11-02 18:55:03 PDT
Created attachment 113416 [details]
Added proper rebaseline for the progressing test.
Comment 4 Julien Chaffraix 2011-11-03 09:19:51 PDT
Created attachment 113503 [details]
Fixed test_expectations.txt that were wrong.
Comment 5 Julien Chaffraix 2011-11-03 10:03:27 PDT
Created attachment 113512 [details]
Patch for landing
Comment 6 WebKit Review Bot 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>
Comment 7 WebKit Review Bot 2011-11-03 10:20:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Csaba Osztrogonác 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.
Comment 9 Julien Chaffraix 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.