Fix ResourceBundleImageSource so that it does not crash when 1.4x resources are requested.

BUG=148841
Test=ResourceBundle.GetImageNamed

Review URL: https://chromiumcodereview.appspot.com/10928231

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

3 files changed, 102 insertions, 33 deletions

diff --git a/ui/base/resource/resource_bundle.cc b/ui/base/resource/resource_bundle.cc
index 0071a5a..80f9675 100644
--- a/ui/base/resource/resource_bundle.cc
+++ b/ui/base/resource/resource_bundle.cc
@@ -45,7 +45,7 @@ ResourceBundle* g_shared_instance_ = NULL;
 // Returns the actual scale factor of |bitmap| given the image representations
 // which have already been added to |image|.
 // TODO(pkotwicz): Remove this once we are no longer loading 1x resources
-// as part of 2x data packs.
+// as part of non 1x data packs.
 ui::ScaleFactor GetActualScaleFactor(const gfx::ImageSkia& image,
                                      const SkBitmap& bitmap,
                                      ui::ScaleFactor data_pack_scale_factor) {
@@ -56,19 +56,20 @@ ui::ScaleFactor GetActualScaleFactor(const gfx::ImageSkia& image,
       static_cast<float>(bitmap.width()) / image.width());
 }
 
-bool ShouldHighlightMissing2xResources() {
+bool ShouldHighlightMissingScaledResources() {
   return CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kHighlightMissing2xResources);
+      switches::kHighlightMissingScaledResources);
 }
 
 }  // namespace
 
-// An ImageSkiaSource that loads bitmaps for given scale factor from
+// An ImageSkiaSource that loads bitmaps for requested scale factor from
 // ResourceBundle on demand for given resource_id. It falls back
-// to 100P image if corresponding 200P image doesn't exist.
-// If 200P image does not have 2x size of 100P images, it will end up
-// with broken UI because it will be drawn as if it has 2x size.
-// When --highlight-missing-2x-resources flag is specified, it
+// to the 1x bitmap if the bitmap for the requested scale factor does not
+// exist. If the resource for the requested scale factor is not exactly
+// |scale_factor| * the size of the 1x resource, it will end up with
+// broken UI because it will be drawn as if the bitmap was the correct size.
+// When --highlight-missing-scaled-resources flag is specified, it
 // will show the scaled image blended with red instead.
 class ResourceBundle::ResourceBundleImageSource : public gfx::ImageSkiaSource {
  public:
@@ -84,52 +85,58 @@ class ResourceBundle::ResourceBundleImageSource : public gfx::ImageSkiaSource {
     ResourceBundle& rb = ResourceBundle::GetSharedInstance();
 
     scoped_ptr<SkBitmap> result(rb.LoadBitmap(resource_id_, scale_factor));
-    gfx::Size size_in_pixel =
-        size_in_dip_.Scale(ui::GetScaleFactorScale(scale_factor));
+    float scale = ui::GetScaleFactorScale(scale_factor);
+    gfx::Size size_in_pixel = size_in_dip_.Scale(scale);
 
-    if (scale_factor == SCALE_FACTOR_200P &&
+    if (scale_factor != SCALE_FACTOR_100P &&
         (!result.get() ||
          result->width() != size_in_pixel.width() ||
          result->height() != size_in_pixel.height())) {
 
-      // If 2x resource is missing from |image| or is the incorrect
-      // size and --highlight-missing-2x-resources is specified, logs
-      // the resource id and creates a 2x version of the resource.
-      // Blends the created resource with red to make it
+      // If non 1x resource is missing from |image| or is the incorrect
+      // size and --highlight-missing-scaled-resources is specified, logs
+      // the resource id and creates a version of the resource at the correct
+      // size. Blends the created resource with red to make it
       // distinguishable from bitmaps in the resource pak.
-      if (ShouldHighlightMissing2xResources()) {
-        if (!result.get())
-          LOG(ERROR) << "Missing 2x resource. id=" << resource_id_;
-        else
-          LOG(ERROR) << "Incorrectly sized 2x resource. id=" << resource_id_;
-
-        SkBitmap bitmap1x = *(rb.LoadBitmap(resource_id_, SCALE_FACTOR_100P));
-        SkBitmap bitmap2x = skia::ImageOperations::Resize(
-            bitmap1x,
+      if (ShouldHighlightMissingScaledResources()) {
+        if (!result.get()) {
+          LOG(ERROR) << "Missing " << scale << "x resource. id="
+                     << resource_id_;
+        } else {
+          LOG(ERROR) << "Incorrectly sized " << scale << "x resource. id="
+                     << resource_id_;
+        }
+
+        scoped_ptr<SkBitmap> bitmap1x(
+            rb.LoadBitmap(resource_id_, SCALE_FACTOR_100P));
+        DCHECK(bitmap1x.get());
+        SkBitmap bitmap_scaled = skia::ImageOperations::Resize(
+            *bitmap1x,
             skia::ImageOperations::RESIZE_LANCZOS3,
-            bitmap1x.width() * 2, bitmap1x.height() * 2);
+            size_in_pixel.width(),
+            size_in_pixel.height());
 
         SkBitmap mask;
         mask.setConfig(SkBitmap::kARGB_8888_Config,
-                       bitmap2x.width(),
-                       bitmap2x.height());
+                       bitmap_scaled.width(),
+                       bitmap_scaled.height());
         mask.allocPixels();
         mask.eraseColor(SK_ColorRED);
         result.reset(new SkBitmap());
-        *result.get() = SkBitmapOperations::CreateBlendedBitmap(bitmap2x, mask,
-                                                                0.2);
-      } else if (!result.get() ||
-                 result->width() == size_in_dip_.width()) {
-        // The 2x resource pack may have the 1x image if its grd file
+        *result.get() = SkBitmapOperations::CreateBlendedBitmap(
+            bitmap_scaled, mask, 0.2);
+      } else if (!result.get() || result->width() == size_in_dip_.width()) {
+        // The scaled resource pack may have the 1x image if its grd file
         // points to 1x image. Fallback to 1x by returning empty image
         // in this case. This 1x image will be scaled when drawn.
         return gfx::ImageSkiaRep();
       }
-      // If the size of 2x image isn't exactly 2x of 1x version,
+      // If the size of scaled image isn't exactly |scale| * 1x version,
       // create ImageSkia as usual. This will end up with
       // corrupted visual representation as the size of image doesn't
       // match the expected size.
     }
+    DCHECK(result.get());
     return gfx::ImageSkiaRep(*result.get(), scale_factor);
   }
 
diff --git a/ui/base/resource/resource_bundle.h b/ui/base/resource/resource_bundle.h
index 570de5c..840036df 100644
--- a/ui/base/resource/resource_bundle.h
+++ b/ui/base/resource/resource_bundle.h
@@ -247,6 +247,7 @@ class UI_EXPORT ResourceBundle {
   FRIEND_TEST_ALL_PREFIXES(ResourceBundle, GetRawDataResource);
   FRIEND_TEST_ALL_PREFIXES(ResourceBundle, LoadDataResourceBytes);
   FRIEND_TEST_ALL_PREFIXES(ResourceBundle, LocaleDataPakExists);
+  FRIEND_TEST_ALL_PREFIXES(ResourceBundle, GetImageNamed);
 
   class ResourceBundleImageSource;
   friend class ResourceBundleImageSource;
diff --git a/ui/base/resource/resource_bundle_unittest.cc b/ui/base/resource/resource_bundle_unittest.cc
index 6c4f953..7a60c0f 100644
--- a/ui/base/resource/resource_bundle_unittest.cc
+++ b/ui/base/resource/resource_bundle_unittest.cc
@@ -15,6 +15,9 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/layout.h"
+#include "ui/base/resource/data_pack.h"
+#include "ui/gfx/codec/png_codec.h"
+#include "ui/gfx/image/image_skia.h"
 
 using ::testing::_;
 using ::testing::Between;
@@ -73,6 +76,22 @@ class MockResourceBundleDelegate : public ui::ResourceBundle::Delegate {
   }
 };
 
+// Creates datapack at |path| with a single bitmap at resource ID 3
+// which is |edge_size|x|edge_size| pixels.
+void CreateDataPackWithSingleBitmap(const FilePath& path,
+                                    int edge_size) {
+  SkBitmap bitmap;
+  bitmap.setConfig(SkBitmap::kARGB_8888_Config, edge_size, edge_size);
+  bitmap.allocPixels();
+  std::vector<unsigned char> bitmap_data;
+  EXPECT_TRUE(gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, false, &bitmap_data));
+
+  std::map<uint16, base::StringPiece> resources;
+  resources[3u] = base::StringPiece(
+      reinterpret_cast<const char*>(&bitmap_data[0]), bitmap_data.size());
+  DataPack::WritePack(path, resources, ui::DataPack::BINARY);
+}
+
 }  // namespace
 
 TEST(ResourceBundle, DelegateGetPathForResourcePack) {
@@ -307,4 +326,46 @@ TEST(ResourceBundle, LocaleDataPakExists) {
   EXPECT_FALSE(resource_bundle.LocaleDataPakExists("not_a_real_locale"));
 }
 
+// Test requesting image reps at various scale factors from the image returned
+// via ResourceBundle::GetImageNamed().
+TEST(ResourceBundle, GetImageNamed) {
+  // On Windows, the default data is compiled into the binary so this does
+  // nothing.
+  ScopedTempDir dir;
+  ASSERT_TRUE(dir.CreateUniqueTempDir());
+
+  FilePath locale_path = dir.path().Append(FILE_PATH_LITERAL("empty.pak"));
+  FilePath data_path = dir.path().Append(FILE_PATH_LITERAL("sample.pak"));
+  FilePath data_2x_path = dir.path().Append(FILE_PATH_LITERAL("sample_2x.pak"));
+
+  {
+    // Create the pak files.
+    ASSERT_EQ(file_util::WriteFile(locale_path, kEmptyPakContents,
+        kEmptyPakSize), static_cast<int>(kEmptyPakSize));
+    CreateDataPackWithSingleBitmap(data_path, 10);
+    CreateDataPackWithSingleBitmap(data_2x_path, 20);
+
+    // Load the regular and 2x pak files.
+    ResourceBundle& resource_bundle = ResourceBundle::GetSharedInstance();
+    resource_bundle.LoadTestResources(data_path, locale_path);
+    resource_bundle.AddDataPackFromPath(data_2x_path, SCALE_FACTOR_200P);
+
+    gfx::ImageSkia* image_skia = resource_bundle.GetImageSkiaNamed(3);
+
+    // Resource ID 3 exists in both 1x and 2x paks. Image reps should be
+    // available for both scale factors in |image_skia|.
+    gfx::ImageSkiaRep image_rep =
+        image_skia->GetRepresentation(ui::SCALE_FACTOR_100P);
+    EXPECT_EQ(ui::SCALE_FACTOR_100P, image_rep.scale_factor());
+    image_rep = image_skia->GetRepresentation(ui::SCALE_FACTOR_200P);
+    EXPECT_EQ(ui::SCALE_FACTOR_200P, image_rep.scale_factor());
+
+    // The 1.4x pack was not loaded. Requesting the 1.4x resource should return
+    // either the 1x or the 2x resource.
+    image_rep = image_skia->GetRepresentation(ui::SCALE_FACTOR_140P);
+    EXPECT_TRUE(image_rep.scale_factor() == ui::SCALE_FACTOR_100P ||
+                image_rep.scale_factor() == ui::SCALE_FACTOR_200P);
+  }
+}
+
 }  // namespace ui