WebView::DownloadImage refactored

The implementation was dependent on webkit_glue::image_resource_fetcher and had to be removed if we want to upstream WebView.
DownloadImage has been moved into RenderView::DownloadImage. WebViewImpl's set of ImageResourceFetchers has been ripped off and moved into RenderView.

Review URL: http://codereview.chromium.org/255006

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

6 files changed, 51 insertions, 75 deletions

diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 8d9b3dd..b0cb9dc 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -105,6 +105,7 @@ using base::Time;
 using base::TimeDelta;
 using webkit_glue::AltErrorPageResourceFetcher;
 using webkit_glue::AutofillForm;
+using webkit_glue::ImageResourceFetcher;
 using webkit_glue::PasswordForm;
 using webkit_glue::PasswordFormDomManager;
 using webkit_glue::SearchableFormData;
@@ -238,6 +239,12 @@ RenderView::~RenderView() {
   if (decrement_shared_popup_at_destruction_)
     shared_popup_counter_->data--;
 
+  // Dispose of un-disposed image fetchers.
+  for (ImageResourceFetcherSet::iterator i = image_fetchers_.begin();
+       i != image_fetchers_.end(); ++i) {
+    delete *i;
+  }
+
 #if defined(OS_MACOSX)
   // Tell the spellchecker that the document is closed.
   if (has_document_tag_)
@@ -2446,12 +2453,31 @@ void RenderView::ShowContextMenu(WebView* webview,
   Send(new ViewHostMsg_ContextMenu(routing_id_, params));
 }
 
-void RenderView::DidDownloadImage(int id,
-                                  const GURL& image_url,
-                                  bool errored,
+bool RenderView::DownloadImage(int id, const GURL& image_url, int image_size) {
+  // Make sure webview was not shut down.
+  if (!webview())
+    return false;
+  // Create an image resource fetcher and assign it with a call back object.
+  image_fetchers_.insert(new ImageResourceFetcher(
+      image_url, webview()->GetMainFrame(), id, image_size,
+      NewCallback(this, &RenderView::DidDownloadImage)));
+  return true;
+}
+
+void RenderView::DidDownloadImage(ImageResourceFetcher* fetcher,
                                   const SkBitmap& image) {
-  Send(new ViewHostMsg_DidDownloadFavIcon(routing_id_, id, image_url, errored,
-                                          image));
+  // Notify requester of image download status.
+   Send(new ViewHostMsg_DidDownloadFavIcon(routing_id_,
+                                           fetcher->id(),
+                                           fetcher->image_url(),
+                                           image.isNull(),
+                                           image));
+  // Dispose of the image fetcher.
+  DCHECK(image_fetchers_.find(fetcher) != image_fetchers_.end());
+  image_fetchers_.erase(fetcher);
+  // We're in the callback from the ImageResourceFetcher, best to delay
+  // deletion.
+  MessageLoop::current()->DeleteSoon(FROM_HERE, fetcher);
 }
 
 void RenderView::OnDownloadFavIcon(int id,
@@ -2468,7 +2494,7 @@ void RenderView::OnDownloadFavIcon(int id,
   }
 
   if (data_image_failed ||
-      !webview()->DownloadImage(id, image_url, image_size)) {
+      !DownloadImage(id, image_url, image_size)) {
     Send(new ViewHostMsg_DidDownloadFavIcon(routing_id_, id, image_url, true,
                                             SkBitmap()));
   }
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index ae0732a..fd12078a 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -37,6 +37,7 @@
 #include "webkit/api/public/WebTextDirection.h"
 #include "webkit/glue/dom_serializer_delegate.h"
 #include "webkit/glue/form_data.h"
+#include "webkit/glue/image_resource_fetcher.h"
 #include "webkit/glue/password_form_dom_manager.h"
 #include "webkit/glue/webaccessibilitymanager.h"
 #include "webkit/glue/webplugin_page_delegate.h"
@@ -177,10 +178,6 @@ class RenderView : public RenderWidget,
   virtual void OnMissingPluginStatus(
       WebPluginDelegateProxy* delegate,
       int status);
-  virtual void DidDownloadImage(int id,
-                                const GURL& image_url,
-                                bool errored,
-                                const SkBitmap& image);
   virtual void ShowContextMenu(WebView* webview,
                                ContextNodeType node_type,
                                int x,
@@ -686,6 +683,19 @@ class RenderView : public RenderWidget,
   // Locates a sub frame with given xpath
   WebKit::WebFrame* GetChildFrame(const std::wstring& frame_xpath) const;
 
+  // Requests to download an image. When done, the RenderView is
+  // notified by way of DidDownloadImage. Returns true if the request was
+  // successfully started, false otherwise. id is used to uniquely identify the
+  // request and passed back to the DidDownloadImage method. If the image has
+  // multiple frames, the frame whose size is image_size is returned. If the
+  // image doesn't have a frame at the specified size, the first is returned.
+  bool DownloadImage(int id, const GURL& image_url, int image_size);
+
+  // This callback is triggered when DownloadImage completes, either
+  // succesfully or with a failure. See DownloadImage for more details.
+  void DidDownloadImage(webkit_glue::ImageResourceFetcher* fetcher,
+                        const SkBitmap& image);
+
   enum ErrorPageType {
     DNS_ERROR,
     HTTP_404,
@@ -954,6 +964,11 @@ class RenderView : public RenderWidget,
   // Shall be cleared as soon as the next key event is processed.
   EditCommands edit_commands_;
 
+  // ImageResourceFetchers schedule via DownloadImage.
+  typedef std::set<webkit_glue::ImageResourceFetcher*> ImageResourceFetcherSet;
+  ImageResourceFetcherSet image_fetchers_;
+
+
   DISALLOW_COPY_AND_ASSIGN(RenderView);
 };
 
diff --git a/webkit/glue/webview.h b/webkit/glue/webview.h
index 53ab714..43af41e 100644
--- a/webkit/glue/webview.h
+++ b/webkit/glue/webview.h
@@ -137,14 +137,6 @@ class WebView : public WebKit::WebWidget {
   // ensure that a text field on the page is not eating keystrokes we send it.
   virtual void ClearFocusedNode() = 0;
 
-  // Requests the webview to download an image. When done, the delegate is
-  // notified by way of DidDownloadImage. Returns true if the request was
-  // successfully started, false otherwise. id is used to uniquely identify the
-  // request and passed back to the DidDownloadImage method. If the image has
-  // multiple frames, the frame whose size is image_size is returned. If the
-  // image doesn't have a frame at the specified size, the first is returned.
-  virtual bool DownloadImage(int id, const GURL& image_url, int image_size) = 0;
-
   // Gets a WebSettings object that can be used to modify the behavior of this
   // WebView.  The object is deleted by this class on destruction, so you must
   // not use it beyond WebView's lifetime.
diff --git a/webkit/glue/webview_delegate.h b/webkit/glue/webview_delegate.h
index 0773109..1acf4ac 100644
--- a/webkit/glue/webview_delegate.h
+++ b/webkit/glue/webview_delegate.h
@@ -183,19 +183,6 @@ class WebViewDelegate : public WebKit::WebViewClient {
                                const std::string& frame_charset) {
   }
 
-  // -------------------------------------------------------------------------
-
-  // Notification that a request to download an image has completed. |errored|
-  // indicates if there was a network error. The image is empty if there was
-  // a network error, the contents of the page couldn't by converted to an
-  // image, or the response from the host was not 200.
-  // NOTE: image is empty if the response didn't contain image data.
-  virtual void DidDownloadImage(int id,
-                                const GURL& image_url,
-                                bool errored,
-                                const SkBitmap& image) {
-  }
-
   // InspectorClient ---------------------------------------------------------
 
   virtual void UpdateInspectorSettings(const std::wstring& raw_settings) { }
diff --git a/webkit/glue/webview_impl.cc b/webkit/glue/webview_impl.cc
index 74a4c3f..63727cd 100644
--- a/webkit/glue/webview_impl.cc
+++ b/webkit/glue/webview_impl.cc
@@ -419,10 +419,6 @@ WebViewImpl::WebViewImpl(WebViewDelegate* delegate)
 
 WebViewImpl::~WebViewImpl() {
   DCHECK(page_ == NULL);
-  for (std::set<ImageResourceFetcher*>::iterator i = image_fetchers_.begin();
-       i != image_fetchers_.end(); ++i) {
-    delete *i;
-  }
 }
 
 RenderTheme* WebViewImpl::theme() const {
@@ -1376,16 +1372,6 @@ void WebViewImpl::SetInitialFocus(bool reverse) {
   }
 }
 
-bool WebViewImpl::DownloadImage(int id, const GURL& image_url,
-                                int image_size) {
-  if (!page_.get())
-    return false;
-  image_fetchers_.insert(new ImageResourceFetcher(
-      image_url, main_frame(), id, image_size,
-      NewCallback(this, &WebViewImpl::OnImageFetchComplete)));
-  return true;
-}
-
 WebSettings* WebViewImpl::GetSettings() {
   if (!web_settings_.get())
     web_settings_.reset(new WebSettingsImpl(page_->settings()));
@@ -1810,15 +1796,6 @@ void WebViewImpl::StartDragging(const WebPoint& event_pos,
   client()->startDragging(event_pos, drag_data, mask);
 }
 
-void WebViewImpl::OnImageFetchComplete(ImageResourceFetcher* fetcher,
-                                       const SkBitmap& image) {
-  if (delegate_) {
-    delegate_->DidDownloadImage(fetcher->id(), fetcher->image_url(),
-                                image.isNull(), image);
-  }
-  DeleteImageResourceFetcher(fetcher);
-}
-
 void WebViewImpl::SetCurrentHistoryItem(WebCore::HistoryItem* item) {
   back_forward_list_client_impl_.SetCurrentHistoryItem(item);
 }
@@ -1834,15 +1811,6 @@ void WebViewImpl::ObserveNewNavigation() {
 #endif
 }
 
-void WebViewImpl::DeleteImageResourceFetcher(ImageResourceFetcher* fetcher) {
-  DCHECK(image_fetchers_.find(fetcher) != image_fetchers_.end());
-  image_fetchers_.erase(fetcher);
-
-  // We're in the callback from the ImageResourceFetcher, best to delay
-  // deletion.
-  MessageLoop::current()->DeleteSoon(FROM_HERE, fetcher);
-}
-
 void WebViewImpl::HideAutoCompletePopup() {
   if (autocomplete_popup_showing_) {
     autocomplete_popup_->hidePopup();
diff --git a/webkit/glue/webview_impl.h b/webkit/glue/webview_impl.h
index bb9f077..165b6bd 100644
--- a/webkit/glue/webview_impl.h
+++ b/webkit/glue/webview_impl.h
@@ -98,7 +98,6 @@ class WebViewImpl : public WebView, public base::RefCounted<WebViewImpl> {
   virtual void StopLoading();
   virtual void SetBackForwardListSize(int size);
   virtual void SetInitialFocus(bool reverse);
-  virtual bool DownloadImage(int id, const GURL& image_url, int image_size);
   virtual WebKit::WebSettings* GetSettings();
   virtual const std::wstring& GetInspectorSettings() const;
   virtual void SetInspectorSettings(const std::wstring& settings);
@@ -265,10 +264,6 @@ class WebViewImpl : public WebView, public base::RefCounted<WebViewImpl> {
   friend class WebView;  // So WebView::Create can call our constructor
   friend class base::RefCounted<WebViewImpl>;
 
-  // ImageResourceFetcher::Callback.
-  void OnImageFetchComplete(webkit_glue::ImageResourceFetcher* fetcher,
-                            const SkBitmap& bitmap);
-
   WebViewImpl(WebViewDelegate* delegate);
   ~WebViewImpl();
 
@@ -325,18 +320,11 @@ class WebViewImpl : public WebView, public base::RefCounted<WebViewImpl> {
   // Returns true if the view was scrolled.
   bool ScrollViewWithKeyboard(int key_code, int modifiers);
 
-  // Removes fetcher from the set of pending image fetchers and deletes it.
-  // This is invoked after the download is completed (or fails).
-  void DeleteImageResourceFetcher(webkit_glue::ImageResourceFetcher* fetcher);
-
   // Converts |pos| from window coordinates to contents coordinates and gets
   // the HitTestResult for it.
   WebCore::HitTestResult HitTestResultForWindowPos(
       const WebCore::IntPoint& pos);
 
-  // ImageResourceFetchers schedule via DownloadImage.
-  std::set<webkit_glue::ImageResourceFetcher*> image_fetchers_;
-
   // The point relative to the client area where the mouse was last pressed
   // down. This is used by the drag client to determine what was under the
   // mouse when the drag was initiated. We need to track this here in