SkColorType instead of (deprecated) SkBitmap::Config

cc/resources/ui_resource_bitmap.h

 // Copyright 2013 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.
 
 #ifndef CC_RESOURCES_UI_RESOURCE_BITMAP_H_
 #define CC_RESOURCES_UI_RESOURCE_BITMAP_H_
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "cc/base/cc_export.h"
@@ skipping 25 matching lines @@
 
   gfx::Size GetSize() const { return size_; }
   UIResourceFormat GetFormat() const { return format_; }
   UIResourceWrapMode GetWrapMode() const { return wrap_mode_; }
   void SetWrapMode(UIResourceWrapMode wrap_mode) { wrap_mode_ = wrap_mode; }
   bool GetOpaque() const { return opaque_; }
   void SetOpaque(bool opaque) { opaque_ = opaque; }
 
   // User must ensure that |skbitmap| is immutable.  The SkBitmap Format should
   // be 32-bit RGBA.
   explicit UIResourceBitmap(const SkBitmap& skbitmap);

[danakj, 2014/03/25 16:13:59]

Are there no callers of this constructor anymore then? Can we remove it?

[reed1, 2014/03/25 17:19:41]

still used. e.g. cc/layers/painted_scrollbar_layer.cc:275

+  UIResourceBitmap(int width, int height, bool isOpaque = false);

[danakj, 2014/03/25 16:13:59]

this should be |is_opaque| or just |opaque|. however, default argument values
are forbidden by the style guide:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Default_Arguments

function overloading is also strongly discouraged, so i'm not sold on just
splitting this into 2 constructors with/out the bool. probably all callsites
should be passing a bool.

also, can you pass a const gfx::Size& instead of a width/height separately?

i'll point out now that in cc we don't pass bool literals to methods directly
since it's unclear what they mean, so callsites would end up looking like:

bool is_opaque = false;
UIResourceBitmap(gfx::Size(10, 10), is_opaque);

[reed1, 2014/03/25 17:19:41]

I'll remove the defaultness of the argument.
not sure how "function overloading" meshes with the existing2 constructors (soon
to be 3 or 4).
Done. I see that the exising constructor that takes a pixelref takes a Size
instead of const Size&. I will change it to const& as well.
Agreed, this is better than just passing 'false'.

[danakj, 2014/03/25 17:31:29]

Yeh, I'm not a fan of so many constructors either. Style guide doesn't outright
ban it, it says: "Use overloaded functions (including constructors) only if a
reader looking at a call site can get a good idea of what is happening without
having to first figure out exactly which overload is being called."

When the arguments are very different, call sites should be somewhat clear. But
when one argument is optionally present, it becomes less so, as you are
essentially making a default value.

   UIResourceBitmap(const skia::RefPtr<SkPixelRef>& pixel_ref, gfx::Size size);
   ~UIResourceBitmap();
 
  private:
   friend class AutoLockUIResourceBitmap;
 
   void Create(const skia::RefPtr<SkPixelRef>& pixel_ref,
               gfx::Size size,
               UIResourceFormat format);
 
@@ skipping 10 matching lines @@
   ~AutoLockUIResourceBitmap();
   const uint8_t* GetPixels() const;
 
  private:
   const UIResourceBitmap& bitmap_;
 };
 
 }  // namespace cc
 
 #endif  // CC_RESOURCES_UI_RESOURCE_BITMAP_H_