Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2629913002:
fix leak of hbitmap (Closed)
Descriptionfix leak of hbitmap
use SaveDC/RestoreDC to track the original/default hbitmap in the DC. This way we can recover the actual hbitmap using GetCurrentObject, use RestoreDC to delect it and restore the original, and then delete our hbitmap and the dc.
BUG=680971
Review-Url: https://codereview.chromium.org/2629913002
Cr-Commit-Position: refs/heads/master@{#443583}
Committed: https://chromium.googlesource.com/chromium/src/+/1e4883b93bc28febedbf8f3927c88daebf0acbc6
Patch Set 1 #
Total comments: 10
Patch Set 2 : fix dchecks #Messages
Total messages: 18 (12 generated)
reed@google.com changed reviewers: + fmalita@chromium.org
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 ========== fix leak of hbitmap BUG= ========== to ========== fix leak of hbitmap use SaveDC/RestoreDC to track the original/default hbitmap in the DC. This way we can recover the actual hbitmap using GetCurrentObject, use RestoreDC to delect it and restore the original, and then delete our hbitmap and the dc. BUG= ==========
lgtm https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... File skia/ext/raster_handle_allocator_win.cc (right): https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... skia/ext/raster_handle_allocator_win.cc:28: DCHECK(hbitmap); nit: DCHECK_NE(hbitmap, nullptr) https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... skia/ext/raster_handle_allocator_win.cc:30: int success = RestoreDC(hdc, -1); // effectly performs the deselect of hbitmap nit: bool https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... skia/ext/raster_handle_allocator_win.cc:35: DCHECK(hbitmap); DCHECK_NE https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... skia/ext/raster_handle_allocator_win.cc:37: DCHECK(hbitmap); DCHECK_NE
Description was changed from ========== fix leak of hbitmap use SaveDC/RestoreDC to track the original/default hbitmap in the DC. This way we can recover the actual hbitmap using GetCurrentObject, use RestoreDC to delect it and restore the original, and then delete our hbitmap and the dc. BUG= ========== to ========== fix leak of hbitmap use SaveDC/RestoreDC to track the original/default hbitmap in the DC. This way we can recover the actual hbitmap using GetCurrentObject, use RestoreDC to delect it and restore the original, and then delete our hbitmap and the dc. BUG=680971 ==========
https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... File skia/ext/raster_handle_allocator_win.cc (right): https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... skia/ext/raster_handle_allocator_win.cc:68: DCHECK(saveIndex != 0); DCHECK_NE(saveIndex, 0)
https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... File skia/ext/raster_handle_allocator_win.cc (right): https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... skia/ext/raster_handle_allocator_win.cc:28: DCHECK(hbitmap); On 2017/01/13 15:51:57, f(malita) wrote: > nit: DCHECK_NE(hbitmap, nullptr) Done. https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... skia/ext/raster_handle_allocator_win.cc:30: int success = RestoreDC(hdc, -1); // effectly performs the deselect of hbitmap On 2017/01/13 15:51:57, f(malita) wrote: > nit: bool Done. https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... skia/ext/raster_handle_allocator_win.cc:35: DCHECK(hbitmap); On 2017/01/13 15:51:57, f(malita) wrote: > DCHECK_NE Done. https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... skia/ext/raster_handle_allocator_win.cc:37: DCHECK(hbitmap); On 2017/01/13 15:51:57, f(malita) wrote: > DCHECK_NE Done. https://codereview.chromium.org/2629913002/diff/1/skia/ext/raster_handle_allo... skia/ext/raster_handle_allocator_win.cc:68: DCHECK(saveIndex != 0); On 2017/01/13 15:52:44, f(malita) wrote: > DCHECK_NE(saveIndex, 0) Done.
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...
The CQ bit was unchecked by fmalita@chromium.org
The CQ bit was checked by fmalita@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from fmalita@chromium.org Link to the patchset: https://codereview.chromium.org/2629913002/#ps20001 (title: "fix dchecks")
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": 1484323242867740,
"parent_rev": "07db4ac1a48f0ebb1ff520d8ae07a550f326a399", "commit_rev":
"1e4883b93bc28febedbf8f3927c88daebf0acbc6"}
Message was sent while issue was closed.
Description was changed from ========== fix leak of hbitmap use SaveDC/RestoreDC to track the original/default hbitmap in the DC. This way we can recover the actual hbitmap using GetCurrentObject, use RestoreDC to delect it and restore the original, and then delete our hbitmap and the dc. BUG=680971 ========== to ========== fix leak of hbitmap use SaveDC/RestoreDC to track the original/default hbitmap in the DC. This way we can recover the actual hbitmap using GetCurrentObject, use RestoreDC to delect it and restore the original, and then delete our hbitmap and the dc. BUG=680971 Review-Url: https://codereview.chromium.org/2629913002 Cr-Commit-Position: refs/heads/master@{#443583} Committed: https://chromium.googlesource.com/chromium/src/+/1e4883b93bc28febedbf8f3927c8... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/1e4883b93bc28febedbf8f3927c8... |
