Chromium Code Reviews
|
|
|
Created:
4 years, 10 months ago by Stephen Chennney Modified:
4 years, 9 months ago Reviewers:
chrishtr CC:
blink-reviews, blink-reviews-paint_chromium.org, dshwang, slimming-paint-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Target Ref:
refs/heads/master Project:
blink Visibility:
Public. |
DescriptionRemove diagnostic code from BlockPainter.
There have been no crashes since the diagnostic code was added,
suggesting that the minor change in behavior that the code
introduced was sufficient to avoid the crash. Either that or
just having more code there avoided the crash, which would be
odd.
This change keeps the new behavior while removing the
diagnostics.
R=chrishtr
BUG=475698
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197857
Patch Set 1 #Patch Set 2 : Mac expectations, not sure why this is changed #
Total comments: 2
Messages
Total messages: 25 (10 generated)
The CQ bit was checked by schenney@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1205143003/1
I cleaning up after myself. There have been no recent crashes hitting this code, so I'm leaving the slight change in behavior but removing the unrolled code.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/60645)
The CQ bit was checked by chrishtr@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1205143003/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/60663)
The CQ bit was checked by schenney@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1205143003/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/60768)
The Mac rel result is now better than it was before (it's drawing text). I can't figure out why this patch changes anything.
The CQ bit was checked by schenney@chromium.org to run a CQ dry run
The patchset sent to the CQ was uploaded after l-g-t-m from chrishtr@chromium.org Link to the patchset: https://codereview.chromium.org/1205143003/#ps20001 (title: "Mac expectations, not sure why this is changed")
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1205143003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by schenney@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1205143003/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://src.chromium.org/viewvc/blink?view=rev&revision=197857
Message was sent while issue was closed.
https://codereview.chromium.org/1205143003/diff/20001/Source/core/paint/Block... File Source/core/paint/BlockPainter.cpp (right): https://codereview.chromium.org/1205143003/diff/20001/Source/core/paint/Block... Source/core/paint/BlockPainter.cpp:42: if (m_layoutBlock.isDocumentElement()) { I think this conditional needs to be inverted. It appears to have caused the performance regression in crbug.com/504955.
Message was sent while issue was closed.
https://codereview.chromium.org/1205143003/diff/20001/Source/core/paint/Block... File Source/core/paint/BlockPainter.cpp (right): https://codereview.chromium.org/1205143003/diff/20001/Source/core/paint/Block... Source/core/paint/BlockPainter.cpp:42: if (m_layoutBlock.isDocumentElement()) { On 2015/07/13 17:09:40, chrishtr wrote: > I think this conditional needs to be inverted. It appears to have caused the > performance regression in crbug.com/504955. Can't believe I messed this up, and nothing broke is even more unbelievable. https://codereview.chromium.org/1219093002/ |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
