Several call sites are wrongly doubling the maximal outline when manipulating the repaint rectangle: * RenderReplaced * RenderTableSection * RenderTableCell There is no historical explanation for the doubling. Moreover the rest of the code handle the maximalOutlineSize by just inflating the repaint rectangle once so it looks like a bug!
Created attachment 129303 [details] Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic.
Comment on attachment 129303 [details] Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic. View in context: https://bugs.webkit.org/attachment.cgi?id=129303&action=review > Source/WebCore/ChangeLog:8 > + Refactoring only, covered by existing tests (mostly repaint ones). Can you name the pixel tests that need a rebaseline from this? Maybe we can pre-emptively mark them in test_expectations.txt so the bots don't go red.
Comment on attachment 129303 [details] Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic. View in context: https://bugs.webkit.org/attachment.cgi?id=129303&action=review >> Source/WebCore/ChangeLog:8 >> + Refactoring only, covered by existing tests (mostly repaint ones). > > Can you name the pixel tests that need a rebaseline from this? Maybe we can pre-emptively mark them in test_expectations.txt so the bots don't go red. There is none that needs to be rebaselined. However there are several tests involving tables and replaced elements in our tree. The repaint tests are the one that checks that I did not break our repainting by "shrinking" the outline. I can change the wording if you want.
Comment on attachment 129303 [details] Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic. View in context: https://bugs.webkit.org/attachment.cgi?id=129303&action=review >>> Source/WebCore/ChangeLog:8 >>> + Refactoring only, covered by existing tests (mostly repaint ones). >> >> Can you name the pixel tests that need a rebaseline from this? Maybe we can pre-emptively mark them in test_expectations.txt so the bots don't go red. > > There is none that needs to be rebaselined. However there are several tests involving tables and replaced elements in our tree. The repaint tests are the one that checks that I did not break our repainting by "shrinking" the outline. I can change the wording if you want. I see, I was confused by "Stop doubling maximalOutlineSize during painting", which sounds like a different behavior. I think the current working is OK. Feel free to cq+.
Comment on attachment 129303 [details] Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic. View in context: https://bugs.webkit.org/attachment.cgi?id=129303&action=review >>>> Source/WebCore/ChangeLog:8 >>>> + Refactoring only, covered by existing tests (mostly repaint ones). >>> >>> Can you name the pixel tests that need a rebaseline from this? Maybe we can pre-emptively mark them in test_expectations.txt so the bots don't go red. >> >> There is none that needs to be rebaselined. However there are several tests involving tables and replaced elements in our tree. The repaint tests are the one that checks that I did not break our repainting by "shrinking" the outline. I can change the wording if you want. > > I see, I was confused by "Stop doubling maximalOutlineSize during painting", which sounds like a different behavior. I think the current working is OK. Feel free to cq+. OK , cqinating :-)
Comment on attachment 129303 [details] Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic. Clearing flags on attachment: 129303 Committed r109246: <http://trac.webkit.org/changeset/109246>
All reviewed patches have been landed. Closing bug.