Consolidate Windows bitmap and DC code

ui/native_theme/native_theme_win.cc

Index: ui/native_theme/native_theme_win.cc
diff --git a/ui/native_theme/native_theme_win.cc b/ui/native_theme/native_theme_win.cc
index be5109a7aedfb75af7cee31cdcd2a5cf381670ee..230250ad469bb4b61bba444bb8420e96777ff4a4 100644
--- a/ui/native_theme/native_theme_win.cc
+++ b/ui/native_theme/native_theme_win.cc
@@ -17,13 +17,13 @@
 #include "base/win/scoped_hdc.h"
 #include "base/win/scoped_select_object.h"
 #include "base/win/windows_version.h"
-#include "skia/ext/bitmap_platform_device.h"
 #include "skia/ext/platform_canvas.h"
 #include "skia/ext/skia_utils_win.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "third_party/skia/include/core/SkColorPriv.h"
 #include "third_party/skia/include/core/SkShader.h"
+#include "third_party/skia/include/core/SkSurface.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/color_utils.h"
@@ -292,10 +292,12 @@ void NativeThemeWin::Paint(SkCanvas* canvas,
     }
   }
 
+  skia::ScopedPlatformPaint paint(canvas);
+  HDC surface = paint.GetPlatformSurface();
   if (needs_paint_indirect)
-    PaintIndirect(canvas, part, state, rect, extra);
+    PaintIndirect(surface, part, state, rect, extra);
   else
-    PaintDirect(canvas, part, state, rect, extra);
+    PaintDirect(surface, part, state, rect, extra);
 }
 
 NativeThemeWin::NativeThemeWin()
@@ -361,14 +363,13 @@ void NativeThemeWin::UpdateSystemColors() {
     system_colors_[kSystemColor] = color_utils::GetSysSkColor(kSystemColor);
 }
 
-void NativeThemeWin::PaintDirect(SkCanvas* canvas,
+void NativeThemeWin::PaintDirect(HDC hdc,
                                  Part part,
                                  State state,
                                  const gfx::Rect& rect,
                                  const ExtraParams& extra) const {
-  skia::ScopedPlatformPaint scoped_platform_paint(canvas);
-  HDC hdc = scoped_platform_paint.GetPlatformSurface();
-
+  SkSurface* surface = nullptr;

[Evan Stade, 2016/02/10 21:41:04]

why is this declared out here

[tomhudson, 2016/02/16 17:47:54]

Acknowledged.

+  SkCanvas* canvas = nullptr;
   switch (part) {
     case kCheckbox:
       PaintCheckbox(hdc, part, state, rect, extra.button);
@@ -423,11 +424,14 @@ void NativeThemeWin::PaintDirect(SkCanvas* canvas,
       return;
     case kScrollbarHorizontalTrack:
     case kScrollbarVerticalTrack:
-      PaintScrollbarTrack(canvas, hdc, part, state, rect,
-                          extra.scrollbar_track);
+      PaintScrollbarTrack(hdc, part, state, rect, extra.scrollbar_track);
       return;
-    case kScrollbarCorner:
-      canvas->drawColor(SK_ColorWHITE, SkXfermode::kSrc_Mode);
+    case kScrollbarCorner: {
+      surface = skia::MapPlatformSurface(hdc);
+      canvas = surface ? surface->getCanvas() : nullptr;
+      if (canvas)
+        canvas->drawColor(SK_ColorWHITE, SkXfermode::kSrc_Mode);

[Evan Stade, 2016/02/10 21:41:04]

use a utility fn?

[Peter Kasting, 2016/02/11 00:19:26]

Not necessary; just this would solve your other comments:

    case kScrollbarCorner: {
      SkSurface* surface = skia::MapPlatformSurface(hdc);
      SkCanvas* canvas = surface ? surface->getCanvas() : nullptr;
      if (canvas)
        canvas->drawColor(SK_ColorWHITE, SkXfermode::kSrc_Mode);
      return;
    }

[tomhudson, 2016/02/16 17:47:54]

Done.

+      }

[Evan Stade, 2016/02/10 21:41:04]

this can't be the right formatting

[tomhudson, 2016/02/16 17:47:54]

Ooh, that's completely broken. Good catch.
Reinforces my request for help figuring out *which* tests to run / *how* to run
the right tests to get coverage of this code. Still working with techstop to get
browser tests to work at all on my Windows dev box.

       return;
     case kTabPanelBackground:
       PaintTabPanelBackground(hdc, rect);
@@ -437,7 +441,7 @@ void NativeThemeWin::PaintDirect(SkCanvas* canvas,
       return;
     case kTrackbarThumb:
     case kTrackbarTrack:
-      PaintTrackbar(canvas, hdc, part, state, rect, extra.trackbar);
+      PaintTrackbar(hdc, part, state, rect, extra.trackbar);
       return;
     case kWindowResizeGripper:
       PaintWindowResizeGripper(hdc, rect);
@@ -651,7 +655,7 @@ SkColor NativeThemeWin::GetSystemColor(ColorId color_id) const {
   return GetAuraColor(color_id, this);
 }
 
-void NativeThemeWin::PaintIndirect(SkCanvas* canvas,
+void NativeThemeWin::PaintIndirect(HDC hdc,
                                    Part part,
                                    State state,
                                    const gfx::Rect& rect,
@@ -662,12 +666,12 @@ void NativeThemeWin::PaintIndirect(SkCanvas* canvas,
   //                  keeping a cache of the resulting bitmaps.
 
   // Create an offscreen canvas that is backed by an HDC.
-  skia::RefPtr<skia::BitmapPlatformDevice> device = skia::AdoptRef(
-      skia::BitmapPlatformDevice::Create(
-          rect.width(), rect.height(), false, NULL));
-  DCHECK(device);
-  SkCanvas offscreen_canvas(device.get());
-  DCHECK(skia::SupportsPlatformPaint(&offscreen_canvas));
+
+  HDC offscreen_hdc = skia::CreateOffscreenSurface(rect.width(), rect.height());
+  SkSurface* offscreen_surface = skia::MapPlatformSurface(offscreen_hdc);
+  DCHECK(offscreen_surface);
+  SkCanvas* offscreen_canvas = offscreen_surface->getCanvas();
+  DCHECK(offscreen_canvas);
 
   // Some of the Windows theme drawing operations do not write correct alpha
   // values for fully-opaque pixels; instead the pixels get alpha 0. This is
@@ -677,7 +681,7 @@ void NativeThemeWin::PaintIndirect(SkCanvas* canvas,
   // which pixels get touched by the paint operation. After paint, set any
   // pixels that have alpha 0 to opaque and placeholders to fully-transparent.
   const SkColor placeholder = SkColorSetARGB(1, 0, 0, 0);
-  offscreen_canvas.clear(placeholder);
+  offscreen_canvas->clear(placeholder);
 
   // Offset destination rects to have origin (0,0).
   gfx::Rect adjusted_rect(rect.size());
@@ -695,20 +699,15 @@ void NativeThemeWin::PaintIndirect(SkCanvas* canvas,
     default:
       break;
   }
-  // Draw the theme controls using existing HDC-drawing code.
-  PaintDirect(&offscreen_canvas, part, state, adjusted_rect, adjusted_extra);
 
-  // Copy the pixels to a bitmap that has ref-counted pixel storage, which is
-  // necessary to have when drawing to a SkPicture.
-  const SkBitmap& hdc_bitmap =
-      offscreen_canvas.getDevice()->accessBitmap(false);
-  SkBitmap bitmap;
-  hdc_bitmap.copyTo(&bitmap, kN32_SkColorType);
+  // Draw the theme controls using existing HDC-drawing code.
+  PaintDirect(offscreen_hdc, part, state, adjusted_rect, adjusted_extra);
 
   // Post-process the pixels to fix up the alpha values (see big comment above).
+  SkBitmap offscreen_bitmap = skia::MapPlatformBitmap(offscreen_hdc);
   const SkPMColor placeholder_value = SkPreMultiplyColor(placeholder);
   const int pixel_count = rect.width() * rect.height();
-  SkPMColor* pixels = bitmap.getAddr32(0, 0);
+  SkPMColor* pixels = offscreen_bitmap.getAddr32(0, 0);
   for (int i = 0; i < pixel_count; i++) {
     if (pixels[i] == placeholder_value) {
       // Pixel wasn't touched - make it fully transparent.
@@ -722,8 +721,13 @@ void NativeThemeWin::PaintIndirect(SkCanvas* canvas,
     }
   }
 
-  // Draw the offscreen bitmap to the destination canvas.
-  canvas->drawBitmap(bitmap, rect.x(), rect.y());
+  SkSurface* destination_surface = skia::MapPlatformSurface(hdc);
+  DCHECK(destination_surface);
+  SkCanvas* destination_canvas = destination_surface->getCanvas();
+  DCHECK(destination_canvas);
+  destination_canvas->drawBitmap(offscreen_bitmap, rect.x(), rect.y());
+
+  DeleteDC(offscreen_hdc);
 }
 
 HRESULT NativeThemeWin::GetThemePartSize(ThemeName theme_name,
@@ -1240,7 +1244,6 @@ HRESULT NativeThemeWin::PaintScrollbarThumb(
 }
 
 HRESULT NativeThemeWin::PaintScrollbarTrack(
-    SkCanvas* canvas,
     HDC hdc,
     Part part,
     State state,
@@ -1281,6 +1284,10 @@ HRESULT NativeThemeWin::PaintScrollbarTrack(
       (system_colors_[COLOR_SCROLLBAR] != system_colors_[COLOR_WINDOW])) {
     FillRect(hdc, &rect_win, reinterpret_cast<HBRUSH>(COLOR_SCROLLBAR + 1));
   } else {
+    SkSurface* surface = skia::MapPlatformSurface(hdc);
+    SkCanvas* canvas = surface ? surface->getCanvas() : nullptr;
+    if (!surface || !canvas)

[Peter Kasting, 2016/02/11 00:19:26]

Nit: "if (!canvas)" suffices

[tomhudson, 2016/02/16 17:47:54]

Done.

+      return E_FAIL;
     SkPaint paint;
     RECT align_rect = gfx::Rect(extra.track_x, extra.track_y, extra.track_width,
                                 extra.track_height).ToRECT();
@@ -1326,7 +1333,6 @@ HRESULT NativeThemeWin::PaintSpinButton(
 }
 
 HRESULT NativeThemeWin::PaintTrackbar(
-    SkCanvas* canvas,
     HDC hdc,
     Part part,
     State state,
@@ -1400,6 +1406,10 @@ HRESULT NativeThemeWin::PaintTrackbar(
 
     // If the button is pressed, draw hatching.
     if (extra.classic_state & DFCS_PUSHED) {
+      SkSurface* surface = skia::MapPlatformSurface(hdc);
+      SkCanvas* canvas = surface ? surface->getCanvas() : nullptr;
+      if (!surface || !canvas)

[Peter Kasting, 2016/02/11 00:19:26]

Nit: "if (!canvas)" suffices

[tomhudson, 2016/02/16 17:47:54]

Done.

+        return E_FAIL;
       SkPaint paint;
       SetCheckerboardShader(&paint, rect_win);