Improve JumpList icon folders' naming style

chrome/browser/win/jumplist.cc

Index: chrome/browser/win/jumplist.cc
diff --git a/chrome/browser/win/jumplist.cc b/chrome/browser/win/jumplist.cc
index a9e27e77aa8de0baf1e25e1aa45957d14cd94f49..77e3a1db5d4040f2887f90cc248f26cfeb4cbeea 100644
--- a/chrome/browser/win/jumplist.cc
+++ b/chrome/browser/win/jumplist.cc
@@ -197,13 +197,21 @@ bool UpdateTaskCategory(
   return jumplist_updater->AddTasks(items);
 }
 
+base::FilePath GenerateJumplistIconDirName(

[grt (UTC plus 2), 2017/05/04 11:41:55]

// Returns the full path of the JumpListIcons[|suffix|] directory in
|profile_dir|.

[chengx, 2017/05/04 19:36:11]

Done.

+    const base::FilePath& profile_dir,
+    const base::FilePath::StringPieceType& suffix) {
+  base::FilePath::StringType dir_name(chrome::kJumpListIconDirname);
+  suffix.AppendToString(&dir_name);
+  return profile_dir.Append(dir_name);
+}
+
 // Updates the application JumpList, which consists of 1) delete old icon files;
 // 2) create new icon files; 3) notify the OS.
 // Note that any timeout error along the way results in the old jumplist being
 // left as-is, while any non-timeout error results in the old jumplist being
 // left as-is, but without icon files.
 bool UpdateJumpList(const wchar_t* app_id,
-                    const base::FilePath& icon_dir,
+                    const base::FilePath& profile_dir,
                     const ShellLinkItemList& most_visited_pages,
                     const ShellLinkItemList& recently_closed_pages,
                     bool most_visited_pages_have_updates,
@@ -261,8 +269,8 @@ bool UpdateJumpList(const wchar_t* app_id,
 
   // Update the icons for "Most Visisted" category of the JumpList if needed.
   if (most_visited_pages_have_updates) {
-    base::FilePath icon_dir_most_visited = icon_dir.DirName().Append(
-        icon_dir.BaseName().value() + FILE_PATH_LITERAL("MostVisited"));
+    base::FilePath icon_dir_most_visited = GenerateJumplistIconDirName(
+        profile_dir, FILE_PATH_LITERAL("MostVisited"));
 
     UpdateIconFiles(icon_dir_most_visited, most_visited_pages,
                     most_visited_items);
@@ -272,8 +280,8 @@ bool UpdateJumpList(const wchar_t* app_id,
 
   // Update the icons for "Recently Closed" category of the JumpList if needed.
   if (recently_closed_pages_have_updates) {
-    base::FilePath icon_dir_recent_closed = icon_dir.DirName().Append(
-        icon_dir.BaseName().value() + FILE_PATH_LITERAL("RecentClosed"));
+    base::FilePath icon_dir_recent_closed = GenerateJumplistIconDirName(
+        profile_dir, FILE_PATH_LITERAL("RecentClosed"));
 
     UpdateIconFiles(icon_dir_recent_closed, recently_closed_pages,
                     recently_closed_items);
@@ -315,7 +323,7 @@ bool UpdateJumpList(const wchar_t* app_id,
 // Updates the jumplist, once all the data has been fetched.
 void RunUpdateJumpList(IncognitoModePrefs::Availability incognito_availability,
                        const std::wstring& app_id,
-                       const base::FilePath& icon_dir,
+                       const base::FilePath& profile_dir,
                        base::RefCountedData<JumpListData>* ref_counted_data) {
   JumpListData* data = &ref_counted_data->data;
   ShellLinkItemList local_most_visited_pages;
@@ -350,7 +358,7 @@ void RunUpdateJumpList(IncognitoModePrefs::Availability incognito_availability,
   // they were so that the corresponding JumpList categories will be tried to
   // update again in the next run.
   if (!UpdateJumpList(
-          app_id.c_str(), icon_dir, local_most_visited_pages,
+          app_id.c_str(), profile_dir, local_most_visited_pages,
           local_recently_closed_pages, most_visited_pages_have_updates,
           recently_closed_pages_have_updates, incognito_availability)) {
     base::AutoLock auto_lock(data->list_lock_);
@@ -400,7 +408,6 @@ JumpList::JumpList(Profile* profile)
 
   app_id_ =
       shell_integration::win::GetChromiumModelIdForProfile(profile_->GetPath());
-  icon_dir_ = profile_->GetPath().Append(chrome::kJumpListIconDirname);
 
   scoped_refptr<history::TopSites> top_sites =
       TopSitesFactory::GetForProfile(profile_);
@@ -675,6 +682,11 @@ void JumpList::DeferredRunUpdate() {
   DCHECK(CalledOnValidThread());
 
   TRACE_EVENT0("browser", "JumpList::DeferredRunUpdate");
+  if (!profile_)
+    return;
+
+  base::FilePath profile_dir = profile_->GetPath();
+
   // Check if incognito windows (or normal windows) are disabled by policy.
   IncognitoModePrefs::Availability incognito_availability =
       profile_ ? IncognitoModePrefs::GetAvailability(profile_->GetPrefs())

[grt (UTC plus 2), 2017/05/04 11:41:55]

this null check is no longer needed due to the early return above.

[chengx, 2017/05/04 19:36:11]

Done.

@@ -684,18 +696,19 @@ void JumpList::DeferredRunUpdate() {
   // 2) create new icons, 3) notify the OS.
   update_jumplist_task_runner_->PostTask(
       FROM_HERE, base::Bind(&RunUpdateJumpList, incognito_availability, app_id_,
-                            icon_dir_, base::RetainedRef(jumplist_data_)));
+                            profile_dir, base::RetainedRef(jumplist_data_)));
 
   // Post a task to delete JumpListIcons folder as it's no longer needed.
   // Now we have JumpListIconsMostVisited folder and JumpListIconsRecentClosed
   // folder instead.
   delete_jumplisticons_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&DeleteDirectory, icon_dir_, kFileDeleteLimit));
+      FROM_HERE, base::Bind(&DeleteDirectory,
+                            profile_dir.Append(chrome::kJumpListIconDirname),

[grt (UTC plus 2), 2017/05/04 11:41:55]

nit: i'd be inclined to use GenerateJumplistIconDirName here for the sake of
uniformity. it's up to you.

[chengx, 2017/05/04 19:36:11]

I've changed to use GenerateJumplistIconDirName.

+                            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"));
-
+  base::FilePath icon_dir_old =
+      GenerateJumplistIconDirName(profile_dir, FILE_PATH_LITERAL("Old"));
   delete_jumplisticons_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&DeleteDirectory, std::move(icon_dir_old), kFileDeleteLimit));