Repurpose NotificationBitmapFetcher to BitmapFetcher

There was nothing specific about this class, so I'm generalizing it and moving it upwards in the tree for more use cases (e.g. app banners).

Code that will use it is not ready for review, but is located at https://chromiumcodereview.appspot.com/156343002/

BUG=341556

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

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

chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.cc → chrome/browser/bitmap_fetcher.cc

-// Copyright 2013 The Chromium Authors. All rights reserved.
+// 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 "chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher.h"
 
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_status.h"
 
-namespace notifier {
+namespace chrome {
 
-NotificationBitmapFetcher::NotificationBitmapFetcher(
-    const GURL& url,
-    NotificationBitmapFetcherDelegate* delegate)
-    : url_(url), delegate_(delegate) {}
+BitmapFetcher::BitmapFetcher(const GURL& url,
+                             BitmapFetcherDelegate* delegate)
+    : url_(url),
+      delegate_(delegate) {
+}
 
-NotificationBitmapFetcher::~NotificationBitmapFetcher() {}
+BitmapFetcher::~BitmapFetcher() {}
 
-void NotificationBitmapFetcher::Start(Profile* profile) {
-  url_fetcher_.reset(
-      net::URLFetcher::Create(url_, net::URLFetcher::GET, this));
-  // The RequestContext is coming from the current profile.
-  // TODO(petewil): Make sure this is the right profile to use.
-  // It seems to work, but we might prefer to use a blank profile with
-  // no cookies.
+void BitmapFetcher::Start(Profile* profile) {
+  DCHECK(url_fetcher_ == NULL);
+  url_fetcher_.reset(net::URLFetcher::Create(url_, net::URLFetcher::GET, this));
   url_fetcher_->SetRequestContext(profile->GetRequestContext());
   url_fetcher_->Start();
 }
 
 // Methods inherited from URLFetcherDelegate.
 
-void NotificationBitmapFetcher::OnURLFetchComplete(
-    const net::URLFetcher* source) {
+void BitmapFetcher::OnURLFetchComplete(const net::URLFetcher* source) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   if (source->GetStatus().status() != net::URLRequestStatus::SUCCESS) {
-    OnDecodeImageFailed(NULL);
+    ReportFailure();
     return;
   }
 
   std::string image_data;
   source->GetResponseAsString(&image_data);
-  // Create an ImageDecoder with the data and assign it to the refptr.
-  image_decoder_ = new ImageDecoder(this, image_data,
-                                    ImageDecoder::DEFAULT_CODEC);
+  image_decoder_ =
+      new ImageDecoder(this, image_data, ImageDecoder::DEFAULT_CODEC);
 
   // Call start to begin decoding.  The ImageDecoder will call OnImageDecoded
   // with the data when it is done.
@@ -54,28 +49,30 @@ void NotificationBitmapFetcher::OnURLFetchComplete(
   image_decoder_->Start(task_runner);
 }
 
-void NotificationBitmapFetcher::OnURLFetchDownloadProgress(
-    const net::URLFetcher* source, int64 current, int64 total) {
+void BitmapFetcher::OnURLFetchDownloadProgress(const net::URLFetcher* source,
+                                               int64 current,
+                                               int64 total) {
   // Do nothing here.
 }
 
 // Methods inherited from ImageDecoder::Delegate.
 
-void NotificationBitmapFetcher::OnImageDecoded(
-    const ImageDecoder* decoder, const SkBitmap& decoded_image) {
+void BitmapFetcher::OnImageDecoded(const ImageDecoder* decoder,
+                                   const SkBitmap& decoded_image) {
   // Make a copy of the bitmap which we pass back to the UI thread.
-  bitmap_.reset(new SkBitmap());
-  decoded_image.deepCopyTo(bitmap_.get(), decoded_image.getConfig());
+  scoped_ptr<SkBitmap> bitmap(new SkBitmap());
+  decoded_image.deepCopyTo(bitmap.get(), decoded_image.getConfig());
 
   // Report success.
-  delegate_->OnFetchComplete(url_, bitmap_.get());
+  delegate_->OnFetchComplete(url_, bitmap.get());
 }
 
-void NotificationBitmapFetcher::OnDecodeImageFailed(
-    const ImageDecoder* decoder) {
+void BitmapFetcher::OnDecodeImageFailed(const ImageDecoder* decoder) {
+  ReportFailure();
+}
 
-  // Report failure.
+void BitmapFetcher::ReportFailure() {
   delegate_->OnFetchComplete(url_, NULL);
 }
 
-}  // namespace notifier
+}  // namespace chrome

chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.h → chrome/browser/bitmap_fetcher.h

-// Copyright 2013 The Chromium Authors. All rights reserved.
+// 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 CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_NOTIFICATION_BITMAP_FETCHER_H_
-#define CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_NOTIFICATION_BITMAP_FETCHER_H_
+#ifndef CHROME_BROWSER_BITMAP_FETCHER_H_
+#define CHROME_BROWSER_BITMAP_FETCHER_H_
 
 #include "base/memory/scoped_ptr.h"
+#include "chrome/browser/bitmap_fetcher_delegate.h"
 #include "chrome/browser/image_decoder.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -17,33 +18,20 @@ class URLFetcher;
 
 class Profile;
 
-namespace notifier {
+namespace chrome {
 
-// A delegate interface for users of NotificationBitmapFetcher.
-class NotificationBitmapFetcherDelegate {
- public:
-  // This will be called when the bitmap has been requested, whether or not the
-  // request succeeds.  |url| is the URL that was originally fetched so we can
-  // match up the bitmap with a specific request.
-  virtual void OnFetchComplete(const GURL url, const SkBitmap* bitmap) = 0;
-
- protected:
-  virtual ~NotificationBitmapFetcherDelegate() {}
-};
-
-class NotificationBitmapFetcher
-    : public net::URLFetcherDelegate,
+// Asynchrounously fetches an image from the given URL and returns the
+// decoded Bitmap to the provided BitmapFetcherDelegate.
+class BitmapFetcher : public net::URLFetcherDelegate,
                       public ImageDecoder::Delegate {
  public:
-  NotificationBitmapFetcher(
-      const GURL& url,
-      NotificationBitmapFetcherDelegate* delegate);
-  virtual ~NotificationBitmapFetcher();
+  BitmapFetcher(const GURL& url, BitmapFetcherDelegate* delegate);
+  virtual ~BitmapFetcher();
 
-  GURL url() const { return url_; }
+  const GURL& url() const { return url_; }
 
-  // Start fetching the URL with the fetcher.  The operation will be continued
-  // in the OnURLFetchComplete callback.
+  // Start fetching the URL with the fetcher. The delegate is notified
+  // asynchronously when done.
   void Start(Profile* profile);
 
   // Methods inherited from URLFetcherDelegate
@@ -56,7 +44,8 @@ class NotificationBitmapFetcher
   // denotes the number of bytes received up to the call, and |total| is the
   // expected total size of the response (or -1 if not determined).
   virtual void OnURLFetchDownloadProgress(const net::URLFetcher* source,
-                                          int64 current, int64 total) OVERRIDE;
+                                          int64 current,
+                                          int64 total) OVERRIDE;
 
   // Methods inherited from ImageDecoder::Delegate
 
@@ -70,15 +59,17 @@ class NotificationBitmapFetcher
   virtual void OnDecodeImageFailed(const ImageDecoder* decoder) OVERRIDE;
 
  private:
+  // Alerts the delegate that a failure occurred.
+  void ReportFailure();
+
   scoped_ptr<net::URLFetcher> url_fetcher_;
   scoped_refptr<ImageDecoder> image_decoder_;
   const GURL url_;
-  scoped_ptr<SkBitmap> bitmap_;
-  NotificationBitmapFetcherDelegate* const delegate_;
+  BitmapFetcherDelegate* const delegate_;
 
-  DISALLOW_COPY_AND_ASSIGN(NotificationBitmapFetcher);
+  DISALLOW_COPY_AND_ASSIGN(BitmapFetcher);
 };
 
-}  // namespace notifier
+}  // namespace chrome
 
-#endif  // CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_NOTIFICATION_BITMAP_FETCHER_H_
+#endif  // CHROME_BROWSER_BITMAP_FETCHER_H_

chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher_browsertest.cc → chrome/browser/bitmap_fetcher_browsertest.cc

-// Copyright 2013 The Chromium Authors. All rights reserved.
+// 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 "chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher.h"
 
 #include "base/compiler_specific.h"
 #include "chrome/browser/ui/browser.h"
@@ -23,20 +23,17 @@ const bool kAsyncCall = true;
 const bool kSyncCall = false;
 }  // namespace
 
-namespace notifier {
+namespace chrome {
 
-// Class to catch events from the NotificationBitmapFetcher for testing.
-class NotificationBitmapFetcherTestDelegate
-    : public NotificationBitmapFetcherDelegate {
+// Class to catch events from the BitmapFetcher for testing.
+class BitmapFetcherTestDelegate : public BitmapFetcherDelegate {
  public:
-  explicit NotificationBitmapFetcherTestDelegate(bool async)
+  explicit BitmapFetcherTestDelegate(bool async)
       : called_(false), success_(false), async_(async) {}
 
-  virtual ~NotificationBitmapFetcherTestDelegate() {
-    EXPECT_TRUE(called_);
-  }
+  virtual ~BitmapFetcherTestDelegate() { EXPECT_TRUE(called_); }
 
-  // Method inherited from NotificationBitmapFetcherDelegate.
+  // Method inherited from BitmapFetcherDelegate.
   virtual void OnFetchComplete(const GURL url,
                                const SkBitmap* bitmap) OVERRIDE {
     called_ = true;
@@ -63,10 +60,10 @@ class NotificationBitmapFetcherTestDelegate
   bool async_;
   SkBitmap bitmap_;
 
-  DISALLOW_COPY_AND_ASSIGN(NotificationBitmapFetcherTestDelegate);
+  DISALLOW_COPY_AND_ASSIGN(BitmapFetcherTestDelegate);
 };
 
-class NotificationBitmapFetcherBrowserTest : public InProcessBrowserTest {
+class BitmapFetcherBrowserTest : public InProcessBrowserTest {
  public:
   virtual void SetUp() OVERRIDE {
     url_fetcher_factory_.reset(new net::FakeURLFetcherFactory(NULL));
@@ -87,8 +84,7 @@ class NotificationBitmapFetcherBrowserTest : public InProcessBrowserTest {
 // --single-process.  The reason is that the sandbox does not get created
 // for us by the test process if --single-process is used.
 
-IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
-                       MAYBE_StartTest) {
+IN_PROC_BROWSER_TEST_F(BitmapFetcherBrowserTest, MAYBE_StartTest) {
   GURL url("http://example.com/this-should-work");
 
   // Put some realistic looking bitmap data into the url_fetcher.
@@ -107,12 +103,12 @@ IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
   std::string image_string(compressed.begin(), compressed.end());
 
   // Set up a delegate to wait for the callback.
-  NotificationBitmapFetcherTestDelegate delegate(kAsyncCall);
+  BitmapFetcherTestDelegate delegate(kAsyncCall);
 
-  NotificationBitmapFetcher fetcher(url, &delegate);
+  BitmapFetcher fetcher(url, &delegate);
 
-  url_fetcher_factory_->SetFakeResponse(url, image_string, net::HTTP_OK,
-                                        net::URLRequestStatus::SUCCESS);
+  url_fetcher_factory_->SetFakeResponse(
+      url, image_string, net::HTTP_OK, net::URLRequestStatus::SUCCESS);
 
   // We expect that the image decoder will get called and return
   // an image in a callback to OnImageDecoded().
@@ -128,8 +124,7 @@ IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
   EXPECT_TRUE(gfx::BitmapsAreEqual(image, found_image));
 }
 
-IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
-                       OnImageDecodedTest) {
+IN_PROC_BROWSER_TEST_F(BitmapFetcherBrowserTest, OnImageDecodedTest) {
   GURL url("http://example.com/this-should-work-as-well");
   SkBitmap image;
 
@@ -138,9 +133,9 @@ IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
   image.allocPixels();
   image.eraseColor(SK_ColorGREEN);
 
-  NotificationBitmapFetcherTestDelegate delegate(kSyncCall);
+  BitmapFetcherTestDelegate delegate(kSyncCall);
 
-  NotificationBitmapFetcher fetcher(url, &delegate);
+  BitmapFetcher fetcher(url, &delegate);
 
   fetcher.OnImageDecoded(NULL, image);
 
@@ -151,16 +146,15 @@ IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
   EXPECT_TRUE(gfx::BitmapsAreEqual(image, delegate.bitmap()));
 }
 
-IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
-                       OnURLFetchFailureTest) {
+IN_PROC_BROWSER_TEST_F(BitmapFetcherBrowserTest, OnURLFetchFailureTest) {
   GURL url("http://example.com/this-should-be-fetch-failure");
 
   // We intentionally put no data into the bitmap to simulate a failure.
 
   // Set up a delegate to wait for the callback.
-  NotificationBitmapFetcherTestDelegate delegate(kAsyncCall);
+  BitmapFetcherTestDelegate delegate(kAsyncCall);
 
-  NotificationBitmapFetcher fetcher(url, &delegate);
+  BitmapFetcher fetcher(url, &delegate);
 
   url_fetcher_factory_->SetFakeResponse(url,
                                         std::string(),
@@ -182,14 +176,14 @@ IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
 #define MAYBE_HandleImageFailedTest HandleImageFailedTest
 #endif
 
-IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
-                       MAYBE_HandleImageFailedTest) {
+IN_PROC_BROWSER_TEST_F(BitmapFetcherBrowserTest, MAYBE_HandleImageFailedTest) {
   GURL url("http://example.com/this-should-be-a-decode-failure");
-  NotificationBitmapFetcherTestDelegate delegate(kAsyncCall);
-  NotificationBitmapFetcher fetcher(url, &delegate);
-  url_fetcher_factory_->SetFakeResponse(
-      url, std::string("Not a real bitmap"),
-      net::HTTP_OK, net::URLRequestStatus::SUCCESS);
+  BitmapFetcherTestDelegate delegate(kAsyncCall);
+  BitmapFetcher fetcher(url, &delegate);
+  url_fetcher_factory_->SetFakeResponse(url,
+                                        std::string("Not a real bitmap"),
+                                        net::HTTP_OK,
+                                        net::URLRequestStatus::SUCCESS);
 
   fetcher.Start(browser()->profile());
 
@@ -199,4 +193,4 @@ IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
   EXPECT_FALSE(delegate.success());
 }
 
-}  // namespace notifier
+}  // namespace chrome

chrome/browser/bitmap_fetcher_delegate.h

+// 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 CHROME_BROWSER_BITMAP_FETCHER_DELEGATE_H_
+#define CHROME_BROWSER_BITMAP_FETCHER_DELEGATE_H_
+
+#include "url/gurl.h"
+
+class SkBitmap;
+
+namespace chrome {
+
+// A delegate interface for users of BitmapFetcher.
+class BitmapFetcherDelegate {
+ public:
+  BitmapFetcherDelegate() {}
+
+  // This will be called when the bitmap has been requested, whether or not the
+  // request succeeds.  |url| is the URL that was originally fetched so we can
+  // match up the bitmap with a specific request.  |bitmap| may be NULL if the
+  // image fails to be downloaded or decoded.
+  virtual void OnFetchComplete(const GURL url, const SkBitmap* bitmap) = 0;
+
+ protected:
+  virtual ~BitmapFetcherDelegate() {}
+
+  DISALLOW_COPY_AND_ASSIGN(BitmapFetcherDelegate);
+};
+
+}  // namespace chrome
+
+#endif  // CHROME_BROWSER_BITMAP_FETCHER_DELEGATE_H_

chrome/browser/notifications/sync_notifier/synced_notification.cc

@@ -211,7 +211,7 @@ void SyncedNotification::QueueBitmapFetchJobs(
 
 void SyncedNotification::StartBitmapFetch() {
   // Now that we have queued and counted them all, start the fetching.
-  ScopedVector<NotificationBitmapFetcher>::iterator iter;
+  ScopedVector<chrome::BitmapFetcher>::iterator iter;
   for (iter = fetchers_.begin(); iter != fetchers_.end(); ++iter) {
     (*iter)->Start(profile_);
   }
@@ -219,14 +219,14 @@ void SyncedNotification::StartBitmapFetch() {
 
 void SyncedNotification::AddBitmapToFetchQueue(const GURL& url) {
   // Check for dups, ignore any request for a dup.
-  ScopedVector<NotificationBitmapFetcher>::iterator iter;
+  ScopedVector<chrome::BitmapFetcher>::iterator iter;
   for (iter = fetchers_.begin(); iter != fetchers_.end(); ++iter) {
     if ((*iter)->url() == url)
       return;
   }
 
   if (url.is_valid()) {
-    fetchers_.push_back(new NotificationBitmapFetcher(url, this));
+    fetchers_.push_back(new chrome::BitmapFetcher(url, this));
     DVLOG(2) << __FUNCTION__ << "Pushing bitmap " << url;
   }
 }

chrome/browser/notifications/sync_notifier/synced_notification.h

@@ -14,7 +14,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
-#include "chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher.h"
 #include "sync/api/sync_data.h"
 #include "sync/protocol/sync.pb.h"
 #include "ui/gfx/image/image.h"
@@ -31,7 +31,7 @@ namespace notifier {
 
 class ChromeNotifierService;
 
-class SyncedNotification : public NotificationBitmapFetcherDelegate {
+class SyncedNotification : public chrome::BitmapFetcherDelegate {
  public:
   explicit SyncedNotification(const syncer::SyncData& sync_data);
 
@@ -110,7 +110,7 @@ class SyncedNotification : public NotificationBitmapFetcherDelegate {
   // Helper function to mark a notification as read or dismissed.
   void SetReadState(const ReadState& read_state);
 
-  // Method inherited from NotificationBitmapFetcher delegate.
+  // Method inherited from BitmapFetcher delegate.
   virtual void OnFetchComplete(const GURL url, const SkBitmap* bitmap) OVERRIDE;
 
   // If this bitmap has a valid GURL, create a fetcher for it.
@@ -124,7 +124,7 @@ class SyncedNotification : public NotificationBitmapFetcherDelegate {
   ChromeNotifierService* notifier_service_;
   Profile* profile_;
   bool toast_state_;
-  ScopedVector<NotificationBitmapFetcher> fetchers_;
+  ScopedVector<chrome::BitmapFetcher> fetchers_;
   gfx::Image app_icon_bitmap_;
   gfx::Image sender_bitmap_;
   gfx::Image image_bitmap_;

chrome/chrome_browser.gypi

@@ -289,6 +289,9 @@
         'browser/background/background_mode_manager_gtk.cc',
         'browser/background/background_mode_manager_mac.mm',
         'browser/background/background_mode_manager_win.cc',
+        'browser/bitmap_fetcher.cc',
+        'browser/bitmap_fetcher.h',
+        'browser/bitmap_fetcher_delegate.h',
         'browser/bookmarks/base_bookmark_model_observer.cc',
         'browser/bookmarks/base_bookmark_model_observer.h',
         'browser/bookmarks/bookmark_codec.cc',
@@ -1432,8 +1435,6 @@
         'browser/notifications/sync_notifier/chrome_notifier_service.h',
         'browser/notifications/sync_notifier/chrome_notifier_service_factory.cc',
         'browser/notifications/sync_notifier/chrome_notifier_service_factory.h',
-        'browser/notifications/sync_notifier/notification_bitmap_fetcher.cc',
-        'browser/notifications/sync_notifier/notification_bitmap_fetcher.h',
         'browser/notifications/sync_notifier/synced_notification.cc',
         'browser/notifications/sync_notifier/synced_notification.h',
         'browser/notifications/sync_notifier/welcome_delegate.cc',

chrome/chrome_tests.gypi

@@ -1005,6 +1005,7 @@
         'browser/autofill/autofill_driver_impl_browsertest.cc',
         'browser/autofill/form_structure_browsertest.cc',
         'browser/autofill/risk/fingerprint_browsertest.cc',
+        'browser/bitmap_fetcher_browsertest.cc',
         'browser/browser_encoding_browsertest.cc',
         'browser/browsing_data/browsing_data_database_helper_browsertest.cc',
         'browser/browsing_data/browsing_data_helper_browsertest.h',
@@ -1364,7 +1365,6 @@
         'browser/net/websocket_browsertest.cc',
         'browser/notifications/login_state_notification_blocker_chromeos_browsertest.cc',
         'browser/notifications/message_center_notifications_browsertest.cc',
-        'browser/notifications/sync_notifier/notification_bitmap_fetcher_browsertest.cc',
         'browser/notifications/sync_notifier/sync_notifier_test_utils.cc',
         'browser/notifications/sync_notifier/sync_notifier_test_utils.h',
         'browser/password_manager/password_manager_browsertest.cc',