Turn on and use the AppInfo data.

chrome/browser/notifications/sync_notifier/image_holder.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.
+
+// This class holds the URL to an image and the bitmap for the fetched image,
+// and has code to fetch the bitmap from the URL.

[dewittj, 2014/03/25 22:31:14]

I think that this comment and the one on line 28 should be merged.  No comments
typically appear before the #include section.

[Pete Williamson, 2014/03/26 18:12:55]

I just removed the comment.

+
+#ifndef CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_IMAGE_HOLDER_H_
+#define CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_IMAGE_HOLDER_H_
+
+#include "base/memory/scoped_vector.h"
+#include "chrome/browser/bitmap_fetcher.h"
+#include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_skia.h"
+#include "url/gurl.h"
+
+class Profile;
+
+namespace notifier {
+
+// This provides a callback so the ImageHolder can inform its parent when a
+// bitmap arrives.
+class ImageHolderDelegate {
+ public:
+  virtual void OnFetchComplete() = 0;
+};
+
+// This class encapsulates the action of fetching a bitmap, reporting when it is
+// fetched, and holding onto the bitmap until no longer needed.
+class ImageHolder : public chrome::BitmapFetcherDelegate {
+ public:
+  ImageHolder(const GURL& low_dpi_url,
+              const GURL& high_dpi_url,
+              Profile* profile,
+              ImageHolderDelegate* delegate);
+  virtual ~ImageHolder();
+
+  // Begin fetching of the URLs we have.
+  void StartFetch();
+
+  // Check whether we have a response from the server for these resources,
+  // even if the response is a failed fetch.
+  bool IsFetchingDone() const;
+
+  // Inherited from BitmapFetcherDelegate
+  virtual void OnFetchComplete(const GURL url, const SkBitmap* bitmap) OVERRIDE;
+
+  // Accessors:
+  GURL low_dpi_url() const { return low_dpi_url_; }
+  void set_low_dpi_url(const GURL& low_dpi_url) {

[dewittj, 2014/03/25 22:31:14]

nit: unused, remove.

[Pete Williamson, 2014/03/26 18:12:55]

I'd really like to leave these in.  They form an integral part of the class
interface, and without them it is hard to understand what this class is supposed
to do.  As accessors, I don't think it is important to remove them if they
aren't called yet.

[dewittj, 2014/03/26 18:38:39]

Can you explain how an unused function is integral?  To me it is just confusing
because now I have to grep the code to see if the URLs are being changed
mid-lifetime of this object.

Also the precedent in Chrome is to not include code that isn't used, even if you
foresee it being used sometime in the future.

[Pete Williamson, 2014/03/26 21:03:50]

Done.

+    low_dpi_url_ = low_dpi_url;
+  }
+  GURL high_dpi_url() const { return high_dpi_url_; }
+  void set_high_dpi_url(const GURL& high_dpi_url) {

[dewittj, 2014/03/25 22:31:14]

nit: unused, remove.

+    high_dpi_url_ = high_dpi_url;
+  }
+  gfx::Image low_dpi_image() { return gfx::Image(image_); }
+  gfx::Image high_dpi_image() { return gfx::Image(image_); }

[dewittj, 2014/03/25 22:31:14]

nit: remove high_dpi_image() since it's not used, and rename low_dpi_image() to
image()

+
+ private:
+  // Helper function to create a bitmap fetcher (but not start the fetch).
+  void CreateBitmapFetcher(const GURL& url);
+
+  GURL low_dpi_url_;
+  GURL high_dpi_url_;
+  bool low_dpi_fetched_;
+  bool high_dpi_fetched_;
+  gfx::ImageSkia image_;
+  ImageHolderDelegate* delegate_;
+  ScopedVector<chrome::BitmapFetcher> fetchers_;
+  Profile* profile_;
+
+  FRIEND_TEST_ALL_PREFIXES(ImageHolderTest, CreateBitmapFetcherTest);
+  FRIEND_TEST_ALL_PREFIXES(ImageHolderTest, OnFetchCompleteTest);
+  FRIEND_TEST_ALL_PREFIXES(ImageHolderTest, IsFetchingDoneTest);
+
+  DISALLOW_COPY_AND_ASSIGN(ImageHolder);
+};
+
+}  // namespace notifier.
+
+#endif  // CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_IMAGE_HOLDER_H_