[Suggestions] Create ImageEncoder, to abstract away image encode/decode.

iOS can't depend on JPEG encoder.

TBR=jam
BUG=387751,409156

Review URL: https://codereview.chromium.org/630073002

Cr-Commit-Position: refs/heads/master@{#298510}

12 files changed, 176 insertions, 38 deletions

diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 4fdba5e..77f2a13 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -97,7 +97,8 @@ static_library("browser") {
     "//components/signin/core/browser",
     "//components/startup_metric_utils",
     "//components/strings",
-    "//components/suggestions",
+    "//components/suggestions:jpeg_image_encoder",
+    "//components/suggestions:suggestions",
     "//components/sync_driver",
     "//components/translate/core/browser",
     "//components/translate/core/common",
diff --git a/chrome/browser/search/suggestions/suggestions_service_factory.cc b/chrome/browser/search/suggestions/suggestions_service_factory.cc
index 4a98018..f8b714b 100644
--- a/chrome/browser/search/suggestions/suggestions_service_factory.cc
+++ b/chrome/browser/search/suggestions/suggestions_service_factory.cc
@@ -14,8 +14,10 @@
 #include "components/leveldb_proto/proto_database_impl.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/suggestions/blacklist_store.h"
+#include "components/suggestions/image_encoder.h"
 #include "components/suggestions/image_fetcher.h"
 #include "components/suggestions/image_manager.h"
+#include "components/suggestions/jpeg/jpeg_image_encoder.h"
 #include "components/suggestions/proto/suggestions.pb.h"
 #include "components/suggestions/suggestions_service.h"
 #include "components/suggestions/suggestions_store.h"
@@ -72,11 +74,13 @@ KeyedService* SuggestionsServiceFactory::BuildServiceInstanceFor(
   base::FilePath database_dir(
       the_profile->GetPath().Append(FILE_PATH_LITERAL("Thumbnails")));
 
+  scoped_ptr<JpegImageEncoder> image_encoder(new JpegImageEncoder());
   scoped_ptr<ImageFetcherImpl> image_fetcher(
       new ImageFetcherImpl(the_profile->GetRequestContext()));
   scoped_ptr<ImageManager> thumbnail_manager(new ImageManager(
-      image_fetcher.PassAs<ImageFetcher>(),
-      db.PassAs<leveldb_proto::ProtoDatabase<ImageData> >(), database_dir));
+      image_fetcher.Pass(),
+      image_encoder.Pass(),
+      db.Pass(), database_dir));
   return new SuggestionsService(
       the_profile->GetRequestContext(), suggestions_store.Pass(),
       thumbnail_manager.Pass(), blacklist_store.Pass());
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 6393474..c9e9133 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2805,6 +2805,7 @@
         '../components/components.gyp:history_core_common',
         '../components/components.gyp:infobars_core',
         '../components/components.gyp:invalidation',
+        '../components/components.gyp:jpeg_image_encoder',
         '../components/components.gyp:metrics',
         '../components/components.gyp:metrics_gpu',
         '../components/components.gyp:metrics_net',
diff --git a/components/components_tests.gyp b/components/components_tests.gyp
index c318f1b..6ddea80 100644
--- a/components/components_tests.gyp
+++ b/components/components_tests.gyp
@@ -430,6 +430,7 @@
             '../google_apis/google_apis.gyp:google_apis_test_support',
 
             # Dependencies of suggestions
+            'components.gyp:jpeg_image_encoder',
             'components.gyp:suggestions',
 
             # Dependencies of sync_driver
diff --git a/components/suggestions.gypi b/components/suggestions.gypi
index e03dc21..88c7135 100644
--- a/components/suggestions.gypi
+++ b/components/suggestions.gypi
@@ -24,6 +24,7 @@
       'sources': [
         'suggestions/blacklist_store.cc',
         'suggestions/blacklist_store.h',
+        'suggestions/image_encoder.h',
         'suggestions/image_fetcher.h',
         'suggestions/image_fetcher_delegate.h',
         'suggestions/image_manager.cc',
@@ -44,5 +45,22 @@
       },
       'includes': [ '../build/protoc.gypi' ],
     },
-  ],
+    # TODO(justincohen): iOS cannot depend on ui/gfx, so we should provide an
+    # implementation of the image encoder, and put this target in a block that
+    # excludes iOS.
+    {
+      # GN version: //components/suggestions:jpeg_image_encoder
+      'target_name': 'jpeg_image_encoder',
+      'type': 'static_library',
+      'dependencies': [
+        '../base/base.gyp:base',
+        '../ui/gfx/gfx.gyp:gfx',
+        'suggestions',
+      ],
+      'sources': [
+        'suggestions/jpeg/jpeg_image_encoder.cc',
+        'suggestions/jpeg/jpeg_image_encoder.h',
+      ],
+    }
+  ]
 }
diff --git a/components/suggestions/BUILD.gn b/components/suggestions/BUILD.gn
index 12c9576..855e028 100644
--- a/components/suggestions/BUILD.gn
+++ b/components/suggestions/BUILD.gn
@@ -6,6 +6,7 @@ static_library("suggestions") {
   sources = [
     "blacklist_store.cc",
     "blacklist_store.h",
+    "image_encoder.h",
     "image_fetcher.h",
     "image_fetcher_delegate.h",
     "image_manager.cc",
@@ -31,3 +32,16 @@ static_library("suggestions") {
     "//url",
   ]
 }
+
+static_library("jpeg_image_encoder") {
+  sources = [
+    "jpeg/jpeg_image_encoder.cc",
+    "jpeg/jpeg_image_encoder.h",
+  ]
+
+  deps = [
+    "//base",
+    "//ui/gfx",
+    ":suggestions",
+  ]
+}
diff --git a/components/suggestions/image_encoder.h b/components/suggestions/image_encoder.h
new file mode 100644
index 0000000..14e511e
--- /dev/null
+++ b/components/suggestions/image_encoder.h
@@ -0,0 +1,36 @@
+// Copyright 2014 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 COMPONENTS_SUGGESTIONS_IMAGE_ENCODER_H_
+#define COMPONENTS_SUGGESTIONS_IMAGE_ENCODER_H_
+
+#include <vector>
+
+#include "base/macros.h"
+
+class SkBitmap;
+
+namespace suggestions {
+
+// A interface to encode/decode images.
+class ImageEncoder {
+ public:
+  ImageEncoder() {}
+  virtual ~ImageEncoder() {}
+
+  // From encoded bytes to SkBitmap. Caller owns the returned pointer.
+  virtual SkBitmap* DecodeImage(
+      const std::vector<unsigned char>& encoded_data) = 0;
+
+  // From SkBitmap to the vector of encoded bytes, |dst|.
+  virtual bool EncodeImage(
+      const SkBitmap& bitmap, std::vector<unsigned char>* dest) = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ImageEncoder);
+};
+
+}  // namespace suggestions
+
+#endif  // COMPONENTS_SUGGESTIONS_IMAGE_ENCODER_H_
diff --git a/components/suggestions/image_manager.cc b/components/suggestions/image_manager.cc
index 9e43381..b702fed 100644
--- a/components/suggestions/image_manager.cc
+++ b/components/suggestions/image_manager.cc
@@ -5,28 +5,21 @@
 #include "components/suggestions/image_manager.h"
 
 #include "base/bind.h"
+#include "components/suggestions/image_encoder.h"
 #include "components/suggestions/image_fetcher.h"
-#include "ui/gfx/codec/jpeg_codec.h"
 
 using leveldb_proto::ProtoDatabase;
 
-namespace {
-
-// From JPEG-encoded bytes to SkBitmap.
-SkBitmap* DecodeImage(const std::vector<unsigned char>& encoded_data) {
-  return gfx::JPEGCodec::Decode(&encoded_data[0], encoded_data.size());
-}
-
-}  // namespace
-
 namespace suggestions {
 
 ImageManager::ImageManager() : weak_ptr_factory_(this) {}
 
 ImageManager::ImageManager(scoped_ptr<ImageFetcher> image_fetcher,
+                           scoped_ptr<ImageEncoder> image_encoder,
                            scoped_ptr<ProtoDatabase<ImageData> > database,
                            const base::FilePath& database_dir)
     : image_fetcher_(image_fetcher.Pass()),
+      image_encoder_(image_encoder.Pass()),
       database_(database.Pass()),
       database_ready_(false),
       weak_ptr_factory_(this) {
@@ -139,7 +132,7 @@ void ImageManager::SaveImage(const GURL& url, const SkBitmap& bitmap) {
   // Attempt to save a JPEG representation to the database. If not successful,
   // the fetched bitmap will still be inserted in the cache, above.
   std::vector<unsigned char> encoded_data;
-  if (EncodeImage(bitmap, &encoded_data)) {
+  if (image_encoder_->EncodeImage(bitmap, &encoded_data)) {
     // Save the resulting bitmap to the database.
     ImageData data;
     data.set_url(url.spec());
@@ -194,7 +187,7 @@ void ImageManager::LoadEntriesInCache(scoped_ptr<ImageDataVector> entries) {
     std::vector<unsigned char> encoded_data(it->data().begin(),
                                             it->data().end());
 
-    scoped_ptr<SkBitmap> bitmap(DecodeImage(encoded_data));
+    scoped_ptr<SkBitmap> bitmap(image_encoder_->DecodeImage(encoded_data));
     if (bitmap.get()) {
       image_map_.insert(std::make_pair(it->url(), *bitmap));
     }
@@ -212,17 +205,4 @@ void ImageManager::ServePendingCacheRequests() {
   }
 }
 
-// static
-bool ImageManager::EncodeImage(const SkBitmap& bitmap,
-                               std::vector<unsigned char>* dest) {
-  SkAutoLockPixels bitmap_lock(bitmap);
-  if (!bitmap.readyToDraw() || bitmap.isNull()) {
-    return false;
-  }
-  return gfx::JPEGCodec::Encode(
-      reinterpret_cast<unsigned char*>(bitmap.getAddr32(0, 0)),
-      gfx::JPEGCodec::FORMAT_SkBitmap, bitmap.width(), bitmap.height(),
-      bitmap.rowBytes(), 100, dest);
-}
-
 }  // namespace suggestions
diff --git a/components/suggestions/image_manager.h b/components/suggestions/image_manager.h
index 02ec268..76f8a94 100644
--- a/components/suggestions/image_manager.h
+++ b/components/suggestions/image_manager.h
@@ -28,6 +28,7 @@ class URLRequestContextGetter;
 namespace suggestions {
 
 class ImageData;
+class ImageEncoder;
 class ImageFetcher;
 class SuggestionsProfile;
 
@@ -37,7 +38,10 @@ class ImageManager : public ImageFetcherDelegate {
  public:
   typedef std::vector<ImageData> ImageDataVector;
 
+  // This class takes ownership of |image_fetcher|, |image_encoder| and
+  // |database|.
   ImageManager(scoped_ptr<ImageFetcher> image_fetcher,
+               scoped_ptr<ImageEncoder> image_encoder,
                scoped_ptr<leveldb_proto::ProtoDatabase<ImageData> > database,
                const base::FilePath& database_dir);
   virtual ~ImageManager();
@@ -119,11 +123,6 @@ class ImageManager : public ImageFetcherDelegate {
 
   void ServePendingCacheRequests();
 
-  // From SkBitmap to the vector of JPEG-encoded bytes, |dst|. Visible only for
-  // testing.
-  static bool EncodeImage(const SkBitmap& bitmap,
-                          std::vector<unsigned char>* dest);
-
   // Map from URL to image URL. Should be kept up to date when a new
   // SuggestionsProfile is available.
   std::map<GURL, GURL> image_url_map_;
@@ -137,6 +136,8 @@ class ImageManager : public ImageFetcherDelegate {
 
   scoped_ptr<ImageFetcher> image_fetcher_;
 
+  scoped_ptr<ImageEncoder> image_encoder_;
+
   scoped_ptr<leveldb_proto::ProtoDatabase<ImageData> > database_;
 
   bool database_ready_;
diff --git a/components/suggestions/image_manager_unittest.cc b/components/suggestions/image_manager_unittest.cc
index 57dd8bf..cd39176 100644
--- a/components/suggestions/image_manager_unittest.cc
+++ b/components/suggestions/image_manager_unittest.cc
@@ -3,14 +3,17 @@
 // found in the LICENSE file.
 
 #include <string>
+#include <vector>
 
 #include "base/files/file_path.h"
 #include "base/run_loop.h"
 #include "components/leveldb_proto/proto_database.h"
 #include "components/leveldb_proto/testing/fake_db.h"
+#include "components/suggestions/image_encoder.h"
 #include "components/suggestions/image_fetcher.h"
 #include "components/suggestions/image_fetcher_delegate.h"
 #include "components/suggestions/image_manager.h"
+#include "components/suggestions/jpeg/jpeg_image_encoder.h"
 #include "components/suggestions/proto/suggestions.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -46,6 +49,15 @@ class MockImageFetcher : public suggestions::ImageFetcher {
   MOCK_METHOD1(SetImageFetcherDelegate, void(ImageFetcherDelegate*));
 };
 
+class MockImageEncoder : public suggestions::ImageEncoder {
+ public:
+  MockImageEncoder() {}
+  virtual ~MockImageEncoder() {}
+  MOCK_METHOD1(DecodeImage,
+               SkBitmap*(const std::vector<unsigned char>&));
+  MOCK_METHOD2(EncodeImage, bool(const SkBitmap&, std::vector<unsigned char>*));
+};
+
 class ImageManagerTest : public testing::Test {
  public:
   ImageManagerTest()
@@ -55,7 +67,8 @@ class ImageManagerTest : public testing::Test {
 
   virtual void SetUp() override {
     fake_db_ = new FakeDB<ImageData>(&db_model_);
-    image_manager_.reset(CreateImageManager(fake_db_));
+    jpeg_image_encoder_ = new JpegImageEncoder();
+    image_manager_.reset(CreateImageManager(fake_db_, jpeg_image_encoder_));
   }
 
   virtual void TearDown() override {
@@ -92,7 +105,7 @@ class ImageManagerTest : public testing::Test {
     ImageData data;
     data.set_url(url);
     std::vector<unsigned char> encoded;
-    EXPECT_TRUE(ImageManager::EncodeImage(bm, &encoded));
+    EXPECT_TRUE(jpeg_image_encoder_->EncodeImage(bm, &encoded));
     data.set_data(std::string(encoded.begin(), encoded.end()));
     return data;
   }
@@ -107,11 +120,13 @@ class ImageManagerTest : public testing::Test {
     loop->Quit();
   }
 
-  ImageManager* CreateImageManager(FakeDB<ImageData>* fake_db) {
+  ImageManager* CreateImageManager(FakeDB<ImageData>* fake_db,
+                                   JpegImageEncoder* jpeg_image_encoder) {
     mock_image_fetcher_ = new StrictMock<MockImageFetcher>();
     EXPECT_CALL(*mock_image_fetcher_, SetImageFetcherDelegate(_));
     return new ImageManager(
         scoped_ptr<ImageFetcher>(mock_image_fetcher_),
+        scoped_ptr<ImageEncoder>(jpeg_image_encoder),
         scoped_ptr<leveldb_proto::ProtoDatabase<ImageData> >(fake_db),
         FakeDB<ImageData>::DirectoryForTestDB());
   }
@@ -119,6 +134,7 @@ class ImageManagerTest : public testing::Test {
   EntryMap db_model_;
   // Owned by the ImageManager under test.
   FakeDB<ImageData>* fake_db_;
+  JpegImageEncoder* jpeg_image_encoder_;
 
   MockImageFetcher* mock_image_fetcher_;
 
@@ -175,7 +191,7 @@ TEST_F(ImageManagerTest, GetImageForURLNetworkCacheHit) {
   // Create the ImageManager with an added entry in the database.
   AddEntry(GetSampleImageData(kTestUrl1), &db_model_);
   FakeDB<ImageData>* fake_db = new FakeDB<ImageData>(&db_model_);
-  image_manager_.reset(CreateImageManager(fake_db));
+  image_manager_.reset(CreateImageManager(fake_db, new JpegImageEncoder()));
   image_manager_->Initialize(suggestions_profile);
   fake_db->InitCallback(true);
   fake_db->LoadCallback(true);
diff --git a/components/suggestions/jpeg/jpeg_image_encoder.cc b/components/suggestions/jpeg/jpeg_image_encoder.cc
new file mode 100644
index 0000000..a3a0cf4
--- /dev/null
+++ b/components/suggestions/jpeg/jpeg_image_encoder.cc
@@ -0,0 +1,29 @@
+// Copyright 2014 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.
+
+#include "components/suggestions/jpeg/jpeg_image_encoder.h"
+
+#include "ui/gfx/codec/jpeg_codec.h"
+#include "ui/gfx/image/image_skia.h"
+
+namespace suggestions {
+
+SkBitmap* JpegImageEncoder::DecodeImage(
+    const std::vector<unsigned char>& encoded_data) {
+  return gfx::JPEGCodec::Decode(&encoded_data[0], encoded_data.size());
+}
+
+bool JpegImageEncoder::EncodeImage(const SkBitmap& bitmap,
+                               std::vector<unsigned char>* dest) {
+  SkAutoLockPixels bitmap_lock(bitmap);
+  if (!bitmap.readyToDraw() || bitmap.isNull()) {
+    return false;
+  }
+  return gfx::JPEGCodec::Encode(
+      reinterpret_cast<unsigned char*>(bitmap.getAddr32(0, 0)),
+      gfx::JPEGCodec::FORMAT_SkBitmap, bitmap.width(), bitmap.height(),
+      bitmap.rowBytes(), 100, dest);
+}
+
+}  // namespace suggestions
diff --git a/components/suggestions/jpeg/jpeg_image_encoder.h b/components/suggestions/jpeg/jpeg_image_encoder.h
new file mode 100644
index 0000000..09f3975
--- /dev/null
+++ b/components/suggestions/jpeg/jpeg_image_encoder.h
@@ -0,0 +1,37 @@
+// Copyright 2014 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 COMPONENTS_SUGGESTIONS_JPEG_JPEG_IMAGE_ENCODER_H_
+#define COMPONENTS_SUGGESTIONS_JPEG_JPEG_IMAGE_ENCODER_H_
+
+#include <vector>
+
+#include "base/macros.h"
+#include "components/suggestions/image_encoder.h"
+
+class SkBitmap;
+
+namespace suggestions {
+
+// A class used to encode/decode images using the JPEG codec.
+class JpegImageEncoder : public ImageEncoder {
+ public:
+  JpegImageEncoder() {}
+  virtual ~JpegImageEncoder() {}
+
+  // From encoded bytes to SkBitmap. Caller owns the returned pointer.
+  virtual SkBitmap* DecodeImage(
+      const std::vector<unsigned char>& encoded_data) OVERRIDE;
+
+  // From SkBitmap to the vector of encoded bytes, |dst|.
+  virtual bool EncodeImage(const SkBitmap& bitmap,
+                           std::vector<unsigned char>* dest) OVERRIDE;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(JpegImageEncoder);
+};
+
+}  // namespace suggestions
+
+#endif  // COMPONENTS_SUGGESTIONS_JPEG_JPEG_IMAGE_ENCODER_H_