C++ readability review for Robert Sesek <rsesek@google.com>.

R=smo
OCL=http://codereview.chromium.org/6312159/
BUG=http://b/3477621
TEST=none

Review URL: http://codereview.chromium.org/6538096

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@79824 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 44 insertions, 52 deletions

diff --git a/chrome/browser/ui/tests/ui_gfx_image_unittest.cc b/chrome/browser/ui/tests/ui_gfx_image_unittest.cc
index c710c18..cfe1c9b 100644
--- a/chrome/browser/ui/tests/ui_gfx_image_unittest.cc
+++ b/chrome/browser/ui/tests/ui_gfx_image_unittest.cc
@@ -20,11 +20,9 @@
 
 namespace {
 
-using namespace gfx::test;
-
 #if defined(TOOLKIT_VIEWS)
 TEST(UiGfxImageTest, ViewsImageView) {
-  gfx::Image image(CreatePlatformImage());
+  gfx::Image image(gfx::test::CreatePlatformImage());
 
   scoped_ptr<views::View> container(new views::View());
   container->SetBounds(0, 0, 200, 200);
@@ -45,7 +43,7 @@ TEST(UiGfxImageTest, GtkImageView) {
   GtkWidget* fixed = gtk_fixed_new();
   gtk_container_add(GTK_CONTAINER(window), fixed);
 
-  gfx::Image image(CreateBitmap());
+  gfx::Image image(gfx::test::CreateBitmap());
   GtkWidget* image_view = gtk_image_new_from_pixbuf(image);
   gtk_fixed_put(GTK_FIXED(fixed), image_view, 10, 10);
   gtk_widget_set_size_request(image_view, 25, 25);
diff --git a/chrome/browser/ui/tests/ui_gfx_image_unittest.mm b/chrome/browser/ui/tests/ui_gfx_image_unittest.mm
index 157d301..9f62332 100644
--- a/chrome/browser/ui/tests/ui_gfx_image_unittest.mm
+++ b/chrome/browser/ui/tests/ui_gfx_image_unittest.mm
@@ -13,13 +13,11 @@
 
 namespace {
 
-using namespace gfx::test;
-
 class UiGfxImageTest : public CocoaTest {
 };
 
 TEST_F(UiGfxImageTest, CheckColor) {
-  gfx::Image image(CreateBitmap());
+  gfx::Image image(gfx::test::CreateBitmap());
   [image lockFocus];
   NSColor* color = NSReadPixel(NSMakePoint(10, 10));
   [image unlockFocus];
@@ -44,7 +42,7 @@ TEST_F(UiGfxImageTest, ImageView) {
   [[test_window() contentView] addSubview:image_view];
   [test_window() orderFront:nil];
 
-  gfx::Image image(CreateBitmap());
+  gfx::Image image(gfx::test::CreateBitmap());
   [image_view setImage:image];
 }
 
diff --git a/ui/gfx/image.cc b/ui/gfx/image.cc
index 41c1594..b42e215 100644
--- a/ui/gfx/image.cc
+++ b/ui/gfx/image.cc
@@ -33,7 +33,7 @@ const SkBitmap* NSImageToSkBitmap(NSImage* image);
 const SkBitmap* GdkPixbufToSkBitmap(GdkPixbuf* pixbuf) {
   gfx::CanvasSkia canvas(gdk_pixbuf_get_width(pixbuf),
                          gdk_pixbuf_get_height(pixbuf),
-                           false);
+                         /*is_opaque=*/false);
   canvas.DrawGdkPixbuf(pixbuf, 0, 0);
   return new SkBitmap(canvas.ExtractBitmap());
 }
@@ -250,11 +250,9 @@ internal::ImageRep* Image::GetRepresentation(RepresentationType rep_type) {
           internal::NSImageToSkBitmap(nsimage_rep->image()));
     }
 #endif
-    if (rep) {
-      AddRepresentation(rep);
-      return rep;
-    }
-    NOTREACHED();
+    CHECK(rep);
+    AddRepresentation(rep);
+    return rep;
   }
 
   // Handle Skia-to-native conversions.
@@ -273,11 +271,9 @@ internal::ImageRep* Image::GetRepresentation(RepresentationType rep_type) {
       native_rep = new internal::NSImageRep(image);
     }
 #endif
-    if (native_rep) {
-      AddRepresentation(native_rep);
-      return native_rep;
-    }
-    NOTREACHED();
+    CHECK(native_rep);
+    AddRepresentation(native_rep);
+    return native_rep;
   }
 
   // Something went seriously wrong...
diff --git a/ui/gfx/image.h b/ui/gfx/image.h
index fb5f45a..fddf4da 100644
--- a/ui/gfx/image.h
+++ b/ui/gfx/image.h
@@ -2,6 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// An Image wraps an image any flavor, be it platform-native GdkBitmap/NSImage,
+// or a SkBitmap. This also provides easy conversion to other image types
+// through operator overloading. It will cache the converted representations
+// internally to prevent double-conversion.
+//
+// The lifetime of both the initial representation and any converted ones are
+// tied to the lifetime of the Image object itself.
+
 #ifndef UI_GFX_IMAGE_H_
 #define UI_GFX_IMAGE_H_
 #pragma once
@@ -25,13 +33,6 @@ namespace internal {
 class ImageRep;
 }
 
-// An Image wraps an image any flavor, be it platform-native GdkBitmap/NSImage,
-// or a SkBitmap. This also provides easy conversion to other image types
-// through operator overloading. It will cache the converted representations
-// internally to prevent double-conversion.
-//
-// The lifetime of both the initial representation and any converted ones are
-// tied to the lifetime of the Image object itself.
 class Image {
  public:
   enum RepresentationType {
@@ -70,7 +71,7 @@ class Image {
   void SwapRepresentations(gfx::Image* other);
 
  private:
-  friend class ::ImageTest;
+  typedef std::map<RepresentationType, internal::ImageRep*> RepresentationMap;
 
   // Returns the ImageRep for the default representation.
   internal::ImageRep* DefaultRepresentation();
@@ -86,11 +87,11 @@ class Image {
   // exist in the |representations_| map.
   RepresentationType default_representation_;
 
-  typedef std::map<RepresentationType, internal::ImageRep*> RepresentationMap;
   // All the representations of an Image. Size will always be at least one, with
   // more for any converted representations.
   RepresentationMap representations_;
 
+  friend class ::ImageTest;
   DISALLOW_COPY_AND_ASSIGN(Image);
 };
 
diff --git a/ui/gfx/image_unittest.cc b/ui/gfx/image_unittest.cc
index 37bfa98..6f41bfe 100644
--- a/ui/gfx/image_unittest.cc
+++ b/ui/gfx/image_unittest.cc
@@ -2,11 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ui/gfx/image.h"
-
 #include "base/memory/scoped_ptr.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/gfx/image.h"
 #include "ui/gfx/image_unittest.h"
 
 #if defined(OS_LINUX)
@@ -19,8 +18,6 @@
 
 namespace {
 
-using namespace gfx::test;
-
 #if defined(TOOLKIT_VIEWS)
 const bool kUsesSkiaNatively = true;
 #else
@@ -34,8 +31,10 @@ class ImageTest : public testing::Test {
   }
 };
 
+namespace gt = gfx::test;
+
 TEST_F(ImageTest, SkiaToSkia) {
-  gfx::Image image(CreateBitmap());
+  gfx::Image image(gt::CreateBitmap());
   const SkBitmap* bitmap = static_cast<const SkBitmap*>(image);
   EXPECT_TRUE(bitmap);
   EXPECT_FALSE(bitmap->isNull());
@@ -49,11 +48,11 @@ TEST_F(ImageTest, SkiaToSkia) {
 
   EXPECT_TRUE(image.HasRepresentation(gfx::Image::kSkBitmapRep));
   if (!kUsesSkiaNatively)
-    EXPECT_FALSE(image.HasRepresentation(GetPlatformRepresentationType()));
+    EXPECT_FALSE(image.HasRepresentation(gt::GetPlatformRepresentationType()));
 }
 
 TEST_F(ImageTest, SkiaToSkiaRef) {
-  gfx::Image image(CreateBitmap());
+  gfx::Image image(gt::CreateBitmap());
 
   const SkBitmap& bitmap = static_cast<const SkBitmap&>(image);
   EXPECT_FALSE(bitmap.isNull());
@@ -65,18 +64,18 @@ TEST_F(ImageTest, SkiaToSkiaRef) {
 
   EXPECT_TRUE(image.HasRepresentation(gfx::Image::kSkBitmapRep));
   if (!kUsesSkiaNatively)
-    EXPECT_FALSE(image.HasRepresentation(GetPlatformRepresentationType()));
+    EXPECT_FALSE(image.HasRepresentation(gt::GetPlatformRepresentationType()));
 }
 
 TEST_F(ImageTest, SkiaToPlatform) {
-  gfx::Image image(CreateBitmap());
+  gfx::Image image(gt::CreateBitmap());
   const size_t kRepCount = kUsesSkiaNatively ? 1U : 2U;
 
   EXPECT_TRUE(image.HasRepresentation(gfx::Image::kSkBitmapRep));
   if (!kUsesSkiaNatively)
-    EXPECT_FALSE(image.HasRepresentation(GetPlatformRepresentationType()));
+    EXPECT_FALSE(image.HasRepresentation(gt::GetPlatformRepresentationType()));
 
-  EXPECT_TRUE(static_cast<PlatformImage>(image));
+  EXPECT_TRUE(static_cast<gt::PlatformImage>(image));
   EXPECT_EQ(kRepCount, GetRepCount(image));
 
   const SkBitmap& bitmap = static_cast<const SkBitmap&>(image);
@@ -84,14 +83,14 @@ TEST_F(ImageTest, SkiaToPlatform) {
   EXPECT_EQ(kRepCount, GetRepCount(image));
 
   EXPECT_TRUE(image.HasRepresentation(gfx::Image::kSkBitmapRep));
-  EXPECT_TRUE(image.HasRepresentation(GetPlatformRepresentationType()));
+  EXPECT_TRUE(image.HasRepresentation(gt::GetPlatformRepresentationType()));
 }
 
 TEST_F(ImageTest, PlatformToSkia) {
-  gfx::Image image(CreatePlatformImage());
+  gfx::Image image(gt::CreatePlatformImage());
   const size_t kRepCount = kUsesSkiaNatively ? 1U : 2U;
 
-  EXPECT_TRUE(image.HasRepresentation(GetPlatformRepresentationType()));
+  EXPECT_TRUE(image.HasRepresentation(gt::GetPlatformRepresentationType()));
   if (!kUsesSkiaNatively)
     EXPECT_FALSE(image.HasRepresentation(gfx::Image::kSkBitmapRep));
 
@@ -100,28 +99,28 @@ TEST_F(ImageTest, PlatformToSkia) {
   EXPECT_FALSE(bitmap->isNull());
   EXPECT_EQ(kRepCount, GetRepCount(image));
 
-  EXPECT_TRUE(static_cast<PlatformImage>(image));
+  EXPECT_TRUE(static_cast<gt::PlatformImage>(image));
   EXPECT_EQ(kRepCount, GetRepCount(image));
 
   EXPECT_TRUE(image.HasRepresentation(gfx::Image::kSkBitmapRep));
 }
 
 TEST_F(ImageTest, PlatformToPlatform) {
-  gfx::Image image(CreatePlatformImage());
-  EXPECT_TRUE(static_cast<PlatformImage>(image));
+  gfx::Image image(gt::CreatePlatformImage());
+  EXPECT_TRUE(static_cast<gt::PlatformImage>(image));
   EXPECT_EQ(1U, GetRepCount(image));
 
   // Make sure double conversion doesn't happen.
-  EXPECT_TRUE(static_cast<PlatformImage>(image));
+  EXPECT_TRUE(static_cast<gt::PlatformImage>(image));
   EXPECT_EQ(1U, GetRepCount(image));
 
-  EXPECT_TRUE(image.HasRepresentation(GetPlatformRepresentationType()));
+  EXPECT_TRUE(image.HasRepresentation(gt::GetPlatformRepresentationType()));
   if (!kUsesSkiaNatively)
     EXPECT_FALSE(image.HasRepresentation(gfx::Image::kSkBitmapRep));
 }
 
 TEST_F(ImageTest, CheckSkiaColor) {
-  gfx::Image image(CreatePlatformImage());
+  gfx::Image image(gt::CreatePlatformImage());
   const SkBitmap& bitmap(image);
 
   SkAutoLockPixels auto_lock(bitmap);
@@ -132,19 +131,19 @@ TEST_F(ImageTest, CheckSkiaColor) {
 TEST_F(ImageTest, SwapRepresentations) {
   const size_t kRepCount = kUsesSkiaNatively ? 1U : 2U;
 
-  gfx::Image image1(CreateBitmap());
+  gfx::Image image1(gt::CreateBitmap());
   const SkBitmap* bitmap1 = image1;
   EXPECT_EQ(1U, GetRepCount(image1));
 
-  gfx::Image image2(CreatePlatformImage());
+  gfx::Image image2(gt::CreatePlatformImage());
   const SkBitmap* bitmap2 = image2;
-  PlatformImage platform_image = static_cast<PlatformImage>(image2);
+  gt::PlatformImage platform_image = static_cast<gt::PlatformImage>(image2);
   EXPECT_EQ(kRepCount, GetRepCount(image2));
 
   image1.SwapRepresentations(&image2);
 
   EXPECT_EQ(bitmap2, static_cast<const SkBitmap*>(image1));
-  EXPECT_EQ(platform_image, static_cast<PlatformImage>(image1));
+  EXPECT_EQ(platform_image, static_cast<gt::PlatformImage>(image1));
   EXPECT_EQ(bitmap1, static_cast<const SkBitmap*>(image2));
   EXPECT_EQ(kRepCount, GetRepCount(image1));
   EXPECT_EQ(1U, GetRepCount(image2));