Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(311)

Unified Diff: chrome/browser/win/jumplist.cc

Issue 2859193005: Cache JumpList icons to avoid unnecessary creation and deletion (Closed)
Patch Set: Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/win/jumplist.cc
diff --git a/chrome/browser/win/jumplist.cc b/chrome/browser/win/jumplist.cc
index 24ac405c820d75e2c19b92784cc03fe21829bc8d..72d70f6b160df963e1b021e601b2c277aafab64e 100644
--- a/chrome/browser/win/jumplist.cc
+++ b/chrome/browser/win/jumplist.cc
@@ -287,6 +287,7 @@ void JumpList::OnMostVisitedURLsAvailable(
link->GetCommandLine()->AppendSwitchASCII(
switches::kWinJumplistAction, jumplist::kMostVisitedCategory);
link->set_title(!url.title.empty() ? url.title : url_string_wide);
+ link->set_url(url_string);
data->most_visited_pages_.push_back(link);
data->icon_urls_.push_back(std::make_pair(url_string, link));
}
@@ -339,6 +340,7 @@ bool JumpList::AddTab(const sessions::TabRestoreService::Tab& tab,
link->GetCommandLine()->AppendSwitchASCII(switches::kWinJumplistAction,
jumplist::kRecentlyClosedCategory);
link->set_title(current_navigation.title());
+ link->set_url(url);
data->recently_closed_pages_.push_back(link);
data->icon_urls_.push_back(std::make_pair(std::move(url), std::move(link)));
@@ -566,28 +568,97 @@ void JumpList::DeferredTabRestoreServiceChanged() {
StartLoadingFavicon();
}
+void JumpList::DeleteIconFiles(const base::FilePath& icon_dir,
+ JumpListCategory category) {
+ std::set<base::FilePath> iconpath_set;
grt (UTC plus 2) 2017/05/11 20:29:13 iconpath_set -> cached_files here, too
chengx 2017/05/12 01:04:24 Done.
+
+ if (category == JumpListCategory::MOST_VISITED) {
grt (UTC plus 2) 2017/05/11 20:29:13 this is a great place for a case statement. if you
chengx 2017/05/12 01:04:24 Done. Thanks so so so much for this suggestion!
+ for (auto url_path_pair : most_visited_map_)
grt (UTC plus 2) 2017/05/11 20:29:13 const auto& url_path_pair
chengx 2017/05/12 01:04:24 Done.
+ iconpath_set.insert(url_path_pair.second);
grt (UTC plus 2) 2017/05/11 20:29:12 flat_set seems like a good choice here -- you can
chengx 2017/05/12 01:04:24 Thanks. I've changed to flat_set. fyi, it should
+ } else if (category == JumpListCategory::RECENTLY_CLOSED) {
+ for (auto url_path_pair : recently_closed_map_)
grt (UTC plus 2) 2017/05/11 20:29:12 const auto&
chengx 2017/05/12 01:04:24 Done.
+ iconpath_set.insert(url_path_pair.second);
+ }
+ DeleteNonCachedFiles(icon_dir, iconpath_set);
+}
+
void JumpList::CreateIconFiles(const base::FilePath& icon_dir,
const ShellLinkItemList& item_list,
- size_t max_items) {
+ size_t max_items,
+ JumpListCategory category) {
// TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407.
SCOPED_UMA_HISTOGRAM_TIMER("WinJumplist.CreateIconFilesDuration");
- for (ShellLinkItemList::const_iterator item = item_list.begin();
- item != item_list.end() && max_items > 0; ++item, --max_items) {
- base::FilePath icon_path;
- if (CreateIconFile((*item)->icon_image(), icon_dir, &icon_path))
- (*item)->set_icon(icon_path.value(), 0);
+ // If the current URL is cached which means there's already an icon for this
+ // URL, then we simply re-use the icon. Otherwise, we need to create a new
+ // icon for this URL.
+
+ if (category == JumpListCategory::MOST_VISITED) {
grt (UTC plus 2) 2017/05/11 20:29:12 please use the technique above to collapse the dup
chengx 2017/05/12 01:04:24 Done. Again, thanks so much for this suggestion!
+ base::flat_map<std::string, base::FilePath> most_visited_map_updated;
+ for (ShellLinkItemList::const_iterator item = item_list.begin();
+ item != item_list.end() && max_items > 0; ++item, --max_items) {
+ if (most_visited_map_.find((*item)->url()) != most_visited_map_.end()) {
+ base::FilePath path = most_visited_map_[(*item)->url()];
+ (*item)->set_icon(most_visited_map_[(*item)->url()].value(), 0);
+ most_visited_map_updated[(*item)->url()] = path;
+ } else {
+ base::FilePath icon_path;
+ if (CreateIconFile((*item)->icon_image(), icon_dir, &icon_path)) {
+ (*item)->set_icon(icon_path.value(), 0);
+ most_visited_map_updated[(*item)->url()] = icon_path;
+ }
+ }
+ }
+ most_visited_map_.swap(most_visited_map_updated);
+ } else if (category == JumpListCategory::RECENTLY_CLOSED) {
+ base::flat_map<std::string, base::FilePath> recently_closed_map_updated;
+ for (ShellLinkItemList::const_iterator item = item_list.begin();
+ item != item_list.end() && max_items > 0; ++item, --max_items) {
+ if (recently_closed_map_.find((*item)->url()) !=
+ recently_closed_map_.end()) {
+ base::FilePath path = recently_closed_map_[(*item)->url()];
+ (*item)->set_icon(path.value(), 0);
+ recently_closed_map_updated[(*item)->url()] = path;
+ } else {
+ base::FilePath icon_path;
+ if (CreateIconFile((*item)->icon_image(), icon_dir, &icon_path)) {
+ (*item)->set_icon(icon_path.value(), 0);
+ recently_closed_map_updated[(*item)->url()] = icon_path;
+ }
+ }
+ }
+ recently_closed_map_.swap(recently_closed_map_updated);
}
}
void JumpList::UpdateIconFiles(const base::FilePath& icon_dir,
const ShellLinkItemList& page_list,
- size_t slot_limit) {
- DeleteDirectoryContentAndLogRuntime(icon_dir, kFileDeleteLimit);
-
- // Create new icons only when the directory exists and is empty.
- if (base::CreateDirectory(icon_dir) && base::IsDirectoryEmpty(icon_dir))
- CreateIconFiles(icon_dir, page_list, slot_limit);
+ size_t slot_limit,
+ JumpListCategory category) {
+ // Maximum number of icon files that each JumpList icon folder allows to hold.
+ size_t icon_limit = (category == JumpListCategory::MOST_VISITED) ? 12 : 6;
+
+ // Clear the JumpList icon folder at |icon_dir| and the cache when
+ // 1) "Most visited" category updates for the 1st time after Chrome is
+ // launched. This actually happens right after Chrome is launched.
+ // 2) "Recently closed" category updates for the 1st time after Chrome is
+ // launched.
+ // 3) The number of icons in |icon_dir| has exceeded the limit.
+ if ((category == JumpListCategory::MOST_VISITED &&
+ most_visited_map_.empty()) ||
+ (category == JumpListCategory::RECENTLY_CLOSED &&
+ recently_closed_map_.empty()) ||
+ FilesExceedLimitInDir(icon_dir, icon_limit)) {
+ DeleteDirectoryContentAndLogRuntime(icon_dir, kFileDeleteLimit);
+ most_visited_map_.clear();
+ recently_closed_map_.clear();
+ // Create new icons only when the directory exists and is empty.
+ if (base::CreateDirectory(icon_dir) && base::IsDirectoryEmpty(icon_dir))
+ CreateIconFiles(icon_dir, page_list, slot_limit, category);
+ } else if (base::CreateDirectory(icon_dir)) {
+ CreateIconFiles(icon_dir, page_list, slot_limit, category);
+ DeleteIconFiles(icon_dir, category);
+ }
}
bool JumpList::UpdateJumpList(
@@ -654,7 +725,7 @@ bool JumpList::UpdateJumpList(
profile_dir, FILE_PATH_LITERAL("MostVisited"));
UpdateIconFiles(icon_dir_most_visited, most_visited_pages,
- most_visited_items);
+ most_visited_items, JumpListCategory::MOST_VISITED);
icons_to_create += std::min(most_visited_pages.size(), most_visited_items);
}
@@ -665,7 +736,7 @@ bool JumpList::UpdateJumpList(
profile_dir, FILE_PATH_LITERAL("RecentClosed"));
UpdateIconFiles(icon_dir_recent_closed, recently_closed_pages,
- recently_closed_items);
+ recently_closed_items, JumpListCategory::RECENTLY_CLOSED);
icons_to_create +=
std::min(recently_closed_pages.size(), recently_closed_items);

Powered by Google App Engine
This is Rietveld 408576698