Extract Favicon Download logic from FaviconTabHelper

chrome/browser/favicon/favicon_download_helper.h

+// Copyright (c) 2012 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_FAVICON_FAVICON_DOWNLOAD_HELPER_H_
+#define CHROME_BROWSER_FAVICON_FAVICON_DOWNLOAD_HELPER_H_
+
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "content/public/browser/web_contents_observer.h"
+
+namespace content {
+class WebContents;
+}
+
+namespace gfx {
+class Image;
+}
+
+class GURL;
+class FaviconDownloadHelperDelegate;
+class SkBitmap;
+
+// FaviconDownloadHelper handles requests to download favicons, and listens for
+// the IPC messages from the renderer.
+//
+class FaviconDownloadHelper
+    : public content::WebContentsObserver,
+      public base::RefCounted<FaviconDownloadHelper> {

[joth, 2012/10/18 21:51:13]

does this need to be ref-counted? seems overkill if it's only for benefit of
reusing the user data adapter.

You might be able to extend WebContentsUserData<FaviconDownloadHelper> instead.
You'll then need to move the delegate param out of c'tor and into a separate
setter method though.

+
+ public:
+  static void CreateForWebContentsAndDelegate(

[joth, 2012/10/18 21:51:13]

nit: I think the AndDelegate is spurious here, and potentially misleading: this
method will create at most one helper instance per web contents, and bind the
lifetime of the helper to that WebContents instance (only). The delegate doesn't
effect this logic. (indeed if you call this method a multiple time for a given
web_contents but with a different delegate, the calls are ignored. perhaps we
should assert against that, or if really necessary, update the existing helper
to point to new delegate)

+      content::WebContents* contents,
+      FaviconDownloadHelperDelegate* delegate);
+
+  static FaviconDownloadHelper* FromWebContents(content::WebContents* contents);
+
+  // Download the favicon at |url|. Returns the unique id of the download
+  // request. The id will be passed to
+  // FaviconDownloadHelperDelegate::OnDidDownloadFavicon once the favicon has
+  // been retrieved.

[joth, 2012/10/18 21:51:13]

could mention what |image_size| is. (is it a _max_size?)
(I assume it doesn't need to be int64? :-)

[Cait (Slow), 2012/10/23 21:36:43]

Added a description of image_size.  It most likely could be an int32, but that
is not what is used elsewhere in the favicon code when dealing with image sizes,
so I am a bit hesitant to change it here.

+  int DownloadFavicon(const GURL& url, int image_size);
+
+ protected:
+  friend class base::RefCounted<FaviconDownloadHelper>;
+
+  FaviconDownloadHelper(content::WebContents* web_contents,
+                        FaviconDownloadHelperDelegate* delegate);
+
+  virtual ~FaviconDownloadHelper();
+
+  // content::WebContentsObserver overrides.
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+
+  // Message handler for IconHostMsg_DidDownloadFavicon. Called when the icon
+  // at |image_url| has been downloaded.
+  // |bitmaps| is a list of all the frames of the icon at |image_url|.
+  void OnDidDownloadFavicon(int id,
+                            const GURL& image_url,
+                            bool errored,
+                            int requested_size,
+                            const std::vector<SkBitmap>& bitmaps);
+
+ private:
+  // Delegate to pass Favicon bitmaps back to.  Weak.
+  FaviconDownloadHelperDelegate* delegate_;
+  DISALLOW_COPY_AND_ASSIGN(FaviconDownloadHelper);

[joth, 2012/10/18 21:51:13]

think this can be DISALLOW_IMPLICIT_CONSTRUCTORS 

[Cait (Slow), 2012/10/23 21:36:43]

Done.

+};
+
+#endif  // CHROME_BROWSER_FAVICON_FAVICON_DOWNLOAD_HELPER_H_