Update favicon URLs on FinishLoad for main frame

chrome/renderer/chrome_render_view_observer.cc

 // 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.
 
 #include "chrome/renderer/chrome_render_view_observer.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 678 matching lines @@
           render_view()->GetContentStateImmediately() ?
               0 : kDelayForCaptureMs));
 
   WebFrame* main_frame = render_view()->GetWebView()->mainFrame();
   GURL osd_url = main_frame->document().openSearchDescriptionURL();
   if (!osd_url.is_empty()) {
     Send(new ChromeViewHostMsg_PageHasOSDD(
         routing_id(), render_view()->GetPageId(), osd_url,
         search_provider::AUTODETECTED_PROVIDER));
   }
+}
 
-  int icon_types = WebIconURL::TypeFavicon;
-  if (chrome::kEnableTouchIcon)
-    icon_types |= WebIconURL::TypeTouchPrecomposed | WebIconURL::TypeTouch;
-
-  WebVector<WebIconURL> icon_urls =
-      render_view()->GetWebView()->mainFrame()->iconURLs(icon_types);
+void ChromeRenderViewObserver::CollectAndUpdateFaviconURLs(

[darin (slow to review), 2012/08/06 20:46:32]

please list functions in the .cpp file in the same order as they are declared in
the header file.  that means this new function should be listed just before
IsStrictSecurityHost.

[aruslan, 2012/08/06 20:52:27]

Done.

+    WebKit::WebFrame* frame, int icon_types) {
+  WebVector<WebIconURL> icon_urls = frame->iconURLs(icon_types);
   std::vector<FaviconURL> urls;
   for (size_t i = 0; i < icon_urls.size(); i++) {
     WebURL url = icon_urls[i].iconURL();
     if (!url.isEmpty())
       urls.push_back(FaviconURL(url, ToFaviconType(icon_urls[i].iconType())));
   }
   if (!urls.empty()) {
     Send(new IconHostMsg_UpdateFaviconURL(
         routing_id(), render_view()->GetPageId(), urls));
   }
 }
 
+void ChromeRenderViewObserver::DidFinishLoad(WebKit::WebFrame* frame) {
+  if (frame->parent())
+    return;
+
+  // Please note that we are updating favicons only for the _main_ frame.
+  // Updating Favicon URLs at DidFinishLoad ensures that icon loads always get
+  // initiated after all of the other page resources have been fetched, so icon
+  // loads should not compete with page resources for network bandwidth.
+  int icon_types = WebIconURL::TypeFavicon;
+  if (chrome::kEnableTouchIcon)
+    icon_types |= WebIconURL::TypeTouchPrecomposed | WebIconURL::TypeTouch;
+
+  CollectAndUpdateFaviconURLs(frame, icon_types);
+}
+
 void ChromeRenderViewObserver::DidChangeIcon(WebFrame* frame,
                                              WebIconURL::Type icon_type) {
   if (frame->parent())
     return;
 
   if (!chrome::kEnableTouchIcon &&
       icon_type != WebIconURL::TypeFavicon)
     return;
 
-  WebVector<WebIconURL> icon_urls = frame->iconURLs(icon_type);
-  std::vector<FaviconURL> urls;
-  for (size_t i = 0; i < icon_urls.size(); i++) {
-    urls.push_back(FaviconURL(icon_urls[i].iconURL(),
-                              ToFaviconType(icon_urls[i].iconType())));
-  }
-  Send(new IconHostMsg_UpdateFaviconURL(
-      routing_id(), render_view()->GetPageId(), urls));
+  CollectAndUpdateFaviconURLs(frame, icon_type);
 }
 
 void ChromeRenderViewObserver::DidCommitProvisionalLoad(
     WebFrame* frame, bool is_new_navigation) {
   if (!is_new_navigation)
     return;
 
   CapturePageInfoLater(
       true,  // preliminary_capture
       base::TimeDelta::FromMilliseconds(kDelayForForcedCaptureMs));
@@ skipping 344 matching lines @@
         reinterpret_cast<const unsigned char*>(&data[0]);
 
     return decoder.Decode(src_data, data.size());
   }
   return SkBitmap();
 }
 
 bool ChromeRenderViewObserver::IsStrictSecurityHost(const std::string& host) {
   return (strict_security_hosts_.find(host) != strict_security_hosts_.end());
 }