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

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

Issue 2836363003: Retire some metrics and update file util methods for JumpList (Closed)
Patch Set: Refactor code Created 3 years, 8 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
« no previous file with comments | « no previous file | chrome/browser/win/jumplist_file_util.h » ('j') | chrome/browser/win/jumplist_file_util.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/win/jumplist.cc
diff --git a/chrome/browser/win/jumplist.cc b/chrome/browser/win/jumplist.cc
index 6dad5579697389d68a137d050de08c7ae6b97199..26f88b2a3bf2a64b5483c62e43e29774736b51d0 100644
--- a/chrome/browser/win/jumplist.cc
+++ b/chrome/browser/win/jumplist.cc
@@ -123,8 +123,8 @@ bool CreateIconFile(const gfx::ImageSkia& image_skia,
return true;
}
-// Helper method for RunUpdate to create icon files for the asynchrounously
-// loaded icons.
+// Helper method for RunUpdateJumpList to create icon files for the
grt (UTC plus 2) 2017/04/28 07:37:07 please follow the function comment style guideline
chengx 2017/04/28 22:29:29 Done. I've updated the comments throughout the fil
+// asynchrounously loaded icons.
void CreateIconFiles(const base::FilePath& icon_dir,
const ShellLinkItemList& item_list,
size_t max_items) {
@@ -139,6 +139,25 @@ void CreateIconFiles(const base::FilePath& icon_dir,
}
}
+// Helper method for RunUpdateJumpList to update icon files in |icon_dir|, which
+// includes deleting old icons and creating at most |slot_limit| new icons for
+// |page_list|.
+void UpdateIconFiles(const base::FilePath& icon_dir,
+ const ShellLinkItemList& page_list,
+ size_t slot_limit) {
+ DeleteDirectoryContentAndLogRuntime(icon_dir, kFileDeleteLimit);
+
+ if (!base::DirectoryExists(icon_dir))
grt (UTC plus 2) 2017/04/28 07:37:07 base::CreateDirectory returns true if the director
chengx 2017/04/28 22:29:29 Done.
+ base::CreateDirectory(icon_dir);
+
+ // If the directory still doesn't exist or is non-empty, skip updating the
+ // jumplist icons for this jumplist category. The jumplist links should be
+ // updated later on anyway, as it doesn't involve disk IO. In this case,
grt (UTC plus 2) 2017/04/28 07:37:07 regarding this comment, are you saying that callin
chengx 2017/04/28 22:29:29 No, I didn't mean that. Calling the shell function
+ // Chrome's icon will be used for the new links.
+ if (base::DirectoryExists(icon_dir) && base::IsDirectoryEmpty(icon_dir))
+ CreateIconFiles(icon_dir, page_list, slot_limit);
+}
+
// Updates the "Tasks" category of the JumpList.
bool UpdateTaskCategory(
JumpListUpdater* jumplist_updater,
@@ -241,42 +260,23 @@ bool UpdateJumpList(const wchar_t* app_id,
}
if (most_visited_pages_have_updates) {
- // Delete the content in JumpListIconsMostVisited folder and log the results
+ // Delete the content in JumpListIconsMostVisited folder and log the runtime
// 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);
- }
+ UpdateIconFiles(icon_dir_most_visited, most_visited_pages,
+ most_visited_items);
}
if (recently_closed_pages_have_updates) {
// Delete the content in JumpListIconsRecentClosed folder and log the
- // results to UMA.
+ // runtime to UMA.
base::FilePath icon_dir_recent_closed = icon_dir.DirName().Append(
icon_dir.BaseName().value() + FILE_PATH_LITERAL("RecentClosed"));
- DeleteDirectoryContentAndLogResults(icon_dir_recent_closed,
- kFileDeleteLimit);
-
- 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);
- }
+ UpdateIconFiles(icon_dir_recent_closed, recently_closed_pages,
+ recently_closed_items);
}
// TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407.
@@ -686,16 +686,15 @@ void JumpList::DeferredRunUpdate() {
// Now we have JumpListIconsMostVisited folder and JumpListIconsRecentClosed
// folder instead.
delete_jumplisticons_task_runner_->PostTask(
- FROM_HERE,
- base::Bind(&DeleteDirectoryAndLogResults, icon_dir_, kFileDeleteLimit));
+ FROM_HERE, base::Bind(&DeleteDirectory, icon_dir_, kFileDeleteLimit));
// Post a task to delete JumpListIconsOld folder as it's no longer needed.
base::FilePath icon_dir_old = icon_dir_.DirName().Append(
icon_dir_.BaseName().value() + FILE_PATH_LITERAL("Old"));
delete_jumplisticons_task_runner_->PostTask(
- FROM_HERE, base::Bind(&DeleteDirectoryAndLogResults,
- std::move(icon_dir_old), kFileDeleteLimit));
+ FROM_HERE,
+ base::Bind(&DeleteDirectory, std::move(icon_dir_old), kFileDeleteLimit));
}
void JumpList::TopSitesLoaded(history::TopSites* top_sites) {
« no previous file with comments | « no previous file | chrome/browser/win/jumplist_file_util.h » ('j') | chrome/browser/win/jumplist_file_util.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698