Supporting high dpi favicons in Instant Extended.

chrome/browser/ui/webui/favicon_source.cc

Index: chrome/browser/ui/webui/favicon_source.cc
diff --git a/chrome/browser/ui/webui/favicon_source.cc b/chrome/browser/ui/webui/favicon_source.cc
index 45522732b755fe411cb35e45929c56361a6d8aee..1ac079943d7dfb74547b959f5d9c6b68f24fdcf5 100644
--- a/chrome/browser/ui/webui/favicon_source.cc
+++ b/chrome/browser/ui/webui/favicon_source.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/search/instant_io_context.h"
 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_factory.h"
+#include "chrome/common/favicon_url_parser.h"
 #include "chrome/common/url_constants.h"
 #include "grit/locale_settings.h"
 #include "grit/ui_resources.h"
@@ -22,31 +23,6 @@
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/webui/web_ui_util.h"
 
-namespace {
-
-// Parameters which can be used in chrome://favicon path. See .h file for a
-// description of what each does.
-const char kIconURLParameter[] = "iconurl/";
-const char kLargestParameter[] = "largest/";
-const char kOriginParameter[] = "origin/";
-const char kSizeParameter[] = "size/";
-
-// Returns true if |search| is a substring of |path| which starts at
-// |start_index|.
-bool HasSubstringAt(const std::string& path,
-                    size_t start_index,
-                    const std::string& search) {
-  if (search.empty())
-    return false;
-
-  if (start_index + search.size() >= path.size())
-    return false;
-
-  return (path.compare(start_index, search.size(), search) == 0);
-}
-
-}  // namespace
-
 FaviconSource::IconRequest::IconRequest()
     : size_in_dip(gfx::kFaviconSize),
       scale_factor(ui::SCALE_FACTOR_NONE) {
@@ -93,32 +69,30 @@ void FaviconSource::StartDataRequest(
     return;
   }
 
-  bool is_icon_url = false;
-  GURL url;
-  int size_in_dip = 16;
-  ui::ScaleFactor scale_factor = ui::SCALE_FACTOR_100P;
-  bool success = ParsePath(path, &is_icon_url, &url, &size_in_dip,
-      &scale_factor);
+  chrome::ParsedFaviconPath parsed;
+  bool success = chrome::ParseFaviconPath(path, icon_types_, &parsed);
+
+  GURL url(parsed.url);
 
   if (!success) {

[samarth, 2013/06/20 00:31:29]

nit: move this to right after the Parse call.  url isn't used later.

[pedro (no code reviews), 2013/06/20 22:43:45]

Done.

     SendDefaultResponse(callback);
     return;
   }
 
-  if (is_icon_url) {
+  if (parsed.is_icon_url) {
     // TODO(michaelbai): Change GetRawFavicon to support combination of
     // IconType.
     favicon_service->GetRawFavicon(
         url,
         chrome::FAVICON,
-        size_in_dip,
-        scale_factor,
+        parsed.size_in_dip,
+        parsed.scale_factor,
         base::Bind(&FaviconSource::OnFaviconDataAvailable,
                    base::Unretained(this),
                    IconRequest(callback,
                                url,
-                               size_in_dip,
-                               scale_factor)),
+                               parsed.size_in_dip,
+                               parsed.scale_factor)),
         &cancelable_task_tracker_);
   } else {
     // Intercept requests for prepopulated pages.
@@ -128,21 +102,21 @@ void FaviconSource::StartDataRequest(
         callback.Run(
             ResourceBundle::GetSharedInstance().LoadDataResourceBytesForScale(
                 history::kPrepopulatedPages[i].favicon_id,
-                scale_factor));
+                parsed.scale_factor));
         return;
       }
     }
 
     favicon_service->GetRawFaviconForURL(
         FaviconService::FaviconForURLParams(
-            profile_, url, icon_types_, size_in_dip),
-        scale_factor,
+            profile_, url, icon_types_, parsed.size_in_dip),
+        parsed.scale_factor,
         base::Bind(&FaviconSource::OnFaviconDataAvailable,
                    base::Unretained(this),
                    IconRequest(callback,
                                url,
-                               size_in_dip,
-                               scale_factor)),
+                               parsed.size_in_dip,
+                               parsed.scale_factor)),
         &cancelable_task_tracker_);
   }
 }
@@ -171,93 +145,6 @@ bool FaviconSource::HandleMissingResource(const IconRequest& request) {
   return false;
 }
 
-bool FaviconSource::ParsePath(const std::string& path,
-                              bool* is_icon_url,
-                              GURL* url,
-                              int* size_in_dip,
-                              ui::ScaleFactor* scale_factor) const {
-  DCHECK_EQ(16, gfx::kFaviconSize);
-
-  *is_icon_url = false;
-  *url = GURL();
-  *size_in_dip = 16;
-  *scale_factor = ui::SCALE_FACTOR_100P;
-
-  if (path.empty())
-    return false;
-
-  size_t parsed_index = 0;
-  if (HasSubstringAt(path, parsed_index, kLargestParameter)) {
-    parsed_index += strlen(kLargestParameter);
-    *size_in_dip = 0;
-  } else if (HasSubstringAt(path, parsed_index, kSizeParameter)) {
-    parsed_index += strlen(kSizeParameter);
-
-    size_t slash = path.find("/", parsed_index);
-    if (slash == std::string::npos)
-      return false;
-
-    size_t scale_delimiter = path.find("@", parsed_index);
-    std::string size_str;
-    std::string scale_str;
-    if (scale_delimiter == std::string::npos) {
-      // Support the legacy size format of 'size/aa/' where 'aa' is the desired
-      // size in DIP for the sake of not regressing the extensions which use it.
-      size_str = path.substr(parsed_index, slash - parsed_index);
-    } else {
-      size_str = path.substr(parsed_index, scale_delimiter - parsed_index);
-      scale_str = path.substr(scale_delimiter + 1, slash - scale_delimiter - 1);
-    }
-
-    if (!base::StringToInt(size_str, size_in_dip))
-      return false;
-
-    if (*size_in_dip != 64 && *size_in_dip != 32) {
-      // Only 64x64, 32x32 and 16x16 icons are supported.
-      *size_in_dip = 16;
-    }
-
-    if (!scale_str.empty())
-      webui::ParseScaleFactor(scale_str, scale_factor);
-
-    // Return the default favicon (as opposed to a resized favicon) for
-    // favicon sizes which are not cached by the favicon service.
-    // Currently the favicon service caches:
-    // - favicons of sizes "16 * scale factor" px of type FAVICON
-    //   where scale factor is one of FaviconUtil::GetFaviconScaleFactors().
-    // - the largest TOUCH_ICON / TOUCH_PRECOMPOSED_ICON
-    if (*size_in_dip != 16 && icon_types_ == chrome::FAVICON)
-      return false;
-
-    parsed_index = slash + 1;
-  }
-
-  if (HasSubstringAt(path, parsed_index, kIconURLParameter)) {
-    parsed_index += strlen(kIconURLParameter);
-    *is_icon_url = true;
-    *url = GURL(path.substr(parsed_index));
-  } else {
-    // URL requests prefixed with "origin/" are converted to a form with an
-    // empty path and a valid scheme. (e.g., example.com -->
-    // http://example.com/ or http://example.com/a --> http://example.com/)
-    if (HasSubstringAt(path, parsed_index, kOriginParameter)) {
-      parsed_index += strlen(kOriginParameter);
-      std::string possibly_invalid_url = path.substr(parsed_index);
-
-      // If the URL does not specify a scheme (e.g., example.com instead of
-      // http://example.com), add "http://" as a default.
-      if (!GURL(possibly_invalid_url).has_scheme())
-        possibly_invalid_url = "http://" + possibly_invalid_url;
-
-      // Strip the path beyond the top-level domain.
-      *url = GURL(possibly_invalid_url).GetOrigin();
-    } else {
-      *url = GURL(path.substr(parsed_index));
-    }
-  }
-  return true;
-}
-
 void FaviconSource::OnFaviconDataAvailable(
     const IconRequest& request,
     const chrome::FaviconBitmapResult& bitmap_result) {