Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1514743002:
cc: turn on strict paint property checking (Closed)
|
Created:
5 years ago by enne (OOO) Modified:
4 years, 11 months ago CC:
cc-bugs_chromium.org, chromium-reviews, darin-cc_chromium.org, dtrainor+watch-blimp_chromium.org, jam, kmarshall+watch-blimp_chromium.org, maniscalco+watch-blimp_chromium.org, marcinjb+watch-blimp_chromium.org, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, nyquist+watch-blimp_chromium.org, piman+watch_chromium.org, sriramsr+watch-blimp_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptioncc: turn on strict paint property checking
This is in preparation for getting rid of paint properties entirely.
Turned on the setting always, turned DCHECKs to CHECKs to get better
canary input, and deleted tests that no longer make sense.
I plan on reverting this patch if this CHECK fires on any page.
R=danakj@chromium.org,chrishtr@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
Committed: https://crrev.com/75e2fba8610567c6b903ecd929707660570d3197
Cr-Commit-Position: refs/heads/master@{#367360}
Patch Set 1 #Patch Set 2 : compile fix: remove another flag reference #Patch Set 3 : Fix DragImageView to not resize during paint #
Total comments: 1
Patch Set 4 : Split drag image view changes to another patch #Patch Set 5 : rebase #
Messages
Total messages: 44 (21 generated)
Description was changed from ========== cc: turn on strict paint property checking This is in preparation for getting rid of paint properties entirely. Turned on the setting always, turned DCHECKs to CHECKs to get better canary input, and deleted tests that no longer make sense. R=danakj@chromium.org,chrishtr@chromium.org ========== to ========== cc: turn on strict paint property checking This is in preparation for getting rid of paint properties entirely. Turned on the setting always, turned DCHECKs to CHECKs to get better canary input, and deleted tests that no longer make sense. R=danakj@chromium.org,chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
LGTM
The CQ bit was checked by enne@chromium.org
The CQ bit was unchecked by enne@chromium.org
Description was changed from ========== cc: turn on strict paint property checking This is in preparation for getting rid of paint properties entirely. Turned on the setting always, turned DCHECKs to CHECKs to get better canary input, and deleted tests that no longer make sense. R=danakj@chromium.org,chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== cc: turn on strict paint property checking This is in preparation for getting rid of paint properties entirely. Turned on the setting always, turned DCHECKs to CHECKs to get better canary input, and deleted tests that no longer make sense. I plan on reverting this patch if this CHECK fires on any page. R=danakj@chromium.org,chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
The CQ bit was checked by enne@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1514743002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1514743002/1
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
enne@chromium.org changed reviewers: + dtrainor@chromium.org
+dtrainor for blimp/ rubber stamp
blimp lgtm
The CQ bit was checked by enne@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org, chrishtr@chromium.org, dtrainor@chromium.org Link to the patchset: https://codereview.chromium.org/1514743002/#ps20001 (title: "compile fix: remove another flag reference")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1514743002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1514743002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
+sievers for content/
On 2015/12/10 19:36:33, enne wrote: > +sievers for content/ lgtm
The CQ bit was checked by enne@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1514743002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1514743002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
mfomitchev@chromium.org changed reviewers: + mfomitchev@chromium.org
I think ideally the changes to ash/drag_drop/drag_image_view.* would be done in a separate CL. For example if you have to revert the change because a CHECK in some obscure case fails - wouldn't you want the changes to drag-and-drop to stick regardless? At the very least, I think we should add some reference to the drag-and-drop changes to the CL description. https://codereview.chromium.org/1514743002/diff/40001/ash/drag_drop/drag_imag... File ash/drag_drop/drag_image_view.cc (right): https://codereview.chromium.org/1514743002/diff/40001/ash/drag_drop/drag_imag... ash/drag_drop/drag_image_view.cc:136: if (!drag_hint) Shouldn't this be drag_hint->IsEmpty()?
Ok, I got confused with the CLs - the DnD changes are in crrev.com/1517873003. So I assume you are planning to remove them from here?
On 2015/12/11 at 17:10:56, mfomitchev wrote: > Ok, I got confused with the CLs - the DnD changes are in crrev.com/1517873003. So I assume you are planning to remove them from here? You're correct. I had the same thought and split it out to a separate CL, but just hadn't updated this one yet, sorry.
The CQ bit was checked by enne@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org, chrishtr@chromium.org, dtrainor@chromium.org, sievers@chromium.org Link to the patchset: https://codereview.chromium.org/1514743002/#ps60001 (title: "Split drag image view changes to another patch")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1514743002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1514743002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by enne@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1514743002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1514743002/60001
The CQ bit was unchecked by enne@chromium.org
The CQ bit was checked by enne@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org, chrishtr@chromium.org, dtrainor@chromium.org, sievers@chromium.org Link to the patchset: https://codereview.chromium.org/1514743002/#ps80001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1514743002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1514743002/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== cc: turn on strict paint property checking This is in preparation for getting rid of paint properties entirely. Turned on the setting always, turned DCHECKs to CHECKs to get better canary input, and deleted tests that no longer make sense. I plan on reverting this patch if this CHECK fires on any page. R=danakj@chromium.org,chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== cc: turn on strict paint property checking This is in preparation for getting rid of paint properties entirely. Turned on the setting always, turned DCHECKs to CHECKs to get better canary input, and deleted tests that no longer make sense. I plan on reverting this patch if this CHECK fires on any page. R=danakj@chromium.org,chrishtr@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/75e2fba8610567c6b903ecd929707660570d3197 Cr-Commit-Position: refs/heads/master@{#367360} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/75e2fba8610567c6b903ecd929707660570d3197 Cr-Commit-Position: refs/heads/master@{#367360}
Message was sent while issue was closed.
please see bug 574649 where this is seemingly causing a lot of crashes, top crasher with 76% of crashes. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
