Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2919153002:
Avoid crash in DocumentProperties (Closed)
DescriptionAvoid crash in DocumentProperties
Some printers crash in DocumentProperties, particularly Dell Win7
drivers running on Win10. These printers have invalid settings as well
and report no page sizes or DPIs, so use the lack of page sizes to
detect them and report an error before calling DocumentProperties.
BUG=724595
Review-Url: https://codereview.chromium.org/2919153002
Cr-Commit-Position: refs/heads/master@{#477102}
Committed: https://chromium.googlesource.com/chromium/src/+/773f81b206a0e30c9742428389af3e1eeec90997
Patch Set 1 #
Total comments: 6
Patch Set 2 : Address comments #Messages
Total messages: 25 (20 generated)
Description was changed from ========== Avoid crash in DocumentProperties Some printers crash in DocumentProperties, particularly Dell Win7 drivers running on Win10. These printers have invalid settings as well and report no page sizes or DPIs, so use the lack of page sizes to detect them and report an error before calling DocumentProperties. BUG=724595 ========== to ========== Avoid crash in DocumentProperties Some printers crash in DocumentProperties, particularly Dell Win7 drivers running on Win10. These printers have invalid settings as well and report no page sizes or DPIs, so use the lack of page sizes to detect them and report an error before calling DocumentProperties. BUG=724595 ==========
rbpotter@chromium.org changed reviewers: + thestig@chromium.org
The CQ bit was checked by rbpotter@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...
lgtm https://codereview.chromium.org/2919153002/diff/1/printing/backend/win_helper.cc File printing/backend/win_helper.cc (right): https://codereview.chromium.org/2919153002/diff/1/printing/backend/win_helper... printing/backend/win_helper.cc:486: return DeviceCapabilities(name, port, DC_PAPERSIZE, NULL, NULL) > 0; nit: Can we write nullptr in new code? https://codereview.chromium.org/2919153002/diff/1/printing/backend/win_helper... printing/backend/win_helper.cc:506: return std::unique_ptr<DEVMODE, base::FreeDeleter>(); I think you can just return nullptr. If so, maybe convert all the returns below too? https://codereview.chromium.org/2919153002/diff/1/printing/backend/win_helper... printing/backend/win_helper.cc:513: // used with Win10. See crbug.com/679160 Append "Also ..." to the existing comment and reference the new bug, or just do these as two separate checks.
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 rbpotter@chromium.org to run a CQ dry run
https://codereview.chromium.org/2919153002/diff/1/printing/backend/win_helper.cc File printing/backend/win_helper.cc (right): https://codereview.chromium.org/2919153002/diff/1/printing/backend/win_helper... printing/backend/win_helper.cc:486: return DeviceCapabilities(name, port, DC_PAPERSIZE, NULL, NULL) > 0; On 2017/06/02 21:29:54, Lei Zhang wrote: > nit: Can we write nullptr in new code? Done. https://codereview.chromium.org/2919153002/diff/1/printing/backend/win_helper... printing/backend/win_helper.cc:506: return std::unique_ptr<DEVMODE, base::FreeDeleter>(); On 2017/06/02 21:29:54, Lei Zhang wrote: > I think you can just return nullptr. If so, maybe convert all the returns below > too? Done. https://codereview.chromium.org/2919153002/diff/1/printing/backend/win_helper... printing/backend/win_helper.cc:513: // used with Win10. See crbug.com/679160 On 2017/06/02 21:29:54, Lei Zhang wrote: > Append "Also ..." to the existing comment and reference the new bug, or just do > these as two separate checks. Done.
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: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by rbpotter@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 rbpotter@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org Link to the patchset: https://codereview.chromium.org/2919153002/#ps20001 (title: "Address comments")
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": 1496701721000360,
"parent_rev": "e487923b15ab54c9dd10e93ba51a9ce0b95fecee", "commit_rev":
"d606d45f3c3bd0a44f6fd4bb6d49f7a98e22f7d1"}
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496701721000360,
"parent_rev": "0f98b21f2bf448e683c66414f79cb5a8b0786c82", "commit_rev":
"7baf5dc6c4ad8c1dc2f53b2c4cfac8e845822232"}
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496701721000360,
"parent_rev": "95d0bac32216dc3d6d4e92df16ef439049e14579", "commit_rev":
"773f81b206a0e30c9742428389af3e1eeec90997"}
Message was sent while issue was closed.
Description was changed from ========== Avoid crash in DocumentProperties Some printers crash in DocumentProperties, particularly Dell Win7 drivers running on Win10. These printers have invalid settings as well and report no page sizes or DPIs, so use the lack of page sizes to detect them and report an error before calling DocumentProperties. BUG=724595 ========== to ========== Avoid crash in DocumentProperties Some printers crash in DocumentProperties, particularly Dell Win7 drivers running on Win10. These printers have invalid settings as well and report no page sizes or DPIs, so use the lack of page sizes to detect them and report an error before calling DocumentProperties. BUG=724595 Review-Url: https://codereview.chromium.org/2919153002 Cr-Commit-Position: refs/heads/master@{#477102} Committed: https://chromium.googlesource.com/chromium/src/+/773f81b206a0e30c9742428389af... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/773f81b206a0e30c9742428389af... |
