Brush up some favicon URL parsing code

R=pkotwicz@chromium.org
BUG=953962

Change-Id: Id13ea7145bc302da3ff427098648517be48b2964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1730258Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Commit-Queue: Dan Beam <dbeam@chromium.org>
Auto-Submit: Dan Beam <dbeam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683671}

components/favicon_base/favicon_url_parser.cc

@@ -31,12 +31,7 @@ bool ParseFaviconPathWithLegacyFormat(const std::string& path,
   const char kIconURLParameter[] = "iconurl/";
   const char kSizeParameter[] = "size/";
 
-  parsed->size_in_dip = gfx::kFaviconSize;
-  parsed->device_scale_factor = 1.0f;
-  parsed->path_index = std::string::npos;
-  // Use of the favicon server is never exposed for the legacy format (only used
-  // by extensions).
-  parsed->allow_favicon_server_fallback = false;
+  *parsed = chrome::ParsedFaviconPath();
 
   if (path.empty())
     return false;
@@ -91,50 +86,39 @@ bool ParseFaviconPathWithLegacyFormat(const std::string& path,
 // Parse with FaviconUrlFormat::kFavicon2 format.
 bool ParseFaviconPathWithFavicon2Format(const std::string& path,
                                         chrome::ParsedFaviconPath* parsed) {
-  // Parameters which can be used in chrome://favicon2 path. See file
-  // "favicon_url_parser.h" for a description of what each one does.
-  const std::string kSizeParameter = "size";
-  const std::string kScaleParameter = "scale_factor";
-  const std::string kPageUrlParameter = "page_url";
-  const std::string kIconUrlParameter = "icon_url";
-  const std::string kAllowFallbackParameter = "allow_google_server_fallback";
-
   if (path.empty())
     return false;
 
-  GURL query_url("chrome://favicon2/" + path);
-
-  std::string size_str;
-  if (!net::GetValueForKeyInQuery(query_url, kSizeParameter, &size_str))
-    parsed->size_in_dip = gfx::kFaviconSize;
-  else if (!base::StringToInt(size_str, &parsed->size_in_dip))
-    return false;
-
-  std::string scale_str;
-  if (!net::GetValueForKeyInQuery(query_url, kScaleParameter, &scale_str))
-    parsed->device_scale_factor = 1.0f;
-  else if (!webui::ParseScaleFactor(scale_str, &parsed->device_scale_factor))
-    return false;
-
-  if (!net::GetValueForKeyInQuery(query_url, kPageUrlParameter,
-                                  &parsed->page_url))
-    parsed->page_url = "";
-
-  if (!net::GetValueForKeyInQuery(query_url, kIconUrlParameter,
-                                  &parsed->icon_url))
-    parsed->icon_url = "";
+  GURL query_url = GURL("chrome://favicon2/").Resolve(path);
+
+  *parsed = chrome::ParsedFaviconPath();
+
+  for (net::QueryIterator it(query_url); !it.IsAtEnd(); it.Advance()) {
+    const std::string key = it.GetKey();
+    // Note: each of these keys can be used in chrome://favicon2 path. See file
+    // "favicon_url_parser.h" for a description of what each one does.
+    if (key == "allow_google_server_fallback") {
+      const std::string val = it.GetUnescapedValue();
+      if (!(val == "0" || val == "1"))
+        return false;
+      parsed->allow_favicon_server_fallback = val == "1";
+    } else if (key == "icon_url") {
+      parsed->icon_url = it.GetUnescapedValue();
+    } else if (key == "page_url") {
+      parsed->page_url = it.GetUnescapedValue();
+    } else if (key == "scale_factor" &&
+               !webui::ParseScaleFactor(it.GetUnescapedValue(),
+                                        &parsed->device_scale_factor)) {
+      return false;
+    } else if (key == "size" && !base::StringToInt(it.GetUnescapedValue(),
+                                                   &parsed->size_in_dip)) {
+      return false;
+    }
+  }
 
   if (parsed->page_url.empty() && parsed->icon_url.empty())
     return false;
 
-  std::string allow_favicon_server_fallback;
-  if (!net::GetValueForKeyInQuery(query_url, kAllowFallbackParameter,
-                                  &allow_favicon_server_fallback))
-    parsed->allow_favicon_server_fallback = false;
-  else if (!base::StringToInt(allow_favicon_server_fallback,
-                              (int*)&parsed->allow_favicon_server_fallback))
-    return false;
-
   if (parsed->allow_favicon_server_fallback && parsed->page_url.empty()) {
     // Since the server is queried by page url, we'll fail if no non-empty page
     // url is provided and the fallback parameter is still set to true.

components/favicon_base/favicon_url_parser.h

@@ -9,6 +9,8 @@
 
 #include <string>
 
+#include "ui/gfx/favicon_size.h"
+
 namespace chrome {
 
 struct ParsedFaviconPath {
@@ -23,18 +25,18 @@ struct ParsedFaviconPath {
   std::string icon_url;
 
   // The size of the requested favicon in dip.
-  int size_in_dip;
+  int size_in_dip = gfx::kFaviconSize;
 
   // The device scale factor of the requested favicon.
-  float device_scale_factor;
+  float device_scale_factor = 1.0f;
 
   // TODO(victorvianna): Remove this parameter.
   // The index of the first character (relative to the path) where the the URL
   // from which the favicon is being requested is located.
-  size_t path_index;
+  size_t path_index = std::string::npos;
 
   // Whether we should allow making a request to the favicon server as fallback.
-  bool allow_favicon_server_fallback;
+  bool allow_favicon_server_fallback = false;
 };
 
 // Enum describing the two possible url formats: the legacy chrome://favicon