Index: chrome/browser/win/jumplist.cc |
diff --git a/chrome/browser/win/jumplist.cc b/chrome/browser/win/jumplist.cc |
index 461c5a4a45b5371c7c64ff66d79060b35a5c7bdd..cd3dbaec56c43925d29adafe0ebb61ceb387d02c 100644 |
--- a/chrome/browser/win/jumplist.cc |
+++ b/chrome/browser/win/jumplist.cc |
@@ -181,6 +181,8 @@ bool UpdateJumpList(const wchar_t* app_id, |
const base::FilePath& icon_dir, |
const ShellLinkItemList& most_visited_pages, |
const ShellLinkItemList& recently_closed_pages, |
+ bool should_update_most_visited, |
+ bool should_update_recent_closed, |
IncognitoModePrefs::Availability incognito_availability) { |
// JumpList is implemented only on Windows 7 or later. |
// So, we should return now when this function is called on earlier versions |
@@ -192,43 +194,74 @@ bool UpdateJumpList(const wchar_t* app_id, |
if (!jumplist_updater.BeginUpdate()) |
return false; |
- // We allocate 60% of the given JumpList slots to "most-visited" items |
- // and 40% to "recently-closed" items, respectively. |
- // Nevertheless, if there are not so many items in |recently_closed_pages|, |
- // we give the remaining slots to "most-visited" items. |
// The default maximum number of items to display in JumpList is 10. |
// https://msdn.microsoft.com/en-us/library/windows/desktop/dd378398(v=vs.85).aspx |
grt (UTC plus 2)
2017/04/25 08:29:39
hot tip: this can be reduced to https://msdn.micro
chengx
2017/04/25 23:00:24
Thanks! I've moved the change of this section to a
|
- const int kMostVisited = 60; |
- const int kRecentlyClosed = 40; |
+ // The "Most visited" category title always takes 1 of the JumpList slots if |
+ // |most_visited_pages| isn't empty. |
+ // The "Recently closed" category title will also take 1 if |
+ // |recently_closed_pages| isn't empty. |
+ // For the remaining slots, we allocate 5/8 (i.e., 5 slots if both categories |
+ // present) to "most-visited" items and 3/8 (i.e., 3 slots if both categories |
+ // present) to "recently-closed" items, respectively. |
+ // Nevertheless, if there are not so many items in |recently_closed_pages|, |
+ // we give the remaining slots to "most-visited" items. |
+ |
+ const int kMostVisited = 50; |
+ const int kRecentlyClosed = 30; |
const int kTotal = kMostVisited + kRecentlyClosed; |
+ |
+ // Adjust the available jumplist slots accordingly. |
+ size_t user_max_items_adjusted = jumplist_updater.user_max_items(); |
+ if (!most_visited_pages.empty()) |
+ user_max_items_adjusted--; |
+ if (!recently_closed_pages.empty()) |
+ user_max_items_adjusted--; |
+ |
size_t most_visited_items = |
- MulDiv(jumplist_updater.user_max_items(), kMostVisited, kTotal); |
- size_t recently_closed_items = |
- jumplist_updater.user_max_items() - most_visited_items; |
+ MulDiv(user_max_items_adjusted, kMostVisited, kTotal); |
+ size_t recently_closed_items = user_max_items_adjusted - most_visited_items; |
if (recently_closed_pages.size() < recently_closed_items) { |
most_visited_items += recently_closed_items - recently_closed_pages.size(); |
recently_closed_items = recently_closed_pages.size(); |
} |
- // Delete the content in JumpListIcons folder and log the results to UMA. |
- DeleteDirectoryContentAndLogResults(icon_dir, kFileDeleteLimit); |
- |
- // If JumpListIcons directory doesn't exist (we have tried to create it |
- // already) or is not empty, skip updating the jumplist icons. The jumplist |
- // links should be updated anyway, as it doesn't involve disk IO. In this |
- // case, Chrome's icon will be used for the new links. |
+ if (should_update_most_visited) { |
+ // Delete the content in JumpListIconsMostVisited folder and log the results |
+ // to UMA. |
+ base::FilePath icon_dir_most_visited = icon_dir.DirName().Append( |
+ icon_dir.BaseName().value() + FILE_PATH_LITERAL("MostVisited")); |
+ |
+ DeleteDirectoryContentAndLogResults(icon_dir_most_visited, |
+ kFileDeleteLimit); |
+ |
+ // If the directory doesn't exist (we have tried to create it in |
+ // DeleteDirectoryContentAndLogResults) or is not empty, skip updating the |
+ // jumplist icons. The jumplist links should be updated anyway, as it |
+ // doesn't involve disk IO. In this case, Chrome's icon will be used for the |
+ // new links. |
+ if (base::DirectoryExists(icon_dir_most_visited) && |
+ base::IsDirectoryEmpty(icon_dir_most_visited)) { |
+ // Create icon files for shortcuts in the "Most Visited" category. |
+ CreateIconFiles(icon_dir_most_visited, most_visited_pages, |
+ most_visited_items); |
+ } |
+ } |
- if (base::DirectoryExists(icon_dir) && base::IsDirectoryEmpty(icon_dir)) { |
- // TODO(chengx): Remove this UMA metric after fixing http://crbug.com/40407. |
- UMA_HISTOGRAM_COUNTS_100( |
- "WinJumplist.CreateIconFilesCount", |
- most_visited_pages.size() + recently_closed_pages.size()); |
+ if (should_update_recent_closed) { |
+ // Delete the content in JumpListIconsRecentClosed folder and log the |
+ // results to UMA. |
+ base::FilePath icon_dir_recent_closed = icon_dir.DirName().Append( |
+ icon_dir.BaseName().value() + FILE_PATH_LITERAL("RecentClosed")); |
- // Create icon files for shortcuts in the "Most Visited" category. |
- CreateIconFiles(icon_dir, most_visited_pages, most_visited_items); |
+ DeleteDirectoryContentAndLogResults(icon_dir_recent_closed, |
+ kFileDeleteLimit); |
- // Create icon files for shortcuts in the "Recently Closed" category. |
- CreateIconFiles(icon_dir, recently_closed_pages, recently_closed_items); |
+ if (base::DirectoryExists(icon_dir_recent_closed) && |
+ base::IsDirectoryEmpty(icon_dir_recent_closed)) { |
+ // Create icon files for shortcuts in the "Recently Closed" category. |
+ CreateIconFiles(icon_dir_recent_closed, recently_closed_pages, |
+ recently_closed_items); |
+ } |
} |
// TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407. |
@@ -265,6 +298,8 @@ bool UpdateJumpList(const wchar_t* app_id, |
void RunUpdateJumpList(IncognitoModePrefs::Availability incognito_availability, |
const std::wstring& app_id, |
const base::FilePath& icon_dir, |
+ bool should_update_most_visited, |
+ bool should_update_recent_closed, |
base::RefCountedData<JumpListData>* ref_counted_data) { |
JumpListData* data = &ref_counted_data->data; |
ShellLinkItemList local_most_visited_pages; |
@@ -286,7 +321,8 @@ void RunUpdateJumpList(IncognitoModePrefs::Availability incognito_availability, |
// jumplist links are updated anyway, while the jumplist icons may not as |
// mentioned above. |
UpdateJumpList(app_id.c_str(), icon_dir, local_most_visited_pages, |
- local_recently_closed_pages, incognito_availability); |
+ local_recently_closed_pages, should_update_most_visited, |
+ should_update_recent_closed, incognito_availability); |
} |
} // namespace |
@@ -299,15 +335,17 @@ JumpList::JumpList(Profile* profile) |
: RefcountedKeyedService(content::BrowserThread::GetTaskRunnerForThread( |
content::BrowserThread::UI)), |
profile_(profile), |
+ should_update_most_visited_(true), |
+ should_update_recent_closed_(true), |
jumplist_data_(new base::RefCountedData<JumpListData>), |
task_id_(base::CancelableTaskTracker::kBadTaskId), |
- update_jumplisticons_task_runner_(base::CreateCOMSTATaskRunnerWithTraits( |
+ update_jumplist_task_runner_(base::CreateCOMSTATaskRunnerWithTraits( |
base::TaskTraits() |
.WithPriority(base::TaskPriority::USER_VISIBLE) |
.WithShutdownBehavior( |
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN) |
.MayBlock())), |
- delete_jumplisticonsold_task_runner_( |
+ delete_jumplisticons_task_runner_( |
base::CreateSequencedTaskRunnerWithTraits( |
base::TaskTraits() |
.WithPriority(base::TaskPriority::BACKGROUND) |
@@ -413,6 +451,11 @@ void JumpList::OnMostVisitedURLsAvailable( |
} |
} |
+ // Since only the "Most Visited" category changes, mark the flags so that icon |
+ // files only for this category are updated later on. |
+ should_update_most_visited_ = true; |
+ should_update_recent_closed_ = false; |
grt (UTC plus 2)
2017/04/25 08:29:39
what if visited URLs arrive immediately after gett
chengx
2017/04/25 23:00:24
You're right. The "TopSites Service" and the "TabR
|
+ |
// Send a query that retrieves the first favicon. |
StartLoadingFavicon(); |
} |
@@ -436,7 +479,7 @@ void JumpList::TabRestoreServiceChanged(sessions::TabRestoreService* service) { |
// An empty string. This value is to be updated in OnFaviconDataAvailable(). |
// This code is copied from |
// RecentlyClosedTabsHandler::TabRestoreServiceChanged() to emulate it. |
- const int kRecentlyClosedCount = 4; |
+ const int kRecentlyClosedCount = 3; |
sessions::TabRestoreService* tab_restore_service = |
TabRestoreServiceFactory::GetForProfile(profile_); |
for (const auto& entry : tab_restore_service->entries()) { |
@@ -459,6 +502,11 @@ void JumpList::TabRestoreServiceChanged(sessions::TabRestoreService* service) { |
data->recently_closed_pages_ = temp_list; |
grt (UTC plus 2)
2017/04/25 08:29:39
swap would be more efficient here so that no copie
chengx
2017/04/25 23:00:24
This piece of code is gone after making JumpListDa
|
} |
+ // Since only the "Recently Closed" category changes, mark the flags so that |
+ // icon files only for this category are updated later on. |
+ should_update_most_visited_ = false; |
+ should_update_recent_closed_ = true; |
+ |
// Send a query that retrieves the first favicon. |
StartLoadingFavicon(); |
} |
@@ -578,10 +626,15 @@ void JumpList::OnIncognitoAvailabilityChanged() { |
base::AutoLock auto_lock(data->list_lock_); |
waiting_for_icons = !data->icon_urls_.empty(); |
} |
- if (!waiting_for_icons) |
+ |
+ // Since neither the "Most Visited" category nor the "Recently Closed" |
+ // category changes, mark the flags so that icon files for those categories |
+ // won't be updated later on. |
+ if (!waiting_for_icons) { |
+ should_update_most_visited_ = false; |
+ should_update_recent_closed_ = false; |
PostRunUpdate(); |
- // If |icon_urls_| isn't empty then OnFaviconDataAvailable will eventually |
- // call PostRunUpdate(). |
+ } |
} |
void JumpList::PostRunUpdate() { |
@@ -611,17 +664,26 @@ void JumpList::DeferredRunUpdate() { |
profile_ ? IncognitoModePrefs::GetAvailability(profile_->GetPrefs()) |
: IncognitoModePrefs::ENABLED; |
- // Post a task to update the jumplist in JumpListIcons folder, which consists |
- // of 1) delete old icons, 2) create new icons, 3) notify the OS. |
- update_jumplisticons_task_runner_->PostTask( |
- FROM_HERE, base::Bind(&RunUpdateJumpList, incognito_availability, app_id_, |
- icon_dir_, base::RetainedRef(jumplist_data_))); |
- |
- // Post a task to delete JumpListIconsOld folder and log the results to UMA. |
+ // Post a task to update the JumpList, which consists of 1) delete old icons, |
+ // 2) create new icons, 3) notify the OS. |
+ update_jumplist_task_runner_->PostTask( |
+ FROM_HERE, |
+ base::Bind(&RunUpdateJumpList, incognito_availability, app_id_, icon_dir_, |
+ should_update_most_visited_, should_update_recent_closed_, |
+ base::RetainedRef(jumplist_data_))); |
+ |
+ // Post a task to delete JumpListIcons folder as it't no longer needed. |
+ // Now we have JumpListIconsMostVisited folder and JumpListIconsRecentClosed |
+ // folder instead. |
+ delete_jumplisticons_task_runner_->PostTask( |
+ FROM_HERE, |
+ base::Bind(&DeleteDirectoryAndLogResults, icon_dir_, kFileDeleteLimit)); |
+ |
+ // Post a task to delete JumpListIconsOld folder as it't no longer needed. |
base::FilePath icon_dir_old = icon_dir_.DirName().Append( |
icon_dir_.BaseName().value() + FILE_PATH_LITERAL("Old")); |
- delete_jumplisticonsold_task_runner_->PostTask( |
+ delete_jumplisticons_task_runner_->PostTask( |
FROM_HERE, base::Bind(&DeleteDirectoryAndLogResults, |
std::move(icon_dir_old), kFileDeleteLimit)); |
} |