Hookup NotificationBitmapFetcher for SyncedNotifications

Changes to have ChromeNotifierService and SyncedNotifiation use bitmap fetching.
The NotificationBitmapFetcher is committed, this change hooks up the rest of the
synced notification system to call it.

BUG=246341

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

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

9 files changed, 280 insertions, 32 deletions

diff --git a/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc b/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc
index 8432999..be0c511 100644
--- a/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc
+++ b/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc
@@ -21,14 +21,15 @@
 
 namespace notifier {
 
+bool ChromeNotifierService::avoid_bitmap_fetching_for_test_ = false;
+
 ChromeNotifierService::ChromeNotifierService(Profile* profile,
                                              NotificationUIManager* manager)
     : profile_(profile), notification_manager_(manager) {}
 ChromeNotifierService::~ChromeNotifierService() {}
 
 // Methods from BrowserContextKeyedService.
-void ChromeNotifierService::Shutdown() {
-}
+void ChromeNotifierService::Shutdown() {}
 
 // syncer::SyncableService implementation.
 
@@ -275,8 +276,19 @@ void ChromeNotifierService::Add(scoped_ptr<SyncedNotification> notification) {
   // Take ownership of the object and put it into our local storage.
   notification_data_.push_back(notification.release());
 
-  // Get the contained bitmaps, and show the notification once we have them.
-  notification_copy->Show(notification_manager_, this, profile_);
+  // Set up to fetch the bitmaps.
+  notification_copy->QueueBitmapFetchJobs(notification_manager_,
+                                          this,
+                                          profile_);
+
+  // Our tests cannot use the network for reliability reasons.
+  if (avoid_bitmap_fetching_for_test_) {
+    return;
+  }
+
+  // Start the bitmap fetching, Show() will be called when the last bitmap
+  // either arrives or times out.
+  notification_copy->StartBitmapFetch();
 }
 
 }  // namespace notifier
diff --git a/chrome/browser/notifications/sync_notifier/chrome_notifier_service.h b/chrome/browser/notifications/sync_notifier/chrome_notifier_service.h
index ddbfc3d..270157f 100644
--- a/chrome/browser/notifications/sync_notifier/chrome_notifier_service.h
+++ b/chrome/browser/notifications/sync_notifier/chrome_notifier_service.h
@@ -64,6 +64,14 @@ class ChromeNotifierService : public syncer::SyncableService,
     Add(notification.Pass());
   }
 
+  // If we allow the tests to do bitmap fetching, they will attempt to fetch
+  // a URL from the web, which will fail.  We can already test the majority
+  // of what we want without also trying to fetch bitmaps.  Other tests will
+  // cover bitmap fetching.
+  static void set_avoid_bitmap_fetching_for_test(bool avoid) {
+    avoid_bitmap_fetching_for_test_ = avoid;
+  }
+
  private:
   // Add a notification to our list.  This takes ownership of the pointer.
   void Add(scoped_ptr<notifier::SyncedNotification> notification);
@@ -75,11 +83,14 @@ class ChromeNotifierService : public syncer::SyncableService,
   Profile* const profile_;
   NotificationUIManager* const notification_manager_;
   scoped_ptr<syncer::SyncChangeProcessor> sync_processor_;
+  static bool avoid_bitmap_fetching_for_test_;
 
   // TODO(petewil): consider whether a map would better suit our data.
   // If there are many entries, lookup time may trump locality of reference.
   ScopedVector<notifier::SyncedNotification> notification_data_;
 
+  friend class ChromeNotifierServiceTest;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeNotifierService);
 };
 
diff --git a/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc b/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc
index 79f5aef..d3037f1 100644
--- a/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc
+++ b/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc
@@ -251,7 +251,9 @@ class ChromeNotifierServiceTest : public testing::Test {
   virtual ~ChromeNotifierServiceTest() {}
 
   // Methods from testing::Test.
-  virtual void SetUp() {}
+  virtual void SetUp() {
+    ChromeNotifierService::set_avoid_bitmap_fetching_for_test(true);
+  }
   virtual void TearDown() {}
 
   TestChangeProcessor* processor() {
@@ -571,6 +573,7 @@ TEST_F(ChromeNotifierServiceTest, ProcessSyncChangesEmptyModel) {
   // We initially have no data.
   StubNotificationUIManager notification_manager;
   ChromeNotifierService notifier(NULL, &notification_manager);
+  notifier.set_avoid_bitmap_fetching_for_test(true);
 
   notifier.MergeDataAndStartSyncing(
       SYNCED_NOTIFICATIONS,
@@ -602,6 +605,7 @@ TEST_F(ChromeNotifierServiceTest, ProcessSyncChangesEmptyModel) {
 TEST_F(ChromeNotifierServiceTest, LocalRemoteBothNonEmptyNoOverlap) {
   StubNotificationUIManager notification_manager;
   ChromeNotifierService notifier(NULL, &notification_manager);
+  notifier.set_avoid_bitmap_fetching_for_test(true);
 
   // Create some local fake data.
   scoped_ptr<SyncedNotification> n1(CreateNotification(
@@ -665,6 +669,7 @@ TEST_F(ChromeNotifierServiceTest, LocalRemoteBothNonEmptyNoOverlap) {
 TEST_F(ChromeNotifierServiceTest, ModelAssocBothNonEmptyReadMismatch1) {
   StubNotificationUIManager notification_manager;
   ChromeNotifierService notifier(NULL, &notification_manager);
+  notifier.set_avoid_bitmap_fetching_for_test(true);
 
   // Create some local fake data.
   scoped_ptr<SyncedNotification> n1(CreateNotification(
@@ -705,6 +710,7 @@ TEST_F(ChromeNotifierServiceTest, ModelAssocBothNonEmptyReadMismatch1) {
 TEST_F(ChromeNotifierServiceTest, ModelAssocBothNonEmptyReadMismatch2) {
   StubNotificationUIManager notification_manager;
   ChromeNotifierService notifier(NULL, &notification_manager);
+  notifier.set_avoid_bitmap_fetching_for_test(true);
 
   // Create some local fake data.
   scoped_ptr<SyncedNotification> n1(CreateNotification(
diff --git a/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.cc b/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.cc
index a8767e7..b143732 100644
--- a/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.cc
+++ b/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.h"
 
+#include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/url_request/url_fetcher.h"
 
@@ -16,10 +17,15 @@ NotificationBitmapFetcher::NotificationBitmapFetcher(
 
 NotificationBitmapFetcher::~NotificationBitmapFetcher() {}
 
-void NotificationBitmapFetcher::Start() {
+void NotificationBitmapFetcher::Start(Profile* profile) {
   if (url_fetcher_ == NULL) {
     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.
+    url_fetcher_->SetRequestContext(profile->GetRequestContext());
   }
   url_fetcher_->Start();
 }
@@ -73,14 +79,14 @@ void NotificationBitmapFetcher::OnImageDecoded(
   decoded_image.deepCopyTo(bitmap_.get(), decoded_image.getConfig());
 
   // Report success.
-  delegate_->OnFetchComplete(bitmap_.get());
+  delegate_->OnFetchComplete(url_, bitmap_.get());
 }
 
 void NotificationBitmapFetcher::OnDecodeImageFailed(
     const ImageDecoder* decoder) {
 
   // Report failure.
-  delegate_->OnFetchComplete(NULL);
+  delegate_->OnFetchComplete(url_, NULL);
 }
 
 }  // namespace notifier
diff --git a/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.h b/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.h
index fb206af..eb95540 100644
--- a/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.h
+++ b/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.h
@@ -15,13 +15,17 @@ namespace net {
 class URLFetcher;
 }  // namespace net
 
+class Profile;
+
 namespace notifier {
 
 // A delegate interface for users of NotificationBitmapFetcher.
 class NotificationBitmapFetcherDelegate {
  public:
-  // This will be called when the bitmap has been fetched, successfully or not.
-  virtual void OnFetchComplete(const SkBitmap* bitmap) = 0;
+  // 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() {}
@@ -36,9 +40,11 @@ class NotificationBitmapFetcher
       NotificationBitmapFetcherDelegate* delegate);
   virtual ~NotificationBitmapFetcher();
 
+  GURL url() const { return url_; }
+
   // Start fetching the URL with the fetcher.  The operation will be continued
   // in the OnURLFetchComplete callback.
-  void Start();
+  void Start(Profile* profile);
 
   // Methods inherited from URLFetcherDelegate
 
diff --git a/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher_browsertest.cc b/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher_browsertest.cc
index 11b1da8..d963544 100644
--- a/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher_browsertest.cc
+++ b/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher_browsertest.cc
@@ -18,8 +18,6 @@
 #include "ui/gfx/skia_util.h"
 
 namespace {
-// FF0000FF is 100% alpha and max green.(A, R, B, G)
-uint32 kMaxGreen = 0xFF0000FF;
 const bool kAsyncCall = true;
 const bool kSyncCall = false;
 }  // namespace
@@ -36,7 +34,9 @@ class NotificationBitmapFetcherTestDelegate
   virtual ~NotificationBitmapFetcherTestDelegate() {}
 
   // Method inherited from NotificationBitmapFetcherDelegate.
-  virtual void OnFetchComplete(const SkBitmap* bitmap) OVERRIDE {
+  virtual void OnFetchComplete(const GURL url,
+                               const SkBitmap* bitmap) OVERRIDE {
+    url_ = url;
     if (NULL != bitmap) {
       success_ = true;
       bitmap->deepCopyTo(&bitmap_, bitmap->getConfig());
@@ -48,10 +48,12 @@ class NotificationBitmapFetcherTestDelegate
     }
   }
 
+  GURL url() const { return url_; }
   bool success() const { return success_; }
   const SkBitmap& bitmap() const { return bitmap_; }
 
  private:
+  GURL url_;
   bool success_;
   bool async_;
   SkBitmap bitmap_;
@@ -75,8 +77,7 @@ IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
   // Put a real bitmap into "image".  2x2 bitmap of green 32 bit pixels.
   image.setConfig(SkBitmap::kARGB_8888_Config, 2, 2);
   image.allocPixels();
-  SkColor c = kMaxGreen;
-  image.eraseColor(c);
+  image.eraseColor(SK_ColorGREEN);
 
   // Encode the bits as a PNG.
   std::vector<unsigned char> compressed;
@@ -91,12 +92,12 @@ IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
   NotificationBitmapFetcher fetcher(url, &delegate);
 
   scoped_ptr<net::URLFetcher> url_fetcher(new net::FakeURLFetcher(
-      url, &fetcher, image_string, /*success=*/true));
+      url, &fetcher, image_string, true));
   fetcher.SetURLFetcherForTest(url_fetcher.Pass());
 
   // We expect that the image decoder will get called and return
   // an image in a callback to OnImageDecoded().
-  fetcher.Start();
+  fetcher.Start(NULL);
 
   // Blocks until test delegate is notified via a callback.
   content::RunMessageLoop();
@@ -116,8 +117,7 @@ IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
   // Put a real bitmap into "image".  2x2 bitmap of green 16 bit pixels.
   image.setConfig(SkBitmap::kARGB_8888_Config, 2, 2);
   image.allocPixels();
-  SkColor c = kMaxGreen;
-  image.eraseColor(c);
+  image.eraseColor(SK_ColorGREEN);
 
   NotificationBitmapFetcherTestDelegate delegate(kSyncCall);
 
@@ -144,10 +144,10 @@ IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
   NotificationBitmapFetcher fetcher(url, &delegate);
 
   scoped_ptr<net::URLFetcher> url_fetcher(new net::FakeURLFetcher(
-      url, &fetcher, std::string(), /*success=*/ false));
+      url, &fetcher, std::string(), false));
   fetcher.SetURLFetcherForTest(url_fetcher.Pass());
 
-  fetcher.Start();
+  fetcher.Start(NULL);
 
   // Blocks until test delegate is notified via a callback.
   content::RunMessageLoop();
@@ -161,10 +161,10 @@ IN_PROC_BROWSER_TEST_F(NotificationBitmapFetcherBrowserTest,
   NotificationBitmapFetcherTestDelegate delegate(kAsyncCall);
   NotificationBitmapFetcher fetcher(url, &delegate);
   scoped_ptr<net::URLFetcher> url_fetcher(new net::FakeURLFetcher(
-      url, &fetcher, std::string("Not a real bitmap"), /*success=*/ true));
+      url, &fetcher, std::string("Not a real bitmap"), true));
   fetcher.SetURLFetcherForTest(url_fetcher.Pass());
 
-  fetcher.Start();
+  fetcher.Start(NULL);
 
   // Blocks until test delegate is notified via a callback.
   content::RunMessageLoop();
diff --git a/chrome/browser/notifications/sync_notifier/synced_notification.cc b/chrome/browser/notifications/sync_notifier/synced_notification.cc
index 99fc4ff..deafcea 100644
--- a/chrome/browser/notifications/sync_notifier/synced_notification.cc
+++ b/chrome/browser/notifications/sync_notifier/synced_notification.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
 #include "chrome/browser/notifications/sync_notifier/chrome_notifier_delegate.h"
+#include "content/public/browser/browser_thread.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/protocol/synced_notification_specifics.pb.h"
 #include "ui/gfx/image/image.h"
@@ -42,7 +43,11 @@ COMPILE_ASSERT(static_cast<sync_pb::CoalescedSyncedNotification_ReadState>(
                sync_pb::CoalescedSyncedNotification_ReadState_DISMISSED,
                local_enum_must_match_protobuf_enum);
 
-SyncedNotification::SyncedNotification(const syncer::SyncData& sync_data) {
+SyncedNotification::SyncedNotification(const syncer::SyncData& sync_data)
+    :   notification_manager_(NULL),
+        notifier_service_(NULL),
+        profile_(NULL),
+        active_fetcher_count_(0)  {
   Update(sync_data);
 }
 
@@ -59,6 +64,95 @@ sync_pb::EntitySpecifics SyncedNotification::GetEntitySpecifics() const {
   return entity_specifics;
 }
 
+void SyncedNotification::OnFetchComplete(const GURL url,
+                                         const SkBitmap* bitmap) {
+  // TODO(petewil): Add timeout mechanism in case bitmaps take too long.  Do we
+  // already have one built into URLFetcher?
+  // Make sure we are on the thread we expect.
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  // Match the incoming bitmaps to URLs.  In case this is a dup, make sure to
+  // try all potentially matching urls.
+  if (GetAppIconUrl() == url && bitmap != NULL) {
+    app_icon_bitmap_ = gfx::Image::CreateFrom1xBitmap(*bitmap);
+  }
+  if (GetImageUrl() == url && bitmap != NULL) {
+    image_bitmap_ = gfx::Image::CreateFrom1xBitmap(*bitmap);
+  }
+  if (GetButtonOneIconUrl() == url.spec() && bitmap != NULL) {
+    button_one_bitmap_ = gfx::Image::CreateFrom1xBitmap(*bitmap);
+  }
+  if (GetButtonTwoIconUrl() == url.spec() && bitmap != NULL) {
+    button_two_bitmap_ = gfx::Image::CreateFrom1xBitmap(*bitmap);
+  }
+
+  // Count off the bitmaps as they arrive.
+  --active_fetcher_count_;
+  DCHECK_GE(active_fetcher_count_, 0);
+  // See if all bitmaps are accounted for, if so call Show.
+  if (active_fetcher_count_ == 0) {
+    Show(notification_manager_, notifier_service_, profile_);
+  }
+}
+
+void SyncedNotification::QueueBitmapFetchJobs(
+    NotificationUIManager* notification_manager,
+    ChromeNotifierService* notifier_service,
+    Profile* profile) {
+
+  // If we are not using the MessageCenter, call show now, and the existing
+  // code will handle the bitmap fetch for us.
+  if (!UseRichNotifications()) {
+    Show(notification_manager, notifier_service, profile);
+    return;
+  }
+
+  // Save off the arguments for the call to Show.
+  notification_manager_ = notification_manager;
+  notifier_service_ = notifier_service;
+  profile_ = profile;
+  DCHECK_EQ(active_fetcher_count_, 0);
+
+  // Get the URLs that we might need to fetch from Synced Notification.
+  // TODO(petewil): clean up the fact that icon and image return a GURL, and
+  // button urls return a string.
+  // TODO(petewil): Eventually refactor this to accept an arbitrary number of
+  // button URLs.
+
+  // If the URL is non-empty, add it to our queue of URLs to fetch.
+  AddBitmapToFetchQueue(GetAppIconUrl());
+  AddBitmapToFetchQueue(GetImageUrl());
+  AddBitmapToFetchQueue(GURL(GetButtonOneIconUrl()));
+  AddBitmapToFetchQueue(GURL(GetButtonTwoIconUrl()));
+
+  // If there are no bitmaps, call show now.
+  if (active_fetcher_count_ == 0) {
+    Show(notification_manager, notifier_service, profile);
+  }
+}
+
+void SyncedNotification::StartBitmapFetch() {
+  // Now that we have queued and counted them all, start the fetching.
+  ScopedVector<NotificationBitmapFetcher>::iterator iter;
+  for (iter = fetchers_.begin(); iter != fetchers_.end(); ++iter) {
+    (*iter)->Start(profile_);
+  }
+}
+
+void SyncedNotification::AddBitmapToFetchQueue(const GURL& url) {
+  // Check for dups, ignore any request for a dup.
+  ScopedVector<NotificationBitmapFetcher>::iterator iter;
+  for (iter = fetchers_.begin(); iter != fetchers_.end(); ++iter) {
+    if ((*iter)->url() == url)
+      return;
+  }
+
+  if (url.is_valid()) {
+    ++active_fetcher_count_;
+    fetchers_.push_back(new NotificationBitmapFetcher(url, this));
+  }
+}
+
 void SyncedNotification::Show(NotificationUIManager* notification_manager,
                               ChromeNotifierService* notifier_service,
                               Profile* profile) {
@@ -159,7 +253,6 @@ void SyncedNotification::Show(NotificationUIManager* notification_manager,
                                  delegate.get());
 
     notification_manager->Add(ui_notification, profile);
-
   }
 
   DVLOG(1) << "Showing Synced Notification! " << heading << " " << text
diff --git a/chrome/browser/notifications/sync_notifier/synced_notification.h b/chrome/browser/notifications/sync_notifier/synced_notification.h
index 307e651..50868f8 100644
--- a/chrome/browser/notifications/sync_notifier/synced_notification.h
+++ b/chrome/browser/notifications/sync_notifier/synced_notification.h
@@ -11,10 +11,14 @@
 
 #include <string>
 
+#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 "googleurl/src/gurl.h"
 #include "sync/api/sync_data.h"
 #include "sync/protocol/sync.pb.h"
+#include "ui/gfx/image/image.h"
 
 namespace sync_pb {
 class SyncedNotificationSpecifics;
@@ -27,11 +31,11 @@ namespace notifier {
 
 class ChromeNotifierService;
 
-class SyncedNotification {
+class SyncedNotification : public NotificationBitmapFetcherDelegate {
  public:
   explicit SyncedNotification(const syncer::SyncData& sync_data);
 
-  ~SyncedNotification();
+  virtual ~SyncedNotification();
 
   enum ReadState {
     kUnread = 1,
@@ -76,6 +80,15 @@ class SyncedNotification {
 
   void NotificationHasBeenDismissed();
 
+  // Fill up the queue of bitmaps to fetch.
+  void QueueBitmapFetchJobs(NotificationUIManager* notification_manager,
+                            ChromeNotifierService* notifier_service,
+                            Profile* profile);
+
+  // Start the bitmap fetching.  When it is complete, the callback
+  // will call Show().
+  void StartBitmapFetch();
+
   // Display the notification in the notification center
   void Show(NotificationUIManager* notification_manager,
             ChromeNotifierService* notifier_service,
@@ -89,10 +102,26 @@ class SyncedNotification {
   // Helper function to mark a notification as read or dismissed.
   void SetReadState(const ReadState& read_state);
 
-  // Parsing functions to get this information out of the sync_data and into
-  // our local variables.
+  // Method inherited from NotificationBitmapFetcher delegate.
+  virtual void OnFetchComplete(const GURL url, const SkBitmap* bitmap) OVERRIDE;
+
+  // If this bitmap has a valid GURL, create a fetcher for it.
+  void AddBitmapToFetchQueue(const GURL& gurl);
 
   sync_pb::SyncedNotificationSpecifics specifics_;
+  NotificationUIManager* notification_manager_;
+  ChromeNotifierService* notifier_service_;
+  Profile* profile_;
+  ScopedVector<NotificationBitmapFetcher> fetchers_;
+  int active_fetcher_count_;
+  gfx::Image app_icon_bitmap_;
+  gfx::Image image_bitmap_;
+  gfx::Image button_one_bitmap_;
+  gfx::Image button_two_bitmap_;
+
+  FRIEND_TEST_ALL_PREFIXES(SyncedNotificationTest, AddBitmapToFetchQueueTest);
+  FRIEND_TEST_ALL_PREFIXES(SyncedNotificationTest, OnFetchCompleteTest);
+  FRIEND_TEST_ALL_PREFIXES(SyncedNotificationTest, QueueBitmapsTest);
 
   DISALLOW_COPY_AND_ASSIGN(SyncedNotification);
 };
diff --git a/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc b/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc
index 4163d8c..7e78ff0 100644
--- a/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc
+++ b/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc
@@ -5,16 +5,20 @@
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/notifications/notification.h"
 #include "chrome/browser/notifications/notification_test_util.h"
 #include "chrome/browser/notifications/notification_ui_manager.h"
 #include "chrome/browser/notifications/sync_notifier/synced_notification.h"
 #include "chrome/browser/profiles/profile.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/test/test_browser_thread.h"
 #include "sync/api/sync_data.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/protocol/synced_notification_specifics.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/message_center/message_center_util.h"
 #include "ui/message_center/notification_types.h"
 
@@ -24,7 +28,6 @@ using sync_pb::EntitySpecifics;
 using sync_pb::SyncedNotificationSpecifics;
 
 namespace {
-
 const uint64 kFakeCreationTime = 42;
 const int kProtobufPriority = static_cast<int>(
     sync_pb::CoalescedSyncedNotification_Priority_LOW);
@@ -77,6 +80,8 @@ const sync_pb::CoalescedSyncedNotification_ReadState kDismissed =
     sync_pb::CoalescedSyncedNotification_ReadState_DISMISSED;
 }  // namespace
 
+namespace notifier {
+
 // Stub out the NotificationUIManager for unit testing.
 class StubNotificationUIManager : public NotificationUIManager {
  public:
@@ -144,7 +149,8 @@ class StubNotificationUIManager : public NotificationUIManager {
 
 class SyncedNotificationTest : public testing::Test {
  public:
-  SyncedNotificationTest() {}
+  SyncedNotificationTest()
+      : ui_thread_(content::BrowserThread::UI, &message_loop_)   {}
   virtual ~SyncedNotificationTest() {}
 
   // Methods from testing::Test.
@@ -421,6 +427,9 @@ class SyncedNotificationTest : public testing::Test {
   }
 
  private:
+  base::MessageLoopForIO message_loop_;
+  content::TestBrowserThread ui_thread_;
+
   DISALLOW_COPY_AND_ASSIGN(SyncedNotificationTest);
 };
 
@@ -609,4 +618,80 @@ TEST_F(SyncedNotificationTest, ShowTest) {
   EXPECT_EQ(UTF8ToUTF16(kContainedMessage3), notification.items()[2].message);
 }
 
+TEST_F(SyncedNotificationTest, AddBitmapToFetchQueueTest) {
+  scoped_ptr<SyncedNotification> notification6;
+  notification6.reset(new SyncedNotification(sync_data1_));
+
+  // Add two bitmaps to the queue.
+  notification6->AddBitmapToFetchQueue(GURL(kIconUrl1));
+  notification6->AddBitmapToFetchQueue(GURL(kIconUrl2));
+
+  EXPECT_EQ(2, notification6->active_fetcher_count_);
+  EXPECT_EQ(GURL(kIconUrl1), notification6->fetchers_[0]->url());
+  EXPECT_EQ(GURL(kIconUrl2), notification6->fetchers_[1]->url());
+
+  notification6->AddBitmapToFetchQueue(GURL(kIconUrl2));
+  EXPECT_EQ(2, notification6->active_fetcher_count_);
+}
+
+TEST_F(SyncedNotificationTest, OnFetchCompleteTest) {
+
+  if (!UseRichNotifications())
+    return;
+
+  StubNotificationUIManager notification_manager;
+
+  // Set up the internal state that FetchBitmaps() would have set.
+  notification1_->notification_manager_ = &notification_manager;
+
+  // Add two bitmaps to the queue for us to match up.
+  notification1_->AddBitmapToFetchQueue(GURL(kIconUrl1));
+  notification1_->AddBitmapToFetchQueue(GURL(kIconUrl2));
+  EXPECT_EQ(2, notification1_->active_fetcher_count_);
+
+  // Put some realistic looking bitmap data into the url_fetcher.
+  SkBitmap bitmap;
+
+  // Put a real bitmap into "bitmap".  2x2 bitmap of green 32 bit pixels.
+  bitmap.setConfig(SkBitmap::kARGB_8888_Config, 2, 2);
+  bitmap.allocPixels();
+  SkColor c = SK_ColorGREEN;
+  bitmap.eraseColor(c);
+
+  notification1_->OnFetchComplete(GURL(kIconUrl1), &bitmap);
+  EXPECT_EQ(1, notification1_->active_fetcher_count_);
+
+  // When we call OnFetchComplete on the second bitmap, show should be called.
+  notification1_->OnFetchComplete(GURL(kIconUrl2), &bitmap);
+  EXPECT_EQ(0, notification1_->active_fetcher_count_);
+
+  // Since we check Show() thoroughly in its own test, we only check cursorily.
+  EXPECT_EQ(message_center::NOTIFICATION_TYPE_IMAGE,
+            notification_manager.notification().type());
+  EXPECT_EQ(kTitle1,
+            UTF16ToUTF8(notification_manager.notification().title()));
+  EXPECT_EQ(kText1,
+            UTF16ToUTF8(notification_manager.notification().message()));
+
+  // TODO(petewil): Check that the bitmap in the notification is what we expect.
+  // This fails today, the type info is different.
+  // EXPECT_TRUE(gfx::BitmapsAreEqual(
+  //     image, notification1_->GetAppIconBitmap()));
+}
+
+TEST_F(SyncedNotificationTest, QueueBitmapsTest) {
+
+  if (!UseRichNotifications())
+    return;
+
+  StubNotificationUIManager notification_manager;
+
+  notification1_->QueueBitmapFetchJobs(&notification_manager, NULL, NULL);
+
+  // There should be 4 urls in the queue, icon, image, and two buttons.
+  EXPECT_EQ(4, notification1_->active_fetcher_count_);
+}
+
 // TODO(petewil): Add a test for a notification being read and or deleted.
+
+}  // namespace notifier