[Favicon] Add new fallback icon rendering flow.

chrome/common/favicon/favicon_url_parser.cc

Index: chrome/common/favicon/favicon_url_parser.cc
diff --git a/chrome/common/favicon/favicon_url_parser.cc b/chrome/common/favicon/favicon_url_parser.cc
index 16d78af4e9f816f4b8984310c7bd4518c5fe9118..49284fe00736eaca9d3e63b4bf79caff2d0d49f1 100644
--- a/chrome/common/favicon/favicon_url_parser.cc
+++ b/chrome/common/favicon/favicon_url_parser.cc
@@ -35,6 +35,7 @@ namespace chrome {
 bool ParseFaviconPath(const std::string& path,
                       int icon_types,
                       ParsedFaviconPath* parsed) {
+  DCHECK(parsed);

[pkotwicz, 2015/01/21 19:44:15]

Nit: Remove the DCHECK. We will crash on the next line anyways if parsed is
nullptr

[huangs, 2015/01/22 01:13:27]

I'd rather keep this.  Controlled crash is better than uncontrolled crash, and
this also serves as documentation.

Meanwhile this is cleanup, so moved to the other CL.

   parsed->is_icon_url = false;
   parsed->url = "";
   parsed->size_in_dip = gfx::kFaviconSize;
@@ -96,24 +97,22 @@ bool ParseFaviconPath(const std::string& path,
     parsed_index += strlen(kIconURLParameter);
     parsed->is_icon_url = true;
     parsed->url = path.substr(parsed_index);
-  } else {
+  } else if (HasSubstringAt(path, parsed_index, kOriginParameter)) {

[pkotwicz, 2015/01/21 19:44:15]

Thanks! Can you move this change to a separate CL?

[huangs, 2015/01/22 01:13:27]

Done.

     // 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);
+    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;
+    // 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.
-      parsed->url = GURL(possibly_invalid_url).GetOrigin().spec();
-    } else {
-      parsed->url = path.substr(parsed_index);
-    }
+    // Strip the path beyond the top-level domain.
+    parsed->url = GURL(possibly_invalid_url).GetOrigin().spec();
+  } else {
+    parsed->url = path.substr(parsed_index);
   }
 
   // The parsed index needs to be returned in order to allow Instant Extended