[Favicon] Minor code cleanup.

chrome/common/favicon/favicon_url_parser.cc

 // Copyright 2013 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/common/favicon/favicon_url_parser.h"
 
 #include "base/strings/string_number_conversions.h"
 #include "components/favicon_base/favicon_types.h"
 #include "net/url_request/url_request.h"
 #include "ui/base/webui/web_ui_util.h"
@@ skipping 17 matching lines @@
   return path.compare(start_index, search.length(), search) == 0;
 }
 
 }  // namespace
 
 namespace chrome {
 
 bool ParseFaviconPath(const std::string& path,
                       int icon_types,
                       ParsedFaviconPath* parsed) {
+  DCHECK(parsed);

[pkotwicz, 2015/01/26 22:27:18]

I think the DCHECK() is not useful. If |parsed| is NULL, Line 39 will crash even
without a DCHECK()

[huangs, 2015/01/26 23:00:43]

It's a controlled-crash vs. uncontrolled-crash issue, which is the raison d'etre
of DCHECK().  I don't feel too strongly to push this though, so removed.

   parsed->is_icon_url = false;
   parsed->url = "";
   parsed->size_in_dip = gfx::kFaviconSize;
   parsed->device_scale_factor = 1.0f;
   parsed->path_index = std::string::npos;
 
   if (path.empty())
     return false;
 
   size_t parsed_index = 0;
@@ skipping 41 matching lines @@
         icon_types == favicon_base::FAVICON)
       return false;
 
     parsed_index = slash + 1;
   }
 
   if (HasSubstringAt(path, parsed_index, kIconURLParameter)) {
     parsed_index += strlen(kIconURLParameter);
     parsed->is_icon_url = true;
     parsed->url = path.substr(parsed_index);
-  } else {
+  } else if (HasSubstringAt(path, parsed_index, kOriginParameter)) {
     // 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
   // to translate favicon URLs using advanced parameters.
   // Example:
   //   "chrome-search://favicon/size/16@2x/<renderer-id>/<most-visited-id>"
   // would be translated to:
   //   "chrome-search://favicon/size/16@2x/<most-visited-item-with-given-id>".
   parsed->path_index = parsed_index;
   return true;
 }
 
 }  // namespace chrome