Using largest favicon for shortcut when possible.

chrome/browser/android/shortcut_helper.cc

 // Copyright 2013 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/android/shortcut_helper.h"
 
 #include <jni.h>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/basictypes.h"
 #include "base/location.h"
 #include "base/strings/string16.h"
 #include "base/threading/worker_pool.h"
 #include "chrome/browser/android/tab_android.h"
 #include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/common/cancelable_task_tracker.h"
 #include "chrome/common/render_messages.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/frame_navigate_params.h"
 #include "jni/ShortcutHelper_jni.h"
 #include "ui/gfx/android/java_bitmap.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/color_analysis.h"
+#include "ui/gfx/favicon_size.h"
 #include "url/gurl.h"
 
 ShortcutBuilder::ShortcutBuilder(content::WebContents* web_contents,
                                  const string16& title)
     : shortcut_type_(BOOKMARK) {
   Observe(web_contents);
   url_ = web_contents->GetURL();
   if (title.length() > 0)
     title_ = title;
   else
@@ skipping 26 matching lines @@
     shortcut_type_ = APP_SHORTCUT_APPLE;
   } else if (is_apple_mobile_webapp_capable || is_mobile_webapp_capable) {
     shortcut_type_ = APP_SHORTCUT;
   } else {
     shortcut_type_ = BOOKMARK;
   }
 
   // Grab the best, largest icon we can find to represent this bookmark.
   // TODO(dfalcantara): Try combining with the new BookmarksHandler once its
   //                    rewrite is further along.
-  FaviconService::FaviconForURLParams favicon_params(
-      profile,
-      url_,
-      chrome::TOUCH_PRECOMPOSED_ICON | chrome::TOUCH_ICON | chrome::FAVICON,
-      0);
-
+  std::vector<int> icon_types;
+  icon_types.push_back(chrome::FAVICON);
+  icon_types.push_back(chrome::TOUCH_PRECOMPOSED_ICON | chrome::TOUCH_ICON);
   FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
       profile, Profile::EXPLICIT_ACCESS);
-
-  favicon_service->GetRawFaviconForURL(
-      favicon_params,
-      ui::SCALE_FACTOR_100P,
+  // The favicon was resized by ScaleFactor and stored in HistoryBackend.
+  // We need to get largest possible size of the resized normal favicon and
+  // pass it into GetLargestFawFaviconForURL(), otherwise, the normal favicon is
+  // returned even there is larger touch icon avaliable.
+  // In theory, the side effect is that the touch icon or precomposed touch
+  // icon whose size is smaller than kFaviconSize*ScaleFactor and larger than
+  // kFaviconSize can't be returned, but it actually not a issue as the normal
+  // favicon is 16x16, the maximum scale factor we used is 2.0f, so the maximum
+  // resized favicon is 32x32 and much smaller than touch icon whose size is
+  // from 57x57 to 144x144.

[michaelbai, 2013/10/23 23:20:47]

I have a CL to fix this issue, the icon will not be resized before stored to
database. Once that CL landed, I will update this comments. Since favicon in
current database is already resized, I have to put this logic here.

+  std::vector<ui::ScaleFactor> favicon_scale_factors =
+      ui::GetSupportedScaleFactors();
+  ui::ScaleFactor scale_factor = ui::SCALE_FACTOR_100P;
+  for (std::vector<ui::ScaleFactor>::const_iterator i =
+           favicon_scale_factors.begin(); i != favicon_scale_factors.end();
+       ++i) {
+    if (scale_factor < *i)
+      scale_factor = *i;
+  }
+  favicon_service->GetLargestRawFaviconForURL(profile, url_, icon_types,
+      gfx::kFaviconSize * ui::GetImageScale(scale_factor),
       base::Bind(&ShortcutBuilder::FinishAddingShortcut,
                  base::Unretained(this)),
       &cancelable_task_tracker_);
 }
 
 void ShortcutBuilder::FinishAddingShortcut(
     const chrome::FaviconBitmapResult& bitmap_result) {
   base::WorkerPool::PostTask(
       FROM_HERE,
       base::Bind(&ShortcutHelper::AddShortcutInBackground,
@@ skipping 106 matching lines @@
 // its otherwise inaccessible WebContents.
 static void AddShortcut(JNIEnv* env,
                         jclass clazz,
                         jint tab_android_ptr,
                         jstring title) {
   TabAndroid* tab = reinterpret_cast<TabAndroid*>(tab_android_ptr);
   ShortcutHelper::AddShortcut(
       tab->web_contents(),
       base::android::ConvertJavaStringToUTF16(env, title));
 }