Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1504673003:
don't rely on -Wno-narrowing in skcanvas_video_renderer.cc (Closed)
|
Created:
5 years ago by Mostyn Bramley-Moore Modified:
5 years ago CC:
chromium-reviews, feature-media-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptiondon't rely on -Wno-narrowing in skcanvas_video_renderer.cc
Committed: https://crrev.com/0fdc1c19e2ff5f28d696aa181fba59a989c2634c
Cr-Commit-Position: refs/heads/master@{#363947}
Patch Set 1 #Patch Set 2 : avoid signed/unsigned comparison issues #Patch Set 3 : additional DCHECK_LE #Messages
Total messages: 30 (14 generated)
Description was changed from ========== don't rely on -Wno-narrowing in skcanvas_video_renderer.cc BUG= ========== to ========== don't rely on -Wno-narrowing in skcanvas_video_renderer.cc ==========
mostynb@opera.com changed reviewers: + dalecurtis@chromium.org, dcastagna@chromium.org
The CQ bit was checked by mostynb@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1504673003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1504673003/1
@Dale: can you please take a look at this followup to Daniele's CL from earlier this year https://codereview.chromium.org/1153623002 ? I am migrating a downstream project from gyp to gn, and hit narrowing warnings here. Rather than figure out why my gn builds lack -Wno-narrowing, I thought it would be better to fix it.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...) linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by mostynb@opera.com
The CQ bit was unchecked by mostynb@opera.com
The CQ bit was checked by mostynb@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1504673003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1504673003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_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 mostynb@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1504673003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1504673003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The warning applies only to initialization lists, right? We do the same implicit conversion from unsigned to int at the end of NewSkImageFromVideoFrameNative. I'd say to be consistent and add the DCHECK_LE there too, or to just initialize source_textures with three assignments.
> The warning applies only to initialization lists, right? Correct. > We do the same implicit conversion from unsigned to int at the end of > NewSkImageFromVideoFrameNative. > I'd say to be consistent and add the DCHECK_LE there too, or to just initialize > source_textures with three assignments. Added one more DCHECK_LE, since it feels (tiny bit) safer.
The CQ bit was checked by mostynb@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1504673003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1504673003/40001
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by mostynb@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1504673003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1504673003/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== don't rely on -Wno-narrowing in skcanvas_video_renderer.cc ========== to ========== don't rely on -Wno-narrowing in skcanvas_video_renderer.cc Committed: https://crrev.com/0fdc1c19e2ff5f28d696aa181fba59a989c2634c Cr-Commit-Position: refs/heads/master@{#363947} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/0fdc1c19e2ff5f28d696aa181fba59a989c2634c Cr-Commit-Position: refs/heads/master@{#363947} |
