[Icons NTP] Allow chrome-search:// large-icon and fallback-icon hosts to use <view_id>/<restricted_…

chrome/renderer/searchbox/searchbox.cc

 // Copyright 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/renderer/searchbox/searchbox.h"
 
 #include <string>
 
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/favicon/fallback_icon_url_parser.h"
 #include "chrome/common/favicon/favicon_url_parser.h"
+#include "chrome/common/favicon/large_icon_url_parser.h"
 #include "chrome/common/omnibox_focus_state.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/renderer/searchbox/searchbox_extension.h"
 #include "components/favicon_base/favicon_types.h"
 #include "content/public/renderer/render_view.h"
 #include "net/base/escape.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
 #include "third_party/WebKit/public/web/WebView.h"
@@ skipping 84 matching lines @@
   // default favicon.
   std::string id_part = raw_path.substr(parsed.path_index);
   InstantRestrictedID id;
   if (!GetInstantRestrictedIDFromPath(render_view_id, id_part, &id))
     return true;
 
   *rid = id;
   return true;
 }
 
+// Similar to GetRestrictedIDFromFaviconUrl(), but for large icons.
+bool GetRestrictedIDFromLargeIconUrl(int render_view_id,
+                                   const GURL& url,
+                                   std::string* favicon_params,
+                                   InstantRestrictedID* rid) {
+  // Strip leading slash.
+  std::string raw_path = url.path();
+  DCHECK_GT(raw_path.length(), (size_t) 0);
+  DCHECK_EQ(raw_path[0], '/');
+  raw_path = raw_path.substr(1);
+
+  LargeIconUrlParser parser;
+  if (!parser.Parse(raw_path))
+    return false;
+
+  *favicon_params = raw_path.substr(0, parser.path_index());
+
+  std::string id_part = raw_path.substr(parser.path_index());
+  InstantRestrictedID id;
+  if (!GetInstantRestrictedIDFromPath(render_view_id, id_part, &id))
+    return true;
+
+  *rid = id;
+  return true;
+}
+
+// Similar to GetRestrictedIDFromFaviconUrl(), but for fallback icons.
+bool GetRestrictedIDFromFallbackIconUrl(int render_view_id,
+                                        const GURL& url,
+                                        std::string* favicon_params,
+                                        InstantRestrictedID* rid) {
+  // Strip leading slash.
+  std::string raw_path = url.path();
+  DCHECK_GT(raw_path.length(), (size_t) 0);
+  DCHECK_EQ(raw_path[0], '/');
+  raw_path = raw_path.substr(1);
+
+  chrome::ParsedFallbackIconPath parser;
+  if (!parser.Parse(raw_path))
+    return false;
+
+  *favicon_params = raw_path.substr(0, parser.path_index());
+
+  std::string id_part = raw_path.substr(parser.path_index());
+  InstantRestrictedID id;
+  if (!GetInstantRestrictedIDFromPath(render_view_id, id_part, &id))
+    return true;
+
+  *rid = id;
+  return true;

[beaudoin, 2015/03/19 02:17:35]

This looks like three copies of the same code with a different parser. Couldn't
you extract that to a helper function and just pass in a different parser to
each?

I'm fine doing the more involved clean-up you propose later, but for the moment
I'd be happy if each method just called something like:
  return GetRestrictedIDFromUrl(render_view_id, url,
chrome::ParsedFallbackIconPath(), favicon_params, rid);

Looking at the code below, I don't even think you need these three flavors, you
can just instantiate a different parser in the three Generate*FromTransientURL.

[huangs, 2015/03/19 02:23:30]

I have grand plans to refactor these.  Triplicating code like a n00b is to
mitigate risk; the diff is add-only, I won't have to change how existing code
works, so easier to review and less chance of merge issues.  crbug.com/468320
states my plans to pay back the debt.

+}
+
+
 // Parses a thumbnail |url| and fills in |id| with the InstantRestrictedID
 // obtained from the |url|. |render_view_id| is the ID of the associated
 // RenderView.
 //
 // Valid |url| forms:
 // chrome-search://thumb/<view_id>/<restricted_id>
 //
 // If the |url| is valid, returns true and fills in |id| with restricted_id
 // value. If the |url| is invalid, returns false and |id| is not set.
 bool GetRestrictedIDFromThumbnailUrl(int render_view_id,
@@ skipping 75 matching lines @@
   std::string item_url;
   if (rid != -1 && GetMostVisitedItemWithID(rid, &item))
     item_url = item.url.spec();
 
   *url = GURL(base::StringPrintf("chrome-search://favicon/%s%s",
                                  favicon_params.c_str(),
                                  item_url.c_str()));
   return true;
 }
 
+bool SearchBox::GenerateLargeIconURLFromTransientURL(const GURL& transient_url,
+                                                     GURL* url) const {
+  std::string favicon_params;
+  InstantRestrictedID rid = -1;
+  bool success = internal::GetRestrictedIDFromLargeIconUrl(
+      render_view()->GetRoutingID(), transient_url, &favicon_params, &rid);
+  if (!success)
+    return false;
+
+  InstantMostVisitedItem item;
+  std::string item_url;
+  if (rid != -1 && GetMostVisitedItemWithID(rid, &item))
+    item_url = item.url.spec();
+
+  *url = GURL(base::StringPrintf("chrome-search://large-icon/%s%s",
+                                 favicon_params.c_str(),
+                                 item_url.c_str()));
+  return true;
+}
+
+bool SearchBox::GenerateFallbackIconURLFromTransientURL(
+    const GURL& transient_url,
+    GURL* url) const {
+  std::string favicon_params;
+  InstantRestrictedID rid = -1;
+  bool success = internal::GetRestrictedIDFromFallbackIconUrl(
+      render_view()->GetRoutingID(), transient_url, &favicon_params, &rid);
+  if (!success)
+    return false;
+
+  InstantMostVisitedItem item;
+  std::string item_url;
+  if (rid != -1 && GetMostVisitedItemWithID(rid, &item))
+    item_url = item.url.spec();
+
+  *url = GURL(base::StringPrintf("chrome-search://fallback-icon/%s%s",
+                                 favicon_params.c_str(),
+                                 item_url.c_str()));
+  return true;

[beaudoin, 2015/03/19 02:17:35]

Same here, I'd be fine with an
internal::GenerateURLFromTransientURL(transient_url, parser, endpoint) that
these three methods would call.

[huangs, 2015/03/19 02:23:30]

Same as above.

+}
+
 bool SearchBox::GenerateThumbnailURLFromTransientURL(const GURL& transient_url,
                                                      GURL* url) const {
   InstantRestrictedID rid = 0;
   if (!internal::GetRestrictedIDFromThumbnailUrl(render_view()->GetRoutingID(),
                                                  transient_url, &rid)) {
     return false;
   }
 
   GURL most_visited_item_url(GetURLForMostVisitedItem(rid));
   if (most_visited_item_url.is_empty())
@@ skipping 260 matching lines @@
 
 void SearchBox::Reset() {
   query_.clear();
   embedded_search_request_params_ = EmbeddedSearchRequestParams();
   suggestion_ = InstantSuggestion();
   start_margin_ = 0;
   is_focused_ = false;
   is_key_capture_enabled_ = false;
   theme_info_ = ThemeBackgroundInfo();
 }