Enables HiDPI favicon to be displayed in tab strip on CrOS

BUG=138550
Test=Manual
Run ChromeOS with --force-device-scale-factor=2
Go to a website for the first time. Favicon appears in HiDPI in tab strip on CrOS.

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

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

9 files changed, 35 insertions, 18 deletions

diff --git a/chrome/browser/favicon/favicon_tab_helper.cc b/chrome/browser/favicon/favicon_tab_helper.cc
index 2dbdf5e..bcb95d3 100644
--- a/chrome/browser/favicon/favicon_tab_helper.cc
+++ b/chrome/browser/favicon/favicon_tab_helper.cc
@@ -199,11 +199,7 @@ void FaviconTabHelper::OnDidDownloadFavicon(
   // TODO: Possibly do bitmap selection in FaviconHandler, so that it can score
   // favicons better.
   std::vector<ui::ScaleFactor> scale_factors;
-#if defined(OS_MACOSX)
   scale_factors = ui::GetSupportedScaleFactors();
-#else
-  scale_factors.push_back(ui::SCALE_FACTOR_100P);  // TODO: Aura?
-#endif
   gfx::Image favicon(SelectFaviconFrames(
       bitmaps, scale_factors, requested_size, &score));
   favicon_handler_->OnDidDownloadFavicon(
diff --git a/chrome/browser/ui/views/tab_icon_view.cc b/chrome/browser/ui/views/tab_icon_view.cc
index 1330cd0..0a2f974 100644
--- a/chrome/browser/ui/views/tab_icon_view.cc
+++ b/chrome/browser/ui/views/tab_icon_view.cc
@@ -99,7 +99,8 @@ void TabIconView::PaintThrobber(gfx::Canvas* canvas) {
             image_size, false);
 }
 
-void TabIconView::PaintFavicon(gfx::Canvas* canvas, const SkBitmap& image) {
+void TabIconView::PaintFavicon(gfx::Canvas* canvas,
+                               const gfx::ImageSkia& image) {
   PaintIcon(canvas, image, 0, 0, image.width(), image.height(), true);
 }
 
@@ -142,7 +143,7 @@ void TabIconView::OnPaint(gfx::Canvas* canvas) {
     rendered = true;
     PaintThrobber(canvas);
   } else {
-    SkBitmap favicon = model_->GetFaviconForTabIconView();
+    gfx::ImageSkia favicon = model_->GetFaviconForTabIconView();
     if (!favicon.isNull()) {
       rendered = true;
       PaintFavicon(canvas, favicon);
diff --git a/chrome/browser/ui/views/tab_icon_view.h b/chrome/browser/ui/views/tab_icon_view.h
index 79188ea..7c94656 100644
--- a/chrome/browser/ui/views/tab_icon_view.h
+++ b/chrome/browser/ui/views/tab_icon_view.h
@@ -9,8 +9,6 @@
 #include "base/compiler_specific.h"
 #include "ui/views/view.h"
 
-class SkBitmap;
-
 namespace chrome {
 class TabIconViewModel;
 }
@@ -39,7 +37,7 @@ class TabIconView : public views::View {
 
  private:
   void PaintThrobber(gfx::Canvas* canvas);
-  void PaintFavicon(gfx::Canvas* canvas, const SkBitmap& bitmap);
+  void PaintFavicon(gfx::Canvas* canvas, const gfx::ImageSkia& image);
   void PaintIcon(gfx::Canvas* canvas,
                  const gfx::ImageSkia& image,
                  int src_x,
diff --git a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
index 7d346f7..973a5cc 100644
--- a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
+++ b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
@@ -514,9 +514,8 @@ void BrowserTabStripController::SetTabRendererDataFromModel(
     TabStatus tab_status) {
   TabContents* tab_contents = TabContents::FromWebContents(contents);
 
-  // TODO: Convert data->favicon to gfx::Image.
   data->favicon =
-      tab_contents->favicon_tab_helper()->GetFavicon().AsBitmap();
+      tab_contents->favicon_tab_helper()->GetFavicon().AsImageSkia();
   data->network_state = TabContentsNetworkState(contents);
   data->title = contents->GetTitle();
   data->url = contents->GetURL();
diff --git a/chrome/browser/ui/views/tabs/tab_renderer_data.cc b/chrome/browser/ui/views/tabs/tab_renderer_data.cc
index d78cd8f..64535e7 100644
--- a/chrome/browser/ui/views/tabs/tab_renderer_data.cc
+++ b/chrome/browser/ui/views/tabs/tab_renderer_data.cc
@@ -21,9 +21,7 @@ TabRendererData::~TabRendererData() {}
 
 bool TabRendererData::Equals(const TabRendererData& data) {
   return
-      favicon.pixelRef() &&
-      favicon.pixelRef() == data.favicon.pixelRef() &&
-      favicon.pixelRefOffset() == data.favicon.pixelRefOffset() &&
+      favicon.BackedBySameObjectAs(data.favicon) &&
       network_state == data.network_state &&
       title == data.title &&
       url == data.url &&
diff --git a/chrome/browser/ui/views/tabs/tab_renderer_data.h b/chrome/browser/ui/views/tabs/tab_renderer_data.h
index d2aa573..aaca08e 100644
--- a/chrome/browser/ui/views/tabs/tab_renderer_data.h
+++ b/chrome/browser/ui/views/tabs/tab_renderer_data.h
@@ -10,7 +10,7 @@
 #include "chrome/browser/ui/search/search_types.h"
 #include "chrome/browser/ui/search/toolbar_search_animator.h"
 #include "googleurl/src/gurl.h"
-#include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/gfx/image/image_skia.h"
 
 // Wraps the state needed by the renderers.
 struct TabRendererData {
@@ -35,11 +35,10 @@ struct TabRendererData {
             crashed_status == base::TERMINATION_STATUS_ABNORMAL_TERMINATION);
   }
 
-  // Returns true if the TabRendererData is same as given |data|. Two favicons
-  // are considered equals if two SkBitmaps point to the same SkPixelRef object.
+  // Returns true if the TabRendererData is same as given |data|.
   bool Equals(const TabRendererData& data);
 
-  SkBitmap favicon;
+  gfx::ImageSkia favicon;
   NetworkState network_state;
   string16 title;
   GURL url;
diff --git a/ui/gfx/image/image_skia.cc b/ui/gfx/image/image_skia.cc
index d5e91c8..5050c9a 100644
--- a/ui/gfx/image/image_skia.cc
+++ b/ui/gfx/image/image_skia.cc
@@ -168,6 +168,10 @@ ImageSkia& ImageSkia::operator=(const SkBitmap& other) {
 ImageSkia::~ImageSkia() {
 }
 
+bool ImageSkia::BackedBySameObjectAs(const gfx::ImageSkia& other) const {
+  return storage_.get() == other.storage_.get();
+}
+
 void ImageSkia::AddRepresentation(const ImageSkiaRep& image_rep) {
   DCHECK(!image_rep.is_null());
 
diff --git a/ui/gfx/image/image_skia.h b/ui/gfx/image/image_skia.h
index 38e6e7a..a32ee9f 100644
--- a/ui/gfx/image/image_skia.h
+++ b/ui/gfx/image/image_skia.h
@@ -67,6 +67,10 @@ class UI_EXPORT ImageSkia {
 
   ~ImageSkia();
 
+  // Returns true if this object is backed by the same ImageSkiaStorage as
+  // |other|. Will also return true if both images are isNull().
+  bool BackedBySameObjectAs(const gfx::ImageSkia& other) const;
+
   // Adds |image_rep| to the image reps contained by this object.
   void AddRepresentation(const gfx::ImageSkiaRep& image_rep);
 
diff --git a/ui/gfx/image/image_skia_unittest.cc b/ui/gfx/image/image_skia_unittest.cc
index 1780254..789fbe2 100644
--- a/ui/gfx/image/image_skia_unittest.cc
+++ b/ui/gfx/image/image_skia_unittest.cc
@@ -196,4 +196,22 @@ TEST(ImageSkiaTest, OperatorBitmapFromSource) {
   EXPECT_FALSE(bitmap.isNull());
 }
 
+TEST(ImageSkiaTest, BackedBySameObjectAs) {
+  // Null images should all be backed by the same object (NULL).
+  ImageSkia image;
+  ImageSkia unrelated;
+  EXPECT_TRUE(image.BackedBySameObjectAs(unrelated));
+
+  image.AddRepresentation(gfx::ImageSkiaRep(gfx::Size(10, 10),
+                                            ui::SCALE_FACTOR_100P));
+  ImageSkia copy = image;
+  copy.AddRepresentation(gfx::ImageSkiaRep(gfx::Size(10, 10),
+                                           ui::SCALE_FACTOR_200P));
+  unrelated.AddRepresentation(gfx::ImageSkiaRep(gfx::Size(10, 10),
+                                                ui::SCALE_FACTOR_100P));
+  EXPECT_TRUE(image.BackedBySameObjectAs(copy));
+  EXPECT_FALSE(image.BackedBySameObjectAs(unrelated));
+  EXPECT_FALSE(copy.BackedBySameObjectAs(unrelated));
+}
+
 }  // namespace gfx