Make scoped dc objects smarter.

base/win/scoped_hdc.h

Index: base/win/scoped_hdc.h
===================================================================
--- base/win/scoped_hdc.h	(revision 120644)
+++ base/win/scoped_hdc.h	(working copy)
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -9,70 +9,169 @@
 #include <windows.h>
 
 #include "base/basictypes.h"
+#include "base/compiler_specific.h"
 #include "base/logging.h"
 
 namespace base {
 namespace win {
 
-// Like ScopedHandle but for HDC.  Only use this on HDCs returned from
-// GetDC.
-class ScopedGetDC {
+// The ScopedGetDC and ScopedCreateDC classes manage the default GDI objects
+// that are initially selected into a DC. They help you avoid the following
+// common mistake:
+//
+// HDC hdc = GetDC(NULL);
+// SelectObject(hdc, new_bitmap);
+// .. drawing code here ..
+// ReleaseDC(hdc);   <--- error: the DC has a custom object still selected!
+//
+// This code should be:
+//
+// HDC hdc = GetDC(NULL);
+// HGDIOBJ old_obj = SelectObject(hdc, new_bitmap);
+// .. drawing code here ..
+// SelectObject(hdc, old_obj);
+// ReleaseDC(hdc);    <--- ok to release now.
+//
+// But why work so hard? Use our handy classes:
+//
+// ScopedGetDC dc(NULL);
+// dc.SelectBitmap(hdc, new_bitmap);
+// .. drawing here
+// .. when dc goes out of scope it will select the original object before
+// .. being released.
+//
+class ScopedDC {
  public:
-  explicit ScopedGetDC(HWND hwnd)
-      : hwnd_(hwnd),
-        hdc_(GetDC(hwnd)) {
-    DCHECK(!hwnd_ || IsWindow(hwnd_));
-    DCHECK(hdc_);
+  virtual ~ScopedDC() {}
+
+  virtual void DisposeDC(HDC hdc) = 0;
+
+  HDC Get() {

[Peter Kasting, 2012/02/08 00:38:49]

Nit: Seems like this should be "get()" since it's inline and cheap.

[cpu_(ooo_6.6-7.5), 2012/02/10 19:41:55]

Done.

+    return hdc_;
   }
 
-  ~ScopedGetDC() {
-    if (hdc_)
-      ReleaseDC(hwnd_, hdc_);
+  void SelectBitmap(HBITMAP bitmap) {

[Peter Kasting, 2012/02/08 00:38:49]

Nit: Can we move these and the other non-unix_hacker()-style function bodies to
a .cc file rather than having them inlined in the header?

+    Select(bitmap, &bitmap_);
   }
 
-  operator HDC() { return hdc_; }
+  void SelectFont(HFONT font) {
+    Select(font, &font_);
+  }
 
+  void SelectBrush(HBRUSH brush) {
+    Select(brush, &brush_);
+  }
+
+  void SelectPen(HPEN pen) {
+    Select(pen, &pen_);
+  }
+
+  void SelectRegion(HRGN region) {
+    Select(region, &region_);
+  }
+
+ protected:
+  ScopedDC(HDC hdc)
+    : hdc_(hdc), bitmap_(0), font_(0), brush_(0), pen_(0), region_(0) {

[Peter Kasting, 2012/02/08 00:38:49]

Nit: Indent 4 (4 places); 1 member per line since the entire definition isn't
one line

[cpu_(ooo_6.6-7.5), 2012/02/10 19:41:55]

Done.

+  }
+
+  void Close() {
+    if (!hdc_)
+      return;
+    ResetObjects();
+    DisposeDC(hdc_);
+  }
+
+  void Reset(HDC hdc) {
+    Close();
+    hdc_ = hdc;
+  }
+
  private:
-  HWND hwnd_;
+  void ResetObjects() {
+    if (bitmap_) {
+      SelectObject(hdc_, bitmap_);
+      bitmap_ = 0;
+    }
+    if (font_) {
+      SelectObject(hdc_, font_);
+      font_ = 0;
+    }
+    if (brush_) {
+      SelectObject(hdc_, brush_);
+      brush_ = 0;
+    }
+    if (pen_) {
+      SelectObject(hdc_, pen_);
+      pen_ = 0;
+    }
+    if (region_) {
+      SelectObject(hdc_, region_);
+      region_ = 0;
+    }
+  }
+
+  void Select(HGDIOBJ object, HGDIOBJ* holder) {
+    HGDIOBJ old = SelectObject(hdc_, object);
+    DCHECK(old);
+    // We only want to store the first |old| object.
+    if (!*holder)
+      *holder = old;
+  }
+
   HDC hdc_;
+  HGDIOBJ bitmap_;
+  HGDIOBJ font_;
+  HGDIOBJ brush_;
+  HGDIOBJ pen_;
+  HGDIOBJ region_;
+};
 
+// Creates and manages a HDC obtained by GetDC.

[Derek Bruening, 2012/02/08 15:13:19]

nit: "an HDC" (unless you pronounce it "huhd-kuh" or something instead of saying
the letters?)

[cpu_(ooo_6.6-7.5), 2012/02/10 19:41:55]

Done.

+class ScopedGetDC : public ScopedDC  {
+ public:
+  explicit ScopedGetDC(HWND hwnd)
+    : ScopedDC(GetDC(hwnd)), hwnd_(hwnd) {
+  }
+
+  virtual ~ScopedGetDC() {
+    Close();
+  }
+
+ private:
+  virtual void DisposeDC(HDC hdc) OVERRIDE {
+    ReleaseDC(hwnd_, hdc);
+  }
+
+  HWND hwnd_;
   DISALLOW_COPY_AND_ASSIGN(ScopedGetDC);
 };
 
 // Like ScopedHandle but for HDC.  Only use this on HDCs returned from
 // CreateCompatibleDC, CreateDC and CreateIC.
-class ScopedCreateDC {
+class ScopedCreateDC : public ScopedDC {
  public:
-  ScopedCreateDC() : hdc_(NULL) { }
-  explicit ScopedCreateDC(HDC h) : hdc_(h) { }
-
-  ~ScopedCreateDC() {
-    Close();
+  ScopedCreateDC()
+    : ScopedDC(0) {
   }
 
-  HDC Get() {
-    return hdc_;
+  explicit ScopedCreateDC(HDC hdc)
+    : ScopedDC(hdc) {
   }
 
-  void Set(HDC h) {
+  virtual ~ScopedCreateDC() {
     Close();
-    hdc_ = h;
   }
 
-  operator HDC() { return hdc_; }
+  void Set(HDC hdc) {
+    Reset(hdc);
+  }
 
  private:
-  void Close() {
-#ifdef NOGDI
-    assert(false);
-#else
-    if (hdc_)
-      DeleteDC(hdc_);
-#endif  // NOGDI
+  virtual void DisposeDC(HDC hdc) OVERRIDE {
+    DeleteDC(hdc);
   }
 
-  HDC hdc_;
-
   DISALLOW_COPY_AND_ASSIGN(ScopedCreateDC);
 };