Completing high dpi support for Instant Extended.

chrome/renderer/searchbox/searchbox.cc

Index: chrome/renderer/searchbox/searchbox.cc
diff --git a/chrome/renderer/searchbox/searchbox.cc b/chrome/renderer/searchbox/searchbox.cc
index b0f7fbff723ade6ec9ab4e967798c4938d8ed358..96aaf50158b016aebab1c1ee81b36a4dfb39423f 100644
--- a/chrome/renderer/searchbox/searchbox.cc
+++ b/chrome/renderer/searchbox/searchbox.cc
@@ -10,6 +10,8 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/favicon/favicon_types.h"
+#include "chrome/common/favicon/favicon_url_parser.h"
 #include "chrome/common/omnibox_focus_state.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
@@ -49,21 +51,9 @@ bool AreMostVisitedItemsEqual(
 
 namespace internal {  // for testing
 
-// Parses |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://favicon/<view_id>/<restricted_id>
-// 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 GetInstantRestrictedIDFromURL(int render_view_id,
-                                   const GURL& url,
-                                   InstantRestrictedID* id) {
-  // Strip leading path.
-  std::string path = url.path().substr(1);
-
+bool GetInstantRestrictedIDFromPath(int render_view_id,

[samarth, 2013/07/24 00:11:05]

Please add a comment with that this does.

[pedro (no code reviews), 2013/07/24 01:10:55]

Done.

+                                    const std::string& path,
+                                    InstantRestrictedID* id) {
   // Check that the path is of Most visited item ID form.
   std::vector<std::string> tokens;
   if (Tokenize(path, "/", &tokens) != 2)
@@ -75,6 +65,70 @@ bool GetInstantRestrictedIDFromURL(int render_view_id,
   return base::StringToInt(tokens[1], id);
 }
 
+bool GetRestrictedIDFromFaviconUrl(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::ParsedFaviconPath parsed;
+  if (!chrome::ParseFaviconPath(raw_path, chrome::FAVICON, &parsed))
+    return false;
+
+  // The part of the URL which details the favicon parameters should be returned
+  // so the favicon URL can be reconstructed, by replacing the restricted_id
+  // with the actual URL from which the favicon is being requested.
+  *favicon_params = raw_path.substr(0, parsed.path_index);
+
+  // The part of the favicon URL which is supposed to contain the URL from
+  // which the favicon is being requested (i.e., the page's URL) actually
+  // contains a pair in the format "<view_id>/<restricted_id>". If the page's
+  // URL is not in the expected format then the execution must be stopped,
+  // returning |true|, indicating that the favicon URL should be translated
+  // without the page's URL part, to prevent search providers from spoofing
+  // the user's browsing history. For example, the following favicon URL
+  // "chrome-search://favicon/http://www.secretsite.com" it is not in the
+  // expected format "chrome-search://favicon/<view_id>/<restricted_id>" so
+  // the pages's URL part ("http://www.secretsite.com") should be removed
+  // entirely from the translated URL otherwise the search engine would know
+  // if the user has visited that page (by verifying whether the favicon URL
+  // returns an image for a particular page's URL); the translated URL in this
+  // case would be "chrome-search://favicon/" which would simply return the
+  // 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;
+}
+
+// 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,
+                                     const GURL& url,
+                                     InstantRestrictedID* id) {
+  // Strip leading slash.
+  std::string path = url.path();
+  DCHECK_GT(path.length(), (size_t) 0);
+  DCHECK_EQ(path[0], '/');
+  path = path.substr(1);
+
+  return internal::GetInstantRestrictedIDFromPath(render_view_id, path, id);

[samarth, 2013/07/24 00:11:05]

You're already in namespace internal so the qualifier is not necessary.

[pedro (no code reviews), 2013/07/24 01:10:55]

Done.

+}
+
 }  // namespace internal
 
 SearchBox::SearchBox(content::RenderView* render_view)
@@ -109,25 +163,29 @@ void SearchBox::DeleteMostVisitedItem(
 
 bool SearchBox::GenerateFaviconURLFromTransientURL(const GURL& transient_url,
                                                    GURL* url) const {
-  InstantRestrictedID rid = 0;
-  if (!internal::GetInstantRestrictedIDFromURL(render_view()->GetRoutingID(),
-                                               transient_url, &rid)) {
+  std::string favicon_params;
+  InstantRestrictedID rid = -1;
+  bool success = internal::GetRestrictedIDFromFaviconUrl(
+      render_view()->GetRoutingID(), transient_url, &favicon_params, &rid);
+  if (!success)
     return false;
-  }
 
-  GURL most_visited_item_url(GetURLForMostVisitedItem(rid));
-  if (most_visited_item_url.is_empty())
-    return false;
-  *url = GURL(base::StringPrintf("chrome-search://favicon/%s",
-                                 most_visited_item_url.spec().c_str()));
+  InstantMostVisitedItem item;
+  std::string item_url = "";

[samarth, 2013/07/24 00:11:05]

= "" is unnecessary.

[pedro (no code reviews), 2013/07/24 01:10:55]

Done.

+  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::GenerateThumbnailURLFromTransientURL(const GURL& transient_url,
                                                      GURL* url) const {
   InstantRestrictedID rid = 0;
-  if (!internal::GetInstantRestrictedIDFromURL(render_view()->GetRoutingID(),
-                                               transient_url, &rid)) {
+  if (!internal::GetRestrictedIDFromThumbnailUrl(render_view()->GetRoutingID(),
+                                                 transient_url, &rid)) {
     return false;
   }