Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2770493004:
Remove disable warnings compile flag in safebrowsing component, and fix some type win64 type errors… (Closed)
DescriptionRemove disable warnings compile flag in safebrowsing component, and fix some type win64 type errors that were found.
BUG=700351, 702773
Review-Url: https://codereview.chromium.org/2770493004
Cr-Commit-Position: refs/heads/master@{#459425}
Committed: https://chromium.googlesource.com/chromium/src/+/ac4f2ee6eef06c8c345eed0a01bf64ab3df642eb
Patch Set 1 #
Total comments: 2
Patch Set 2 : Sync #
Messages
Total messages: 26 (17 generated)
The CQ bit was checked by lpz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lpz@chromium.org changed reviewers: + nparker@chromium.org, timvolodine@chromium.org
Tim: I duped your cl 2764883002 just to confirm that it fixed your error. Let me know if you object to submitting that change as part of this CL.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2770493004/diff/1/components/safe_browsing/re... File components/safe_browsing/renderer/threat_dom_details.cc (right): https://codereview.chromium.org/2770493004/diff/1/components/safe_browsing/re... components/safe_browsing/renderer/threat_dom_details.cc:174: const int child_id = static_cast<int>(element_to_node_map->size()) + 1; Why cast to int? I'm not confident I know which is safer, since an overflow here would turn negative, which may have bad effects.
https://codereview.chromium.org/2770493004/diff/1/components/safe_browsing/re... File components/safe_browsing/renderer/threat_dom_details.cc (right): https://codereview.chromium.org/2770493004/diff/1/components/safe_browsing/re... components/safe_browsing/renderer/threat_dom_details.cc:174: const int child_id = static_cast<int>(element_to_node_map->size()) + 1; On 2017/03/22 21:12:56, Nathan Parker wrote: > Why cast to int? I'm not confident I know which is safer, since an overflow > here would turn negative, which may have bad effects. The alternative would be to keep size_t here but then the IPC message fields will have to be changed to size_t, which is a blacklisted IPC type apparently. I could cast to ints before setting the IPC fields, but seems easier to do it here. Note also that the size of this map is capped by kMaxNodes, which is currently at 500, so at least some control from overflows.
lgtm Got it, make sense.
The CQ bit was checked by lpz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nparker@chromium.org Link to the patchset: https://codereview.chromium.org/2770493004/#ps20001 (title: "Sync")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by lpz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
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 lpz@chromium.org
The CQ bit was unchecked by lpz@chromium.org
On 2017/03/22 17:26:11, lpz wrote: > Tim: I duped your cl 2764883002 just to confirm that it fixed your error. Let me > know if you object to submitting that change as part of this CL. fine with me, thanks for the fix! lgtm
The CQ bit was checked by lpz@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490367832241500,
"parent_rev": "34e20090704e62dbb27e402b116e5a0259c385ee", "commit_rev":
"ac4f2ee6eef06c8c345eed0a01bf64ab3df642eb"}
Message was sent while issue was closed.
Description was changed from ========== Remove disable warnings compile flag in safebrowsing component, and fix some type win64 type errors that were found. BUG=700351,702773 ========== to ========== Remove disable warnings compile flag in safebrowsing component, and fix some type win64 type errors that were found. BUG=700351,702773 Review-Url: https://codereview.chromium.org/2770493004 Cr-Commit-Position: refs/heads/master@{#459425} Committed: https://chromium.googlesource.com/chromium/src/+/ac4f2ee6eef06c8c345eed0a01bf... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/ac4f2ee6eef06c8c345eed0a01bf... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
