Continue bitmap fetching for notifications.

chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher_unittest.cc

Index: chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher_unittest.cc
diff --git a/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher_unittest.cc b/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..cf4b41d3aca8f987d4e88a5bc975ee5e286bb9fa
--- /dev/null
+++ b/chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher_unittest.cc
@@ -0,0 +1,145 @@
+// Copyright (c) 2013 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 "base/message_loop.h"
+#include "chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/test/test_browser_thread.h"
+#include "net/base/host_port_pair.h"
+#include "net/url_request/test_url_fetcher_factory.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_status.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SKBitmap.h"
+
+namespace {
+// FF0000FF is 100% alpha and max green.(A, R, B, G)
+uint32_t kMaxGreen = 0xFF0000FF;
+
+}  // namespace
+
+namespace net {
+class HttpRequestHeaders;
+class HttpResponseHeaders;
+class URLRequestContextGetter;
+}
+
+namespace notifier {
+
+// Class to catch events from the NotificationBitmapFetcher for testing.
+class NotificationBitmapFetcherTestDelegate
+    : public NotificationBitmapFetcherDelegate {
+ public:
+  NotificationBitmapFetcherTestDelegate()
+      : success_(false), bitmap_(NULL) {}
+
+  ~NotificationBitmapFetcherTestDelegate() {}
+
+  // Method inherited from NotificationBitmapFetcherDelegate
+  void OnFetchComplete(const bool success, SkBitmap* bitmap) {
+    success_ = success;
+    bitmap_ = bitmap;
+  }
+
+  bool success() { return success_; }
+  SkBitmap* bitmap() { return bitmap_; }
+
+ private:
+  bool success_;
+  SkBitmap* bitmap_;
+
+  DISALLOW_COPY_AND_ASSIGN(NotificationBitmapFetcherTestDelegate);
+};
+
+class NotificationBitmapFetcherTest : public testing::Test {
+ public:
+  NotificationBitmapFetcherTest()
+      : ui_thread_(content::BrowserThread::UI, &message_loop_) {}
+
+ private:
+  MessageLoopForIO message_loop_;
+  content::TestBrowserThread ui_thread_;
+};
+
+TEST_F(NotificationBitmapFetcherTest, OnImageDecodedTest) {

[dcheng, 2013/05/28 20:11:34]

It feels strange to have the functional tests split between unit tests and
browser tests. Since the fetcher uses ImageDecoder (which requires a utility
process), it seems logical to just make these all browser tests.

In particular, the tests feel too tightly coupled since it directly invokes
various URLFetcherDelegate and ImageDecoder::Delegate callbacks.

[Pete Williamson, 2013/05/29 18:05:00]

OK, I can easily make these browser tests.  I'm not sure I understand the
problem with coupling though, unit tests are often tightly coupled to the code
to get effective and direct test coverage.  In what way are you thinking it can
be improved?

[dcheng, 2013/05/29 20:40:29]

I think there are two potential pitfalls with this approach:
1) The tests won't catch problems in Start() because they simply don't invoke
it.
2) The code paths taken by the test differ from those of real code. If image
decoding failed, the bitmap fetcher would receive a OnURLFetchComplete callback
followed by a callback to OnDecodeImageFailed. But in the HandleImageFailedTest,
the testing code simply invokes OnDecodeImageFailed directly.

+  GURL url("http://localhost");
+  scoped_ptr<SkBitmap> image(new SkBitmap());
+
+  // Put a real bitmap into "image".  2x2 bitmap of green 16 bit pixels.
+  image->setConfig(SkBitmap::kRGB_565_Config, 2, 2);
+  image->allocPixels();
+  SkColor c = kMaxGreen;
+  image->eraseColor(c);
+  // Test that the image is stored and ready. Pixel [0,0] should be green.
+  EXPECT_EQ(8, image->getSize());
+  EXPECT_EQ(kMaxGreen, image->getColor(0, 0));
+
+  NotificationBitmapFetcherTestDelegate delegate;
+
+  scoped_refptr<NotificationBitmapFetcher> fetcher =
+      new NotificationBitmapFetcher(url, &delegate);
+
+  scoped_ptr<net::URLFetcher> url_fetcher(
+      new net::TestURLFetcher(1, url, fetcher.get()));
+  fetcher->SetURLFetcherForTest(url_fetcher);
+
+  fetcher->OnImageDecoded(NULL, *image.get());
+
+  // Ensure image is marked as succeeded.
+  EXPECT_EQ(true, delegate.success());
+  // Test that the image is stored and ready. Pixel [0,0] should be green.
+  EXPECT_EQ(8, delegate.bitmap()->getSize());
+  EXPECT_EQ(2, delegate.bitmap()->width());
+  EXPECT_EQ(2, delegate.bitmap()->height());
+  EXPECT_TRUE(delegate.bitmap()->getPixels() != NULL);
+  EXPECT_EQ(kMaxGreen, delegate.bitmap()->getColor(0, 0));
+}
+
+TEST_F(NotificationBitmapFetcherTest, OnURLFetchFailureTest) {
+  GURL url("http://localhost");
+
+  // We intentionally put no data into the bitmap to simulate a failure.
+
+  // Set up a delegate to wait for the callback.
+  NotificationBitmapFetcherTestDelegate delegate;
+
+  // The fetcher controls the lifetime of url_fetcher, but we keep a
+  // pointer to it with stub_url_fetcher.
+  scoped_refptr<NotificationBitmapFetcher> fetcher =
+      new NotificationBitmapFetcher(url, &delegate);
+
+  // The fetcher controls the lifetime of url_fetcher, but we keep a
+  // pointer to it with stub_url_fetcher.
+  net::TestURLFetcher* stub_url_fetcher =
+      new net::TestURLFetcher(1, url, fetcher.get());
+  scoped_ptr<net::URLFetcher> url_fetcher(stub_url_fetcher);
+  fetcher->SetURLFetcherForTest(url_fetcher);
+
+  // We expect that the fetch complete notification will be sent, but that the
+  // image will be marked as failed.
+  fetcher->OnURLFetchComplete(stub_url_fetcher);
+
+  EXPECT_FALSE(delegate.success());
+  EXPECT_EQ(NULL, delegate.bitmap());
+}
+
+TEST_F(NotificationBitmapFetcherTest, HandleImageFailedTest) {
+  GURL url("http://localhost");
+  NotificationBitmapFetcherTestDelegate delegate;
+  scoped_refptr<NotificationBitmapFetcher> fetcher =
+      new NotificationBitmapFetcher(url, &delegate);
+
+  scoped_ptr<net::URLFetcher> url_fetcher(
+      new net::TestURLFetcher(1, url, fetcher.get()));
+  fetcher->SetURLFetcherForTest(url_fetcher);
+
+  EXPECT_EQ(false, delegate.success());
+
+  fetcher->OnDecodeImageFailed(NULL);
+
+  EXPECT_EQ(NULL, delegate.bitmap());
+  EXPECT_EQ(false, delegate.success());
+}
+
+}  // namespace notifier