Applied the IconType.

chrome/browser/history/history_backend.cc

 // Copyright (c) 2011 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/browser/history/history_backend.h"
 
 #include <list>
 #include <map>
 #include <set>
 #include <vector>
@@ skipping 1686 matching lines @@
     const GURL& icon_url,
     scoped_refptr<GetFavIconRequest> request,
     int icon_types) {
   // Check only a single type was given when the page_url was specified.
   DCHECK(!page_url || (page_url && (icon_types == FAV_ICON ||
       icon_types == TOUCH_ICON || icon_types == TOUCH_PRECOMPOSED_ICON)));
 
   if (request->canceled())
     return;
 
-  bool know_favicon = false;
-  bool expired = true;
-  scoped_refptr<RefCountedBytes> data;
+  FaviconService::FaviconData favicon;
 
   if (thumbnail_db_.get()) {
-    IconType returned_icon_type;
     const FavIconID favicon_id =
         thumbnail_db_->GetFavIconIDForFavIconURL(
-            icon_url, icon_types, &returned_icon_type);
+            icon_url, icon_types, &favicon.icon_type);
     if (favicon_id) {
+      scoped_refptr<RefCountedBytes> data;

[sky, 2011/03/10 17:54:44]

Does this really work? Don't you need to set favicon.image_data = new
RefCountedBytes() ? Also, why use data here, just use the one in
favicon.image_data.

[michaelbai, 2011/03/11 01:11:28]

The new RefCountedBytes is actually in next line, moved it up.

Using data avoids casting back to RefCountedBytes in line 1719.

[sky, 2011/03/11 01:19:26]

My mistake.
I don't understand this. Can't 1719 be &(favicon.image_data->data) ?

[michaelbai, 2011/03/11 16:56:19]

data is the member of RefCountedBytes, the type of favicon.image_data is
scoped_refptr<RefCountedMemory> which doesn't have std::vector<unsigned char>
data member.

[sky, 2011/03/11 17:56:45]

GAH! Sorry. I get it.
I'm tempted to say the callback should take a RefCountedBytes, but we don't
really want to expose the implementation so what you have is right.

       data = new RefCountedBytes;
-      know_favicon = true;
+      favicon.known_icon = true;
       Time last_updated;
-      if (thumbnail_db_->GetFavIcon(favicon_id, &last_updated, &data->data,
+      if (thumbnail_db_->GetFavIcon(favicon_id, &last_updated,
+                                    &data->data,
                                     NULL)) {
-        expired = (Time::Now() - last_updated) >
+        favicon.expired = (Time::Now() - last_updated) >
             TimeDelta::FromDays(kFavIconRefetchDays);
+        favicon.image_data = data;
       }
 
       if (page_url)
-        SetFavIconMapping(*page_url, favicon_id, returned_icon_type);
+        SetFavIconMapping(*page_url, favicon_id, favicon.icon_type);
     }
     // else case, haven't cached entry yet. Caller is responsible for
     // downloading the favicon and invoking SetFavIcon.
   }
   request->ForwardResult(GetFavIconRequest::TupleType(
-                             request->handle(), know_favicon, data, expired,
-                             icon_url));
+                             request->handle(), favicon));
 }
 
 void HistoryBackend::GetFavIconForURL(
     scoped_refptr<GetFavIconRequest> request,
     const GURL& page_url,
     int icon_types) {
   if (request->canceled())
     return;
 
-  bool know_favicon = false;
-  bool expired = false;
-  GURL icon_url;
-
-  scoped_refptr<RefCountedBytes> data;
+  FaviconService::FaviconData favicon;
 
   if (db_.get() && thumbnail_db_.get()) {
     // Time the query.
     TimeTicks beginning_time = TimeTicks::Now();
 
     std::vector<IconMapping> icon_mappings;
+    Time last_updated;
+    scoped_refptr<RefCountedBytes> data;

[sky, 2011/03/10 17:54:44]

Same comment as above here.

[michaelbai, 2011/03/11 01:11:28]

Moved new RefCountedBytes up

     data = new RefCountedBytes;
-    Time last_updated;
     if (thumbnail_db_->GetIconMappingsForPageURL(page_url, &icon_mappings) &&
         (icon_mappings.front().icon_type & icon_types) &&
         thumbnail_db_->GetFavIcon(icon_mappings.front().icon_id, &last_updated,
-                                  &data->data, &icon_url)) {
-      know_favicon = true;
-      expired = (Time::Now() - last_updated) >
+                                  &data->data, &favicon.icon_url)) {
+      favicon.known_icon = true;
+      favicon.expired = (Time::Now() - last_updated) >
           TimeDelta::FromDays(kFavIconRefetchDays);
+      favicon.icon_type = icon_mappings.front().icon_type;
+      favicon.image_data = data;
     }
 
     UMA_HISTOGRAM_TIMES("History.GetFavIconForURL",
                         TimeTicks::Now() - beginning_time);
   }
 
   request->ForwardResult(
-      GetFavIconRequest::TupleType(request->handle(), know_favicon, data,
-                                   expired, icon_url));
+      GetFavIconRequest::TupleType(request->handle(), favicon));
 }
 
 void HistoryBackend::SetFavIcon(
     const GURL& page_url,
     const GURL& icon_url,
     scoped_refptr<RefCountedMemory> data,
     IconType icon_type) {
   DCHECK(data.get());
   if (!thumbnail_db_.get() || !db_.get())
     return;
@@ skipping 472 matching lines @@
   return true;
 }
 
 BookmarkService* HistoryBackend::GetBookmarkService() {
   if (bookmark_service_)
     bookmark_service_->BlockTillLoaded();
   return bookmark_service_;
 }
 
 }  // namespace history