don't rely on save/restore for the selected bitmap

skia/ext/raster_handle_allocator_win.cc

 // Copyright (c) 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <windows.h>
 #include <psapi.h>
 #include <stddef.h>
 
 #include "base/debug/gdi_debug_util_win.h"
 #include "base/memory/ptr_util.h"
 #include "base/logging.h"
 #include "base/win/win_util.h"
 #include "skia/ext/platform_canvas.h"
 #include "skia/ext/skia_utils_win.h"
 #include "third_party/skia/include/core/SkMatrix.h"
 #include "third_party/skia/include/core/SkPath.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 #include "third_party/skia/include/core/SkRect.h"
 
 namespace {
 
+struct HDCContextRec {
+  HDC fHDC;
+  HBITMAP fOldBitmap;  // we need to restore this to the HDC
+  HBITMAP fNewBitmap;  // we need to delete this

[bungeman-chromium, 2017/03/06 16:30:27]

nit: THIS IS SPARTA!!!!

all the other fields in this directory follow the Chromium naming convention of
important_field_

[reed1, 2017/03/06 16:41:51]

Done.

+};
+
 static void DeleteHDCCallback(void*, void* context) {
   DCHECK_NE(context, nullptr);
-  HDC hdc = static_cast<HDC>(context);
+  const HDCContextRec* rec = static_cast<const HDCContextRec*>(context);
 
-  // We must get this before we call RestoreDC, as that will drop hbitmap and
-  // reselect the original/default bitmap.
-  HGDIOBJ hbitmap = GetCurrentObject(hdc, OBJ_BITMAP);
-  DCHECK_NE(hbitmap, nullptr);
-
-  bool success = RestoreDC(hdc, -1); // effectly performs the deselect of hbitmap
+  // must first select back in the old bitmap, so that fNewBitmap will be
+  // unowned.
+  SelectObject(rec->fHDC, rec->fOldBitmap);
+  bool success = DeleteObject(rec->fNewBitmap);

[bungeman-chromium, 2017/03/06 16:30:27]

nit: maybe DCHECK that the old oblect is 'NewBitmap'

[reed1, 2017/03/06 16:41:51]

will delete what is returned

   DCHECK(success);
-
-  // Now we are the only owner, so we can delete our bitmap
-  success = DeleteObject(hbitmap);
+  success = DeleteDC(rec->fHDC);
   DCHECK(success);
-  success = DeleteDC(hdc);
-  DCHECK(success);
+  delete rec;
 }
 
 // Allocate the layer and fill in the fields for the Rec, or return false
 // on error.
 static bool Create(int width,
                    int height,
                    bool is_opaque,
                    HANDLE shared_section,
                    bool do_clear,
                    SkRasterHandleAllocator::Rec* rec) {
   void* pixels;
-  HBITMAP hbitmap =
+  HBITMAP newBitmap =
       skia::CreateHBitmap(width, height, is_opaque, shared_section, &pixels);
-  if (!hbitmap) {
+  if (!newBitmap) {
     LOG(ERROR) << "CreateHBitmap failed";
     return false;
   }
 
   size_t row_bytes = skia::PlatformCanvasStrideForWidth(width);
   if (do_clear)
     sk_bzero(pixels, row_bytes * height);
 
   HDC hdc = CreateCompatibleDC(nullptr);
   if (!hdc) {
-    DeleteObject(hbitmap);
+    DeleteObject(newBitmap);
     return false;
   }
   SetGraphicsMode(hdc, GM_ADVANCED);
 
-  int saveIndex = SaveDC(hdc);  // so we can Restore in the delete callback
-  DCHECK_NE(saveIndex, 0);
-
-  // Be sure to select *after* we called SaveDC.
-  // Because we're using save/restore, we don't need to explicitly track the
-  // returned "previous" value.
-  SelectObject(hdc, hbitmap);
+  HBITMAP oldBitmap = static_cast<HBITMAP>(SelectObject(hdc, newBitmap));
+  DCHECK(oldBitmap);
 
   rec->fReleaseProc = DeleteHDCCallback;
-  rec->fReleaseCtx = hdc;
+  rec->fReleaseCtx = new HDCContextRec{hdc, oldBitmap, newBitmap};
   rec->fPixels = pixels;
   rec->fRowBytes = row_bytes;
   rec->fHandle = hdc;
   return true;
 }
 
 /**
  *  Subclass of SkRasterHandleAllocator that returns an HDC as its "handle".
  */
 class GDIAllocator : public SkRasterHandleAllocator {
@@ skipping 65 matching lines @@
   if (failure_type == CRASH_ON_FAILURE)
     SK_CRASH();
   return nullptr;
 }
 
 HDC GetNativeDrawingContext(SkCanvas* canvas) {
   return canvas ? static_cast<HDC>(canvas->accessTopRasterHandle()) : nullptr;
 }
 
 }  // namespace skia