Summary: | [CSS Regions] Add ASSERT to make sure using the flowThread cache does not return incorrect results | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Radu Stavila <stavila> | ||||||||
Component: | CSS | Assignee: | Radu Stavila <stavila> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, esprehn+autocc, glenn, kondapallykalyan, WebkitBugTracker | ||||||||
Priority: | P2 | Keywords: | AdobeTracked | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 132913 | ||||||||||
Bug Blocks: | 57312 | ||||||||||
Attachments: |
|
Description
Radu Stavila
2014-05-14 07:17:41 PDT
Created attachment 231448 [details]
Patch
Comment on attachment 231448 [details]
Patch
r=me
Comment on attachment 231448 [details] Patch Clearing flags on attachment: 231448 Committed r168837: <http://trac.webkit.org/changeset/168837> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by bug 132913 The assert fires on 6 tests, let's roll out? http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK2%20(Tests)/r168837%20(4654)/results.html Created attachment 231499 [details] Patch which applies only to regions Applied assertion only for regions, as multicol still has some issues. Created https://bugs.webkit.org/show_bug.cgi?id=132946 for multicol. Comment on attachment 231499 [details] Patch which applies only to regions View in context: https://bugs.webkit.org/attachment.cgi?id=231499&action=review > Source/WebCore/rendering/RenderObject.cpp:550 > + // Failing tests when enabling this for multicol: > + // http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK2%20(Tests)/r168837%20(4654)/results.html I don't think it's useful to put this URL in a comment. Eventually the test results will be purged from the server. This kind of info should go in the bug. > Source/WebCore/rendering/RenderObject.cpp:552 > + ASSERT(flowThread == locateFlowThreadContainingBlockNoCache()); This if() block only contains an assertion. It would be better to make the "if" test part of the assertion so it's compiled out in release. > Source/WebCore/rendering/RenderObject.h:893 > + // This will walk the tree to find the flow thread. Comment seems to be inaccurate if you're in layout. Created attachment 231577 [details]
Patch for landing
Comment on attachment 231577 [details] Patch for landing Clearing flags on attachment: 231577 Committed r168971: <http://trac.webkit.org/changeset/168971> |