Allow the password manager in the settings page to have hidpi favicons too

chrome/browser/ui/webui/favicon_source.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/browser/ui/webui/favicon_source.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/history/top_sites.h"
@@ skipping 16 matching lines @@
 FaviconSource::FaviconSource(Profile* profile,
                              IconType type,
                              const std::string& source_name)
     : DataSource(source_name, MessageLoop::current()) {
   Init(profile, type);
 }
 
 FaviconSource::~FaviconSource() {
 }
 
-void FaviconSource::Init(Profile* profile, IconType type) {
-  profile_ = profile->GetOriginalProfile();
-  icon_types_ = type == FAVICON ? history::FAVICON :
-      history::TOUCH_PRECOMPOSED_ICON | history::TOUCH_ICON |
-      history::FAVICON;
-}
-
 void FaviconSource::StartDataRequest(const std::string& path,
                                      bool is_incognito,
                                      int request_id) {
   FaviconService* favicon_service =
       FaviconServiceFactory::GetForProfile(profile_, Profile::EXPLICIT_ACCESS);
   if (!favicon_service || path.empty()) {
     SendDefaultResponse(IconRequest(request_id,
                                     "",
                                     16,
                                     ui::SCALE_FACTOR_100P));
     return;
   }
 
   int size_in_dip = gfx::kFaviconSize;
   ui::ScaleFactor scale_factor = ui::SCALE_FACTOR_100P;
 
-  if (path.size() > 8 &&
-      (path.substr(0, 8) == "iconurl/" || path.substr(0, 8) == "iconurl@")) {
-    size_t prefix_length = 8;
-    // Optional scale factor appended to iconurl, which may be @1x or @2x.
-    if (path.at(7) == '@') {
-        size_t slash = path.find("/");
-        std::string scale_str = path.substr(8, slash - 8);
-        web_ui_util::ParseScaleFactor(scale_str, &scale_factor);
-        prefix_length = slash + 1;
+  size_t parsed_index = 0;

[Evan Stade, 2013/01/07 20:23:56]

can you add some docs about the expected format

+  if (path.size() > 5 && path.substr(0, 5) == "size/") {

[Evan Stade, 2013/01/07 20:23:56]

this is a lot of 5s, can you pull it out into a constant?

+    size_t slash = path.find("/", 5);
+    size_t scale_delimiter = path.find("@", 5);

[Evan Stade, 2013/01/07 20:23:56]

define variables as late as possible

+    std::string size = path.substr(5, slash - 5);
+    size_in_dip = atoi(size.c_str());

[Evan Stade, 2013/01/07 20:23:56]

base::StringToInt

+    if (size_in_dip != 64 && size_in_dip != 32 && size_in_dip != 16) {
+      // Only 64x64, 32x32 and 16x16 icons are supported.
+      size_in_dip = 16;
     }
+    // Optional scale factor.
+    if (scale_delimiter != std::string::npos && scale_delimiter < slash) {
+      DCHECK(size_in_dip == 16);
+      std::string scale_str = path.substr(scale_delimiter + 1,
+                                          slash - scale_delimiter - 1);
+      web_ui_util::ParseScaleFactor(scale_str, &scale_factor);
+    }
+    parsed_index = slash + 1;
+  }
+
+  if (path.size() > parsed_index + 8 &&
+      path.substr(parsed_index, 8) == "iconurl/") {
+    parsed_index += 8;

[Evan Stade, 2013/01/07 20:23:56]

ditto with the 8s

     // TODO(michaelbai): Change GetRawFavicon to support combination of
     // IconType.
     favicon_service->GetRawFavicon(
-        GURL(path.substr(prefix_length)),
+        GURL(path.substr(parsed_index)),
         history::FAVICON,
         size_in_dip,
         scale_factor,
         base::Bind(&FaviconSource::OnFaviconDataAvailable,
                    base::Unretained(this),
                    IconRequest(request_id,
-                               path.substr(prefix_length),
+                               path.substr(parsed_index),
                                size_in_dip,
                                scale_factor)),
         &cancelable_task_tracker_);
   } else {
     GURL url;
-    if (path.size() > 5 && path.substr(0, 5) == "size/") {
-      size_t slash = path.find("/", 5);
-      size_t scale_delimiter = path.find("@", 5);
-      std::string size = path.substr(5, slash - 5);
-      size_in_dip = atoi(size.c_str());
-      if (size_in_dip != 64 && size_in_dip != 32 && size_in_dip != 16) {
-        // Only 64x64, 32x32 and 16x16 icons are supported.
-        size_in_dip = 16;
-      }
-      // Optional scale factor.
-      if (scale_delimiter != std::string::npos && scale_delimiter < slash) {
-        DCHECK(size_in_dip == 16);
-        std::string scale_str = path.substr(scale_delimiter + 1,
-                                            slash - scale_delimiter - 1);
-        web_ui_util::ParseScaleFactor(scale_str, &scale_factor);
-      }
-      url = GURL(path.substr(slash + 1));
+
+    // 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 (path.size() > parsed_index + 7 &&
+        path.substr(parsed_index, 7) == "origin/") {
+      parsed_index += 7;

[Evan Stade, 2013/01/07 20:23:56]

ditto with the 7s

+      std::string originalUrl = path.substr(parsed_index);

[Evan Stade, 2013/01/07 20:23:56]

original_url

+
+      // If the original URL does not specify a scheme (e.g., example.com
+      // instead of http://example.com), add "http://" as a default.
+      if (!GURL(originalUrl).has_scheme())
+        originalUrl = "http://" + originalUrl;
+
+      // Strip the path beyond the top-level domain.
+      url = GURL(originalUrl).GetOrigin();
     } 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 (path.size() > 7 && path.substr(0, 7) == "origin/") {
-        std::string originalUrl = path.substr(7);
-
-        // If the original URL does not specify a scheme (e.g., example.com
-        // instead of http://example.com), add "http://" as a default.
-        if (!GURL(originalUrl).has_scheme())
-          originalUrl = "http://" + originalUrl;
-
-        // Strip the path beyond the top-level domain.
-        url = GURL(originalUrl).GetOrigin();
-      } else {
-        url = GURL(path);
-      }
+      url = GURL(path.substr(parsed_index));
     }
 
     // Intercept requests for prepopulated pages.
     for (size_t i = 0; i < arraysize(history::kPrepopulatedPages); i++) {
       if (url.spec() ==
           l10n_util::GetStringUTF8(history::kPrepopulatedPages[i].url_id)) {
         SendResponse(request_id,
             ResourceBundle::GetSharedInstance().LoadDataResourceBytesForScale(
                 history::kPrepopulatedPages[i].favicon_id,
                 scale_factor));
@@ skipping 26 matching lines @@
   // requests on the floor.
   return false;
 }
 
 bool FaviconSource::HandleMissingResource(const IconRequest& request) {
   // No additional checks to locate the favicon resource in the base
   // implementation.
   return false;
 }
 
+void FaviconSource::Init(Profile* profile, IconType type) {
+  profile_ = profile->GetOriginalProfile();
+  icon_types_ = type == FAVICON ? history::FAVICON :
+      history::TOUCH_PRECOMPOSED_ICON | history::TOUCH_ICON |
+      history::FAVICON;
+}
+
 void FaviconSource::OnFaviconDataAvailable(
     const IconRequest& request,
     const history::FaviconBitmapResult& bitmap_result) {
   if (bitmap_result.is_valid()) {
     // Forward the data along to the networking system.
     SendResponse(request.request_id, bitmap_result.bitmap_data);
   } else if (!HandleMissingResource(request)) {
     SendDefaultResponse(request);
   }
 }
@@ skipping 19 matching lines @@
 
   if (!default_favicon) {
     ui::ScaleFactor scale_factor = icon_request.scale_factor;
     default_favicon = ResourceBundle::GetSharedInstance()
         .LoadDataResourceBytesForScale(resource_id, scale_factor);
     default_favicons_[favicon_index] = default_favicon;
   }
 
   SendResponse(icon_request.request_id, default_favicon);
 }