Continue bitmap fetching for notifications.

chrome/browser/notifications/sync_notifier/notification_bitmap_fetcher.h

+// 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.
+
+#ifndef CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_NOTIFICATION_BITMAP_FETCHER_H_
+#define CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_NOTIFICATION_BITMAP_FETCHER_H_
+
+#include "base/gtest_prod_util.h"

[dcheng, 2013/05/30 22:38:09]

This include is no longer used.

+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/image_decoder.h"
+#include "googleurl/src/gurl.h"
+#include "net/url_request/url_fetcher_delegate.h"
+#include "third_party/skia/include/core/SKBitmap.h"
+
+namespace net {
+class URLFetcher;
+}  // namespace net
+
+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;
+
+ protected:
+  virtual ~NotificationBitmapFetcherDelegate() {}
+};
+
+class NotificationBitmapFetcher
+    : public net::URLFetcherDelegate,
+      public ImageDecoder::Delegate,
+      public base::RefCountedThreadSafe<NotificationBitmapFetcher> {

[dcheng, 2013/05/30 22:38:09]

This no longer needs to be refcounted.

[Pete Williamson, 2013/05/31 00:17:39]

Done.

+ public:
+        explicit NotificationBitmapFetcher(

[miket_OOO, 2013/05/30 22:53:20]

This doesn't need to be explicit anymore now that there are two arguments.

[Pete Williamson, 2013/05/31 00:17:39]

Done.

+            const GURL& url,
+            NotificationBitmapFetcherDelegate* delegate);

[dcheng, 2013/05/30 22:38:09]

Style nit: indent is wonky.

[Pete Williamson, 2013/05/31 00:17:39]

Done.

+  ~NotificationBitmapFetcher();

[dcheng, 2013/05/30 22:38:09]

Nit: Explicitly mark destructor virtual. Note that since this (was) a refcounted
object, it should have been private as well, but since that no longer needs to
be the case, it can remain here.

+
+  // Start fetching the URL with the fetcher.  The operation will be continued
+  // in the OnURLFetchComplete callback.
+  void Start();
+
+  // Methods inherited from URLFetcherDelegate
+
+  // This will be called when the URL has been fetched, successfully or not.
+  // Use accessor methods on |source| to get the results.
+  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+
+  // This will be called when some part of the response is read. |current|
+  // 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;
+
+  // Methods inherited from ImageDecoder::Delegate
+
+  // Called when image is decoded. |decoder| is used to identify the image in
+  // case of decoding several images simultaneously.  This will not be called
+  // on the UI thread.
+  virtual void OnImageDecoded(const ImageDecoder* decoder,
+                              const SkBitmap& decoded_image) OVERRIDE;
+
+  // Called when decoding image failed.
+  virtual void OnDecodeImageFailed(const ImageDecoder* decoder) OVERRIDE;
+
+  // Used to pass in a fetcher for dependency injection in tests.
+  void SetURLFetcherForTest(scoped_ptr<net::URLFetcher> url_fetcher);
+
+ private:
+  scoped_ptr<net::URLFetcher> url_fetcher_;
+  scoped_refptr<ImageDecoder> image_decoder_;
+  const GURL url_;
+  scoped_ptr<SkBitmap> bitmap_;

[dcheng, 2013/05/30 22:38:09]

Note that you don't have to include SkBitmap here since it's only being used as
a template parameter to scoped_ptr. A forward declaration would suffice. In the
past, the preference was to forward declare when possible and only #include the
header in the implementation file when needed. The reason for this was (and
still is) compilation speed, since including a header ends up including more
headers which include more headers...

It probably doesn't matter here but just something to keep in mind.

+  NotificationBitmapFetcherDelegate* const delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(NotificationBitmapFetcher);
+};
+
+}  // namespace notifier
+
+#endif  // CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_NOTIFICATION_BITMAP_FETCHER_H_