Improve JumpList icon folders' naming style

chrome/browser/win/jumplist.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/win/jumplist.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 179 matching lines @@
     base::ReplaceSubstringsAfterOffset(
         &incognito_title, 0, L"&", base::StringPiece16());
     incognito->set_title(incognito_title);
     incognito->set_icon(chrome_path.value(), icon_resources::kIncognitoIndex);
     items.push_back(incognito);
   }
 
   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,
                     bool recently_closed_pages_have_updates,
                     IncognitoModePrefs::Availability incognito_availability) {
   if (!JumpListUpdater::IsEnabled())
     return true;
 
   JumpListUpdater jumplist_updater(app_id);
 
@@ skipping 37 matching lines @@
   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();
   }
 
   // Record the desired number of icons to create in this JumpList update.
   int icons_to_create = 0;
 
   // 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);
 
     icons_to_create += std::min(most_visited_pages.size(), most_visited_items);
   }
 
   // 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);
 
     icons_to_create +=
         std::min(recently_closed_pages.size(), recently_closed_items);
   }
 
   // TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407.
   UMA_HISTOGRAM_COUNTS_100("WinJumplist.CreateIconFilesCount", icons_to_create);
@@ skipping 21 matching lines @@
   if (!UpdateTaskCategory(&jumplist_updater, incognito_availability))
     return false;
 
   // Commit this transaction and send the updated JumpList to Windows.
   return jumplist_updater.CommitUpdate();
 }
 
 // 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;
   ShellLinkItemList local_recently_closed_pages;
   bool most_visited_pages_have_updates;
   bool recently_closed_pages_have_updates;
 
   {
     base::AutoLock auto_lock(data->list_lock_);
     // Make sure we are not out of date: if icon_urls_ is not empty, then
@@ skipping 14 matching lines @@
     data->recently_closed_pages_have_updates_ = false;
   }
 
   if (!most_visited_pages_have_updates && !recently_closed_pages_have_updates)
     return;
 
   // Update the application JumpList. If it fails, reset the flags to true if
   // 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_);
     if (most_visited_pages_have_updates)
       data->most_visited_pages_have_updates_ = true;
     if (recently_closed_pages_have_updates)
       data->recently_closed_pages_have_updates_ = true;
   }
 }
 
@@ skipping 29 matching lines @@
   // When we add this object to the observer list, we save the pointer to this
   // TabRestoreService object. This pointer is used when we remove this object
   // from the observer list.
   sessions::TabRestoreService* tab_restore_service =
       TabRestoreServiceFactory::GetForProfile(profile_);
   if (!tab_restore_service)
     return;
 
   app_id_ =
       shell_integration::win::GetChromiumModelIdForProfile(profile_->GetPath());
-  icon_dir_ = profile_->GetPath().Append(chrome::kJumpListIconDirname);
 
   scoped_refptr<history::TopSites> top_sites =
       TopSitesFactory::GetForProfile(profile_);
   if (top_sites) {
     // TopSites updates itself after a delay. This is especially noticable when
     // your profile is empty. Ask TopSites to update itself when jumplist is
     // initialized.
     top_sites->SyncWithHistory();
     // Register as TopSitesObserver so that we can update ourselves when the
     // TopSites changes.
@@ skipping 254 matching lines @@
   } else {
     timer_.Start(FROM_HERE, kDelayForJumplistUpdate, this,
                  &JumpList::DeferredRunUpdate);
   }
 }
 
 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.

                : IncognitoModePrefs::ENABLED;
 
   // 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_, 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));
 }
 
 void JumpList::TopSitesLoaded(history::TopSites* top_sites) {
 }
 
 void JumpList::TopSitesChanged(history::TopSites* top_sites,
                                ChangeReason change_reason) {
   top_sites->GetMostVisitedURLs(
       base::Bind(&JumpList::OnMostVisitedURLsAvailable,
                  weak_ptr_factory_.GetWeakPtr()),
       false);
 }