[Fallback icons] Change "explicit flow" interface so color hex strings don't use "#".

chrome/common/favicon/fallback_icon_url_parser.cc

Index: chrome/common/favicon/fallback_icon_url_parser.cc
diff --git a/chrome/common/favicon/fallback_icon_url_parser.cc b/chrome/common/favicon/fallback_icon_url_parser.cc
index 418617f46fec37ecc6fc13fa3289667f45ccbaea..4683d21c566df96de8ccdf4e2c431bda2ae211cd 100644
--- a/chrome/common/favicon/fallback_icon_url_parser.cc
+++ b/chrome/common/favicon/fallback_icon_url_parser.cc
@@ -7,9 +7,24 @@
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "third_party/skia/include/utils/SkParse.h"
 #include "ui/gfx/favicon_size.h"
 
+namespace {
+
+bool IsHexColorString(const std::string& color_str) {

[James Hawkins, 2015/03/06 22:24:52]

Please add documentation about the conditions which make a string a hex color.

[huangs, 2015/03/06 22:39:48]

Done.

+  size_t len = color_str.length();
+  if (len != 3 && len != 4 && len != 6 && len != 8)
+    return false;
+  for (auto ch : color_str)
+    if (!IsHexDigit(ch))
+      return false;
+  return true;
+}
+
+}  // namespace
+
 namespace chrome {
 
 ParsedFallbackIconPath::ParsedFallbackIconPath()
@@ -76,9 +91,22 @@ bool ParsedFallbackIconPath::ParseSpecs(
 // static
 bool ParsedFallbackIconPath::ParseColor(const std::string& color_str,
                                         SkColor* color) {
-  const char* end = SkParse::FindColor(color_str.c_str(), color);
-  // Return true if FindColor() succeeds and |color_str| is entirely consumed.
-  return end && !*end;
+  // Exclude empty case and disallow '#' prefix since it messes up URL.

[James Hawkins, 2015/03/06 22:24:52]

Please clarify 'messes up URL'.

[huangs, 2015/03/06 22:39:48]

Done. Also note that escaping '#' as "%23" would be yucky in our usage.

+  if (color_str.empty() || color_str[0] == '#')
+    return false;
+  // Force alpha = 0xFF since SkParse::FindColor() preserves unspecified alpha.
+  SkColor temp_color = SK_ColorWHITE;
+  // Prepend '#' if color hex string is given. This is unambiguous because no
+  // named color consists of leters 'a' to 'f' only.
+  const char* end = IsHexColorString(color_str) ?
+      SkParse::FindColor(("#" + color_str).c_str(), &temp_color) :
+      SkParse::FindColor(color_str.c_str(), &temp_color);
+  // Successful if FindColor() succeeds and |color_str| is completely consumed.
+  if (end && !*end) {
+    *color = temp_color;
+    return true;
+  }
+  return false;
 }
 
 }  // namespace chrome