Most visited thumbnails and favicons need id-based urls

chrome/renderer/searchbox/searchbox_extension.cc

Index: chrome/renderer/searchbox/searchbox_extension.cc
diff --git a/chrome/renderer/searchbox/searchbox_extension.cc b/chrome/renderer/searchbox/searchbox_extension.cc
index 62b8397536d177b9dbba5cf96dc8cbf9c50aec6a..dc07cc6c950325712da9cae76b2784b7d441b01f 100644
--- a/chrome/renderer/searchbox/searchbox_extension.cc
+++ b/chrome/renderer/searchbox/searchbox_extension.cc
@@ -63,6 +63,20 @@ void Dispatch(WebKit::WebFrame* frame, const WebKit::WebString& script) {
   frame->executeScript(WebKit::WebScriptSource(script));
 }
 
+v8::Handle<v8::String> GenerateThumbnailURL(uint64 restricted_id) {
+  std::ostringstream ostr;
+  ostr << "chrome-search://thumb/" << restricted_id;

[palmer, 2013/03/11 20:42:31]

This seems kind of heavy-weight (including having to convert it to a string,
below). Maybe just use base/stringprintf.h (already #included)?

[dhollowa, 2013/03/11 23:27:59]

Done.  I was blindly copy/pasting...

+  GURL url = GURL(ostr.str());
+  return UTF8ToV8String(url.spec());
+}
+
+v8::Handle<v8::String> GenerateFaviconURL(uint64 restricted_id) {
+  std::ostringstream ostr;
+  ostr << "chrome-search://favicon/" << restricted_id;
+  GURL url = GURL(ostr.str());
+  return UTF8ToV8String(url.spec());
+}
+
 }  // namespace
 
 namespace extensions_v8 {
@@ -872,20 +886,16 @@ v8::Handle<v8::Value> SearchBoxExtensionWrapper::ShowOverlay(
 v8::Handle<v8::Value> SearchBoxExtensionWrapper::GetMostVisitedItems(
     const v8::Arguments& args) {
   content::RenderView* render_view = GetRenderView();
-  if (!render_view) return v8::Undefined();
-
+  if (!render_view)
+    return v8::Undefined();
   DVLOG(1) << render_view << " GetMostVisitedItems";
 
-  const std::vector<MostVisitedItem>& items =
-      SearchBox::Get(render_view)->GetMostVisitedItems();
-  v8::Handle<v8::Array> items_array = v8::Array::New(items.size());
-  for (size_t i = 0; i < items.size(); ++i) {
-
-    const string16 url = UTF8ToUTF16(items[i].url.spec());
-    const string16 host = UTF8ToUTF16(items[i].url.host());
-    int restrict_id =
-        SearchBox::Get(render_view)->UrlToRestrictedId(url);
+  SearchBox* search_box = SearchBox::Get(render_view);

[palmer, 2013/03/11 20:42:31]

Can |search_box| be declared const?

[dhollowa, 2013/03/11 23:27:59]

Yes. Done.

 
+  const std::vector<InstantMostVisitedItem>& instant_mv_items =
+      search_box->GetMostVisitedItems();
+  v8::Handle<v8::Array> v8_mv_items = v8::Array::New(instant_mv_items.size());
+  for (size_t i = 0; i < instant_mv_items.size(); ++i) {
     // We set the "dir" attribute of the title, so that in RTL locales, a LTR
     // title is rendered left-to-right and truncated from the right. For
     // example, the title of http://msdn.microsoft.com/en-us/default.aspx is
@@ -898,33 +908,31 @@ v8::Handle<v8::Value> SearchBoxExtensionWrapper::GetMostVisitedItems(
     // title will be rendered as "!Yahoo" if its "dir" attribute is not set to
     // "ltr".
     std::string direction;
-    if (base::i18n::StringContainsStrongRTLChars(items[i].title))
+    if (base::i18n::StringContainsStrongRTLChars(instant_mv_items[i].title))
       direction = kRTLHtmlTextDirection;
     else
       direction = kLTRHtmlTextDirection;
 
-    string16 title = items[i].title;
+    string16 title = instant_mv_items[i].title;
     if (title.empty())
-      title = url;
+      title = UTF8ToUTF16(instant_mv_items[i].url.spec());
 
     v8::Handle<v8::Object> item = v8::Object::New();
     item->Set(v8::String::New("rid"),
-              v8::Int32::New(restrict_id));
+              v8::Int32::New(instant_mv_items[i].restricted_id));
     item->Set(v8::String::New("thumbnailUrl"),
-              UTF16ToV8String(SearchBox::Get(render_view)->
-                              GenerateThumbnailUrl(restrict_id)));
+              GenerateThumbnailURL(instant_mv_items[i].restricted_id));
     item->Set(v8::String::New("faviconUrl"),
-              UTF16ToV8String(SearchBox::Get(render_view)->
-                              GenerateFaviconUrl(restrict_id)));
+              GenerateFaviconURL(instant_mv_items[i].restricted_id));
     item->Set(v8::String::New("title"),
               UTF16ToV8String(title));
-    item->Set(v8::String::New("domain"), UTF16ToV8String(host));
-    item->Set(v8::String::New("direction"),
-              UTF8ToV8String(direction));
+    item->Set(v8::String::New("domain"),
+              UTF8ToV8String(instant_mv_items[i].url.host()));
+    item->Set(v8::String::New("direction"), UTF8ToV8String(direction));
 
-    items_array->Set(i, item);
+    v8_mv_items->Set(i, item);
   }
-  return items_array;
+  return v8_mv_items;
 }
 
 // static