Cleanup gfx::Canvas now that 10562027 has landed

ui/gfx/canvas.cc

Index: ui/gfx/canvas.cc
diff --git a/ui/gfx/canvas.cc b/ui/gfx/canvas.cc
index 26f2e301a4d1b8affb7cc7bf76614e1761c78b14..f993203e43422fbd0de44efac27c6cb1d30b2a72 100644
--- a/ui/gfx/canvas.cc
+++ b/ui/gfx/canvas.cc
@@ -22,23 +22,10 @@
 
 namespace gfx {
 
-Canvas::Canvas(const gfx::Size& size, bool is_opaque)
-    : owned_canvas_(new skia::PlatformCanvas(size.width(), size.height(),
-                                             is_opaque)),
-      canvas_(owned_canvas_.get()) {
-#if defined(OS_WIN) || defined(OS_MACOSX)
-  // skia::PlatformCanvas instances are initialized to 0 by Cairo on Linux, but
-  // uninitialized on Win and Mac.
-  if (!is_opaque)
-    owned_canvas_->clear(SkColorSetARGB(0, 0, 0, 0));
-#endif
-
-  ApplyScaleFactor(ui::SCALE_FACTOR_100P, false);
-}
-
 Canvas::Canvas(const gfx::Size& size,
                ui::ScaleFactor scale_factor,
-               bool is_opaque) {
+               bool is_opaque)
+      : scale_factor_(scale_factor) {
   gfx::Size pixel_size = size.Scale(ui::GetScaleFactorScale(scale_factor));

[sky, 2012/07/16 14:26:51]

Member initialize canvas_ to NULL.

[pkotwicz, 2012/07/17 17:19:52]

canvas_ is initialized in line 30 as part of the constructor.
Do you want me to initialize it in the initializer list too?

[sky, 2012/07/17 21:58:32]

Yes.

   owned_canvas_.reset(new skia::PlatformCanvas(pixel_size.width(),
                                                pixel_size.height(),
@@ -51,50 +38,55 @@ Canvas::Canvas(const gfx::Size& size,
     owned_canvas_->clear(SkColorSetARGB(0, 0, 0, 0));
 #endif
 
-  ApplyScaleFactor(scale_factor, true);
+  scale_factor_ = scale_factor;

[sky, 2012/07/16 14:26:51]

Doesn't line 28 set the scale_factor_?

[pkotwicz, 2012/07/17 17:19:52]

Done.

+  SkScalar scale = SkFloatToScalar(ui::GetScaleFactorScale(scale_factor));
+  canvas_->scale(scale, scale);
 }
 
 Canvas::Canvas(const gfx::ImageSkiaRep& image_rep, bool is_opaque)
-    : owned_canvas_(new skia::PlatformCanvas(image_rep.pixel_width(),
+    : scale_factor_(image_rep.scale_factor()),
+      owned_canvas_(new skia::PlatformCanvas(image_rep.pixel_width(),
                                              image_rep.pixel_height(),
                                              is_opaque)),
       canvas_(owned_canvas_.get()) {
-  ApplyScaleFactor(image_rep.scale_factor(), true);
+  SkScalar scale = SkFloatToScalar(ui::GetScaleFactorScale(scale_factor_));
+  canvas_->scale(scale, scale);
   DrawImageInt(gfx::ImageSkia(image_rep), 0, 0);
 }
 
 Canvas::Canvas()
-    : owned_canvas_(new skia::PlatformCanvas()),
+    : scale_factor_(ui::SCALE_FACTOR_100P),
+      owned_canvas_(new skia::PlatformCanvas()),
       canvas_(owned_canvas_.get()) {
-  ApplyScaleFactor(ui::SCALE_FACTOR_100P, false);
 }
 
-Canvas::Canvas(SkCanvas* canvas,
-               ui::ScaleFactor scale_factor,
-               bool scale_canvas)
-    : owned_canvas_(),
+Canvas::Canvas(SkCanvas* canvas, ui::ScaleFactor scale_factor)

[sky, 2012/07/16 14:26:51]

Move to match position in header.

[pkotwicz, 2012/07/17 17:19:52]

Done.

+    : scale_factor_(scale_factor),
+      owned_canvas_(),
       canvas_(canvas) {
   DCHECK(canvas);
-  ApplyScaleFactor(scale_factor, scale_canvas);
 }
 
 Canvas::~Canvas() {
-  if (scale_factor_scales_canvas_) {
-    SkScalar scale = 1.0f / ui::GetScaleFactorScale(scale_factor_);
-    canvas_->scale(scale, scale);
-  }
+}
+
+// static
+Canvas* Canvas::CreateCanvasWithoutScaling(SkCanvas* canvas,
+                                          ui::ScaleFactor scale_factor) {

[sky, 2012/07/16 14:26:51]

nit: spacing

[pkotwicz, 2012/07/17 17:19:52]

Done.

+  return new Canvas(canvas, scale_factor);
 }
 
 void Canvas::RecreateBackingCanvas(const gfx::Size& size,
                                    ui::ScaleFactor scale_factor,
                                    bool is_opaque) {
+  scale_factor_ = scale_factor;
   gfx::Size pixel_size = size.Scale(ui::GetScaleFactorScale(scale_factor));
   owned_canvas_.reset(new skia::PlatformCanvas(pixel_size.width(),
                                                pixel_size.height(),
                                                is_opaque));
   canvas_ = owned_canvas_.get();
-
-  ApplyScaleFactor(scale_factor, true);
+  SkScalar scale = SkFloatToScalar(ui::GetScaleFactorScale(scale_factor_));
+  canvas_->scale(scale, scale);
 }
 
 // static
@@ -109,7 +101,7 @@ int Canvas::DefaultCanvasTextAlignment() {
   return base::i18n::IsRTL() ? TEXT_ALIGN_RIGHT : TEXT_ALIGN_LEFT;
 }
 
-SkBitmap Canvas::ExtractBitmap() const {
+gfx::ImageSkiaRep Canvas::ExtractImageRep() const {
   const SkBitmap& device_bitmap = canvas_->getDevice()->accessBitmap(false);
 
   // Make a bitmap to return, and a canvas to draw into it. We don't just want
@@ -117,11 +109,8 @@ SkBitmap Canvas::ExtractBitmap() const {
   // of the bitmap.
   SkBitmap result;
   device_bitmap.copyTo(&result, SkBitmap::kARGB_8888_Config);
-  return result;
-}
 
-gfx::ImageSkiaRep Canvas::ExtractImageSkiaRep() const {
-  return gfx::ImageSkiaRep(ExtractBitmap(), scale_factor_);
+  return gfx::ImageSkiaRep(result, scale_factor_);
 }
 
 void Canvas::DrawDashedRect(const gfx::Rect& rect, SkColor color) {
@@ -513,16 +502,6 @@ bool Canvas::IntersectsClipRect(const gfx::Rect& rect) {
                                rect.width(), rect.height());
 }
 
-void Canvas::ApplyScaleFactor(ui::ScaleFactor scale_factor,
-                              bool scale_canvas) {
-  scale_factor_scales_canvas_ = scale_canvas;
-  scale_factor_ = scale_factor;
-  if (scale_canvas) {
-    SkScalar scale = SkFloatToScalar(ui::GetScaleFactorScale(scale_factor));
-    canvas_->scale(scale, scale);
-  }
-}
-
 const gfx::ImageSkiaRep& Canvas::GetImageRepToPaint(
     const gfx::ImageSkia& image) const {
   return GetImageRepToPaint(image, 1.0f, 1.0f);