Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2736743002:
don't rely on save/restore for the selected bitmap (Closed)
|
Created:
3 years, 9 months ago by reed1 Modified:
3 years, 9 months ago Reviewers:
bungeman-chromium CC:
chromium-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptiondon't rely on save/restore for the selected bitmap
speculative fix, on the guess that save/restore may not always work... This does not knowingly change the behavior from before.
BUG=697396
Review-Url: https://codereview.chromium.org/2736743002
Cr-Commit-Position: refs/heads/master@{#454887}
Committed: https://chromium.googlesource.com/chromium/src/+/60b11593548d66fc8d661b40f140d46ca9e76d8f
Patch Set 1 #Patch Set 2 : make it compile #
Total comments: 4
Patch Set 3 : no need to store new bitmap #Messages
Total messages: 19 (13 generated)
The CQ bit was checked by reed@google.com to run a CQ dry run
reed@google.com changed reviewers: + bungeman@chromium.org
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 checked by reed@google.com 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/2736743002/diff/20001/skia/ext/raster_handle_... File skia/ext/raster_handle_allocator_win.cc (right): https://codereview.chromium.org/2736743002/diff/20001/skia/ext/raster_handle_... skia/ext/raster_handle_allocator_win.cc:25: HBITMAP fNewBitmap; // we need to delete this nit: THIS IS SPARTA!!!! all the other fields in this directory follow the Chromium naming convention of important_field_ https://codereview.chromium.org/2736743002/diff/20001/skia/ext/raster_handle_... skia/ext/raster_handle_allocator_win.cc:35: bool success = DeleteObject(rec->fNewBitmap); nit: maybe DCHECK that the old oblect is 'NewBitmap'
https://codereview.chromium.org/2736743002/diff/20001/skia/ext/raster_handle_... File skia/ext/raster_handle_allocator_win.cc (right): https://codereview.chromium.org/2736743002/diff/20001/skia/ext/raster_handle_... skia/ext/raster_handle_allocator_win.cc:25: HBITMAP fNewBitmap; // we need to delete this On 2017/03/06 16:30:27, bungeman-chromium wrote: > nit: THIS IS SPARTA!!!! > > all the other fields in this directory follow the Chromium naming convention of > important_field_ Done. https://codereview.chromium.org/2736743002/diff/20001/skia/ext/raster_handle_... skia/ext/raster_handle_allocator_win.cc:35: bool success = DeleteObject(rec->fNewBitmap); On 2017/03/06 16:30:27, bungeman-chromium wrote: > nit: maybe DCHECK that the old oblect is 'NewBitmap' will delete what is returned
The CQ bit was checked by reed@google.com 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...
Description was changed from ========== don't rely on save/restore for the selected bitmap BUG=697396 ========== to ========== don't rely on save/restore for the selected bitmap speculative fix, on the guess that save/restore may not always work... This does not knowingly change the behavior from before. BUG=697396 ==========
The CQ bit was unchecked by reed@google.com
The CQ bit was checked by reed@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from bungeman@chromium.org Link to the patchset: https://codereview.chromium.org/2736743002/#ps40001 (title: "no need to store new bitmap")
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": 40001, "attempt_start_ts": 1488822412698220,
"parent_rev": "c1b9a334b148b22b8fe0a158f1adff10db126574", "commit_rev":
"60b11593548d66fc8d661b40f140d46ca9e76d8f"}
Message was sent while issue was closed.
Description was changed from ========== don't rely on save/restore for the selected bitmap speculative fix, on the guess that save/restore may not always work... This does not knowingly change the behavior from before. BUG=697396 ========== to ========== don't rely on save/restore for the selected bitmap speculative fix, on the guess that save/restore may not always work... This does not knowingly change the behavior from before. BUG=697396 Review-Url: https://codereview.chromium.org/2736743002 Cr-Commit-Position: refs/heads/master@{#454887} Committed: https://chromium.googlesource.com/chromium/src/+/60b11593548d66fc8d661b40f140... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/60b11593548d66fc8d661b40f140... |
