Skip to content

Commit

Permalink
Rip out browser-side RID caching for most visited items.
Browse files Browse the repository at this point in the history 
(1) Most visited thumbnails and favicons no longer require id-based urls. Updated ThumbnailSource and FaviconSource to serve requests of the form:
   chrome-search://favicon/<most_visited_item_favicon_url>
      chrome-search://thumb/<most_visited_item_thumbnail_url>

(2) Removed |most_visited_items_cache_| from InstantIOContext and InstantService.
(3) SearchTabHelper will track the last sent most visited items. This prevents duplicate most visited item related IPCs while switching tabs.

BUG=239253, 225760, 242667
TEST=none

Review URL: https://chromiumcodereview.appspot.com/15907006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@205026 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
kmadhusu@chromium.org committed Jun 8, 2013
1 parent 18d7de1 commit ab01dd7
Show file tree
Hide file tree
Showing 24 changed files with 275 additions and 284 deletions.
41 changes: 1 addition & 40 deletions chrome/browser/search/instant_io_context.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ InstantIOContext* GetDataForRequest(const net::URLRequest* request) {

const char InstantIOContext::kInstantIOContextKeyName[] = "instant_io_context";

InstantIOContext::InstantIOContext()
: most_visited_item_cache_(kMaxInstantMostVisitedItemCacheSize) {
InstantIOContext::InstantIOContext() {
// The InstantIOContext is created on the UI thread but is accessed
// on the IO thread.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
Expand Down Expand Up @@ -76,15 +75,6 @@ void InstantIOContext::ClearInstantProcessesOnIO(
instant_io_context->process_ids_.clear();
}

// static
void InstantIOContext::AddMostVisitedItemsOnIO(
scoped_refptr<InstantIOContext> instant_io_context,
std::vector<InstantMostVisitedItemIDPair> items) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
instant_io_context->most_visited_item_cache_.AddItemsWithRestrictedID(items);
}


// static
bool InstantIOContext::ShouldServiceRequest(const net::URLRequest* request) {
const content::ResourceRequestInfo* info =
Expand All @@ -104,36 +94,7 @@ bool InstantIOContext::ShouldServiceRequest(const net::URLRequest* request) {
return false;
}

// static
bool InstantIOContext::GetURLForMostVisitedItemID(
const net::URLRequest* request,
InstantRestrictedID most_visited_item_id,
GURL* url) {
InstantIOContext* instant_io_context = GetDataForRequest(request);
if (!instant_io_context) {
*url = GURL();
return false;
}

return instant_io_context->GetURLForMostVisitedItemID(most_visited_item_id,
url);
}

bool InstantIOContext::IsInstantProcess(int process_id) const {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
return process_ids_.find(process_id) != process_ids_.end();
}

bool InstantIOContext::GetURLForMostVisitedItemID(
InstantRestrictedID most_visited_item_id,
GURL* url) const {
InstantMostVisitedItem item;
if (most_visited_item_cache_.GetItemWithRestrictedID(most_visited_item_id,
&item)) {
*url = item.url;
return true;
}

*url = GURL();
return false;
}
22 changes: 0 additions & 22 deletions chrome/browser/search/instant_io_context.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,10 @@
#ifndef CHROME_BROWSER_SEARCH_INSTANT_IO_CONTEXT_H_
#define CHROME_BROWSER_SEARCH_INSTANT_IO_CONTEXT_H_

#include <map>
#include <set>
#include <vector>

#include "base/basictypes.h"
#include "base/memory/ref_counted.h"
#include "chrome/common/instant_restricted_id_cache.h"

class GURL;

Expand Down Expand Up @@ -51,22 +48,10 @@ class InstantIOContext : public base::RefCountedThreadSafe<InstantIOContext> {
static void ClearInstantProcessesOnIO(
scoped_refptr<InstantIOContext> instant_io_context);

// Associates the |most_visited_item_id| with the |url|.
static void AddMostVisitedItemsOnIO(
scoped_refptr<InstantIOContext> instant_io_context,
std::vector<InstantMostVisitedItemIDPair> items);

// Determine if this chrome-search: request is coming from an Instant render
// process.
static bool ShouldServiceRequest(const net::URLRequest* request);

// If there is a mapping for the |most_visited_item_id|, sets |url| and
// returns true.
static bool GetURLForMostVisitedItemID(
const net::URLRequest* request,
InstantRestrictedID most_visited_item_id,
GURL* url);

protected:
virtual ~InstantIOContext();

Expand All @@ -77,18 +62,11 @@ class InstantIOContext : public base::RefCountedThreadSafe<InstantIOContext> {
// |process_ids_|.
bool IsInstantProcess(int process_id) const;

bool GetURLForMostVisitedItemID(InstantRestrictedID most_visited_item_id,
GURL* url) const;

// The process IDs associated with Instant processes. Mirror of the process
// IDs in InstantService. Duplicated here for synchronous access on the IO
// thread.
std::set<int> process_ids_;

// The Most Visited item cache. Mirror of the Most Visited item cache in
// InstantService. Duplicated here for synchronous access on the IO thread.
InstantRestrictedIDCache<InstantMostVisitedItem> most_visited_item_cache_;

DISALLOW_COPY_AND_ASSIGN(InstantIOContext);
};

Expand Down
86 changes: 11 additions & 75 deletions chrome/browser/search/instant_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "chrome/browser/search/instant_service_factory.h"
#include "chrome/browser/search/local_ntp_source.h"
#include "chrome/browser/search/most_visited_iframe_source.h"
#include "chrome/browser/search/search.h"
#include "chrome/browser/search/suggestion_iframe_source.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_instant_controller.h"
Expand All @@ -38,7 +39,6 @@ using content::BrowserThread;

InstantService::InstantService(Profile* profile)
: profile_(profile),
most_visited_item_cache_(kMaxInstantMostVisitedItemCacheSize),
weak_ptr_factory_(this) {
// Stub for unit tests.
if (!BrowserThread::CurrentlyOn(BrowserThread::UI))
Expand Down Expand Up @@ -78,52 +78,6 @@ InstantService::InstantService(Profile* profile)
InstantService::~InstantService() {
}

// static
const std::string InstantService::MaybeTranslateInstantPathOnUI(
Profile* profile, const std::string& path) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
InstantService* instant_service =
InstantServiceFactory::GetForProfile(profile);
if (!instant_service)
return path;

InstantRestrictedID restricted_id = 0;
DCHECK_EQ(sizeof(InstantRestrictedID), sizeof(int));
if (base::StringToInt(path, &restricted_id)) {
InstantMostVisitedItem item;
if (instant_service->GetMostVisitedItemForID(restricted_id, &item))
return item.url.spec();
}
return path;
}

const std::string InstantService::MaybeTranslateInstantPathOnIO(
const net::URLRequest* request, const std::string& path) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));

InstantRestrictedID restricted_id = 0;
DCHECK_EQ(sizeof(InstantRestrictedID), sizeof(int));
if (base::StringToInt(path, &restricted_id)) {
GURL url;
if (InstantIOContext::GetURLForMostVisitedItemID(request,
restricted_id,
&url)) {
return url.spec();
}
}
return path;
}

// static
bool InstantService::IsInstantPath(const GURL& url) {
// Strip leading slash.
std::string path = url.path().substr(1);

// Check that path is of Most Visited item ID form.
InstantRestrictedID dummy = 0;
return base::StringToInt(path, &dummy);
}

void InstantService::AddInstantProcess(int process_id) {
process_ids_.insert(process_id);

Expand All @@ -140,22 +94,6 @@ bool InstantService::IsInstantProcess(int process_id) const {
return process_ids_.find(process_id) != process_ids_.end();
}

void InstantService::AddMostVisitedItems(
const std::vector<InstantMostVisitedItem>& items) {
most_visited_item_cache_.AddItems(items);

// Post task to the IO thread to copy the data.
if (instant_io_context_.get()) {
std::vector<InstantMostVisitedItemIDPair> items;
most_visited_item_cache_.GetCurrentItems(&items);
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
base::Bind(&InstantIOContext::AddMostVisitedItemsOnIO,
instant_io_context_,
items));
}
}

void InstantService::DeleteMostVisitedItem(const GURL& url) {
history::TopSites* top_sites = profile_->GetTopSites();
if (!top_sites)
Expand All @@ -181,8 +119,8 @@ void InstantService::UndoAllMostVisitedDeletions() {
}

void InstantService::GetCurrentMostVisitedItems(
std::vector<InstantMostVisitedItemIDPair>* items) const {
most_visited_item_cache_.GetCurrentItems(items);
std::vector<InstantMostVisitedItem>* items) const {
*items = most_visited_items_;
}

void InstantService::Shutdown() {
Expand Down Expand Up @@ -231,29 +169,27 @@ void InstantService::Observe(int type,
}
}

bool InstantService::GetMostVisitedItemForID(
InstantRestrictedID most_visited_item_id,
InstantMostVisitedItem* item) const {
return most_visited_item_cache_.GetItemWithRestrictedID(
most_visited_item_id, item);
}

void InstantService::OnMostVisitedItemsReceived(
const history::MostVisitedURLList& data) {
// Android doesn't use Browser/BrowserList. Do nothing for Android platform.
#if !defined(OS_ANDROID)
history::MostVisitedURLList reordered_data(data);
history::TopSites::MaybeShuffle(&reordered_data);

std::vector<InstantMostVisitedItem> most_visited_items;
std::vector<InstantMostVisitedItem> new_most_visited_items;
for (size_t i = 0; i < reordered_data.size(); i++) {
const history::MostVisitedURL& url = reordered_data[i];
InstantMostVisitedItem item;
item.url = url.url;
item.title = url.title;
most_visited_items.push_back(item);
new_most_visited_items.push_back(item);
}
AddMostVisitedItems(most_visited_items);
if (chrome::AreMostVisitedItemsEqual(new_most_visited_items,
most_visited_items_)) {
return;
}

most_visited_items_ = new_most_visited_items;

const BrowserList* browser_list =
BrowserList::GetInstance(chrome::GetActiveDesktop());
Expand Down
35 changes: 6 additions & 29 deletions chrome/browser/search/instant_service.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@
#include <map>
#include <set>
#include <string>
#include <vector>

#include "base/basictypes.h"
#include "base/compiler_specific.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/history/history_types.h"
#include "chrome/common/instant_restricted_id_cache.h"
#include "chrome/common/instant_types.h"
#include "components/browser_context_keyed_service/browser_context_keyed_service.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
Expand All @@ -34,19 +35,6 @@ class InstantService : public BrowserContextKeyedService,
explicit InstantService(Profile* profile);
virtual ~InstantService();

// A utility to translate an Instant path if it is of Most Visited item ID
// form. If path is a Most Visited item ID and we have a URL for it, then
// this URL is returned in string form. The |path| is a URL fragment
// corresponding to the path of url with the leading slash ("/") stripped.
// For example, chrome-search://favicon/72 would yield a |path| value of "72",
// and since 72 is a valid uint64 the path is translated to a valid url,
// "http://bingo.com/", say.
static const std::string MaybeTranslateInstantPathOnUI(
Profile* profile, const std::string& path);
static const std::string MaybeTranslateInstantPathOnIO(
const net::URLRequest* request, const std::string& path);
static bool IsInstantPath(const GURL& url);

// Add, remove, and query RenderProcessHost IDs that are associated with
// Instant processes.
void AddInstantProcess(int process_id);
Expand All @@ -60,10 +48,6 @@ class InstantService : public BrowserContextKeyedService,

// Most visited item API.

// Adds |items| to the |most_visited_item_cache_| assigning restricted IDs in
// the process.
void AddMostVisitedItems(const std::vector<InstantMostVisitedItem>& items);

// Invoked by the InstantController when the Instant page wants to delete a
// Most Visited item.
void DeleteMostVisitedItem(const GURL& url);
Expand All @@ -76,11 +60,9 @@ class InstantService : public BrowserContextKeyedService,
// Most Visited deletions.
void UndoAllMostVisitedDeletions();

// Returns the last added InstantMostVisitedItems. After the call to
// |AddMostVisitedItems|, the caller should call this to get the items with
// the assigned IDs.
// Returns the last added InstantMostVisitedItems.
void GetCurrentMostVisitedItems(
std::vector<InstantMostVisitedItemIDPair>* items) const;
std::vector<InstantMostVisitedItem>* items) const;

private:
// Overridden from BrowserContextKeyedService:
Expand All @@ -91,11 +73,6 @@ class InstantService : public BrowserContextKeyedService,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;

// If the |most_visited_item_id| is found in the cache, sets the |item| to it
// and returns true.
bool GetMostVisitedItemForID(InstantRestrictedID most_visited_item_id,
InstantMostVisitedItem* item) const;

// Called when we get new most visited items from TopSites, registered as an
// async callback. Parses them and sends them to the renderer via
// SendMostVisitedItems.
Expand All @@ -106,8 +83,8 @@ class InstantService : public BrowserContextKeyedService,
// The process ids associated with Instant processes.
std::set<int> process_ids_;

// A cache of the InstantMostVisitedItems sent to the Instant Pages.
InstantRestrictedIDCache<InstantMostVisitedItem> most_visited_item_cache_;
// InstantMostVisitedItems sent to the Instant Pages.
std::vector<InstantMostVisitedItem> most_visited_items_;

content::NotificationRegistrar registrar_;

Expand Down
15 changes: 15 additions & 0 deletions chrome/browser/search/search.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -740,4 +740,19 @@ void ResetInstantExtendedOptInStateGateForTest() {
instant_extended_opt_in_state_gate = false;
}

bool AreMostVisitedItemsEqual(
const std::vector<InstantMostVisitedItem>& items_a,
const std::vector<InstantMostVisitedItem>& items_b) {
if (items_a.size() != items_b.size())
return false;

for (size_t i = 0; i < items_b.size(); ++i) {
if (items_b[i].url != items_a[i].url ||
items_b[i].title != items_a[i].title) {
return false;
}
}
return true;
}

} // namespace chrome
6 changes: 6 additions & 0 deletions chrome/browser/search/search.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#include "base/basictypes.h"
#include "base/string16.h"
#include "chrome/common/instant_types.h"

class GURL;
class Profile;
Expand Down Expand Up @@ -231,6 +232,11 @@ bool DefaultSearchProviderSupportsInstant(Profile* profile);
// once.
void ResetInstantExtendedOptInStateGateForTest();

// Returns true if |items_a| and |items_b| are equal.
bool AreMostVisitedItemsEqual(
const std::vector<InstantMostVisitedItem>& items_a,
const std::vector<InstantMostVisitedItem>& items_b);

} // namespace chrome

#endif // CHROME_BROWSER_SEARCH_SEARCH_H_
Loading

0 comments on commit ab01dd7

Please sign in to comment.