Most visited thumbnails and favicons need id-based urls

chrome/browser/instant/instant_controller.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/browser/instant/instant_controller.h"
 
 #include "base/metrics/histogram.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_provider.h"
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/history_tab_helper.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/instant/instant_ntp.h"
 #include "chrome/browser/instant/instant_overlay.h"
+#include "chrome/browser/instant/instant_service.h"
+#include "chrome/browser/instant/instant_service_factory.h"
 #include "chrome/browser/instant/instant_tab.h"
 #include "chrome/browser/instant/search.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/search_engines/search_terms_data.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/ui/browser_instant_controller.h"
 #include "chrome/browser/ui/search/search_tab_helper.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
@@ skipping 160 matching lines @@
       std::string(),
       contents->GetBrowserContext());
   transient->SetExtraData(chrome::search::kInstantExtendedSearchTermsKey,
                           search_terms);
   controller->SetTransientEntry(transient);
 
   chrome::search::SearchTabHelper::FromWebContents(contents)->
       NavigationEntryUpdated();
 }
 
+bool GetURLForRestrictedId(Profile* profile, uint64 restricted_id, GURL* url) {

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

|profile| should be declared const, or const & — it's not an out parameter.

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

This is endemic in Chrome code.  Pretty much nowhere supports const Profile.  So
doing anywhere becomes a viral nightmare.

+  InstantService* instant_service =
+      InstantServiceFactory::GetForProfile(profile);
+  if (!instant_service)
+    return false;
+  return instant_service->GetURLForRestrictedId(restricted_id, url);
+}
+
+// Creates a new restriced id if one is not found.
+size_t GetRestrictedIDForURL(Profile* profile, const GURL& url) {

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

const |profile|.

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

Ditto.

+  InstantService* instant_service =

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

Perhaps the caller of these two functions should pass a const InstantService*
(or &), instead of Profile.

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

Pushing that up to the caller becomes quite cluttered.  I could keep a cached
member variable for a new |instant_service_| but that feels like overkill for
this modest replication.  If this pattern continues to grow I will do so
however.

+      InstantServiceFactory::GetForProfile(profile);
+  if (!instant_service)
+    return 0;
+  return instant_service->AddURL(url);
+}
+
 }  // namespace
 
 InstantController::InstantController(chrome::BrowserInstantController* browser,
                                      bool extended_enabled)
     : browser_(browser),
       extended_enabled_(extended_enabled),
       instant_enabled_(false),
       use_local_overlay_only_(true),
       model_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       last_omnibox_text_has_inline_autocompletion_(false),
@@ skipping 645 matching lines @@
       base::Time::Now().ToInternalValue(), info));
   static const size_t kMaxDebugEventSize = 2000;
   if (debug_events_.size() > kMaxDebugEventSize)
     debug_events_.pop_back();
 }
 
 void InstantController::ClearDebugEvents() {
   debug_events_.clear();
 }
 
-void InstantController::DeleteMostVisitedItem(const GURL& url) {
+void InstantController::DeleteMostVisitedItem(uint64 restricted_id) {
   history::TopSites* top_sites = browser_->profile()->GetTopSites();
   if (!top_sites)
     return;
 
-  top_sites->AddBlacklistedURL(url);
+  GURL url;
+  if (GetURLForRestrictedId(browser_->profile(), restricted_id, &url))
+    top_sites->AddBlacklistedURL(url);
 }
 
-void InstantController::UndoMostVisitedDeletion(const GURL& url) {
+void InstantController::UndoMostVisitedDeletion(uint64 restricted_id) {
   history::TopSites* top_sites = browser_->profile()->GetTopSites();
   if (!top_sites)
     return;
 
-  top_sites->RemoveBlacklistedURL(url);
+  GURL url;
+  if (GetURLForRestrictedId(browser_->profile(), restricted_id, &url))
+    top_sites->RemoveBlacklistedURL(url);
 }
 
 void InstantController::UndoAllMostVisitedDeletions() {
   history::TopSites* top_sites = browser_->profile()->GetTopSites();
   if (!top_sites)
     return;
 
   top_sites->ClearBlacklistedURLs();
 }
 
@@ skipping 566 matching lines @@
   history::TopSites* top_sites = browser_->profile()->GetTopSites();
   if (top_sites) {
     top_sites->GetMostVisitedURLs(
         base::Bind(&InstantController::OnMostVisitedItemsReceived,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
 void InstantController::OnMostVisitedItemsReceived(
     const history::MostVisitedURLList& data) {
-  std::vector<MostVisitedItem> most_visited_items;
+  std::vector<InstantMostVisitedItem> most_visited_items;
   for (size_t i = 0; i < data.size(); i++) {
     const history::MostVisitedURL& url = data[i];
 
-    MostVisitedItem item;
+    InstantMostVisitedItem item;
+    item.restricted_id = GetRestrictedIDForURL(browser_->profile(), url.url);
     item.url = url.url;
     item.title = url.title;
 
     most_visited_items.push_back(item);
   }
   SendMostVisitedItems(most_visited_items);
 }
 
 void InstantController::SendMostVisitedItems(
-    const std::vector<MostVisitedItem>& items) {
+    const std::vector<InstantMostVisitedItem>& items) {
   if (overlay_)
     overlay_->SendMostVisitedItems(items);
   if (ntp_)
     ntp_->SendMostVisitedItems(items);
   if (instant_tab_)
     instant_tab_->SendMostVisitedItems(items);
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_INSTANT_SENT_MOST_VISITED_ITEMS,
       content::Source<InstantController>(this),
       content::NotificationService::NoDetails());
 }