Delete the right JumpList icons conditional on shell notification

chrome/browser/win/jumplist.h

Index: chrome/browser/win/jumplist.h
diff --git a/chrome/browser/win/jumplist.h b/chrome/browser/win/jumplist.h
index da68edb72389eae8bb66f5815718e52dede17143..9d985c2b5bff41dec85186fe34197a1bd1e4be7a 100644
--- a/chrome/browser/win/jumplist.h
+++ b/chrome/browser/win/jumplist.h
@@ -68,8 +68,11 @@ class JumpList : public sessions::TabRestoreServiceObserver,
   void Shutdown() override;
 
  private:
+  enum class JumpListVersion { kCurrent, kNext, kShared };
+
   using UrlAndLinkItem = std::pair<std::string, scoped_refptr<ShellLinkItem>>;
   using URLIconCache = base::flat_map<std::string, base::FilePath>;
+  using IconAssociation = base::flat_map<base::FilePath, JumpListVersion>;
 
   // Holds results of the RunUpdateJumpList run.
   struct UpdateResults {
@@ -80,13 +83,32 @@ class JumpList : public sessions::TabRestoreServiceObserver,
     // Holding a copy of most_visited_icons_ initially, it's updated by the
     // JumpList update run. If the update run succeeds, it overwrites
     // most_visited_icons_.
-    URLIconCache most_visited_icons_in_update;
+    URLIconCache most_visited_icons;
 
-    // icon file paths of the recently closed links, indexed by tab url.
-    // Holding a copy of recently_closed_icons_ initially, it's updated by the
+    // Icon file paths of the recently closed links, indexed by tab url.
+    // Holding a copy of recent_closed_icons_ initially, it's updated by the
     // JumpList update run. If the update run succeeds, it overwrites
-    // recently_closed_icons_.
-    URLIconCache recently_closed_icons_in_update;
+    // recent_closed_icons_.
+    URLIconCache recent_closed_icons;

[grt (UTC plus 2), 2017/06/20 10:29:18]

i prefer "recently" over "recent" since these are for the tabs that were
recently closed. could you revert the variable renamings? it makes the CL harder
to review.

[chengx, 2017/06/20 19:33:59]

Done.

+
+    // The association of icon files in most visited category with the JumpList
+    // versions, defined in enum JumpListVersion. Holding a copy of
+    // most_visited_icon_assoc_ initially, it's updated by the JumpList update
+    // run. If the update run succeeds, it overwrites most_visited_icon_assoc_.
+    IconAssociation most_visited_icon_assoc;
+
+    // The association of icon files in recently closed category with the
+    // JumpList versions, defined in enum JumpListVersion. Holding a copy of
+    // recent_closed_icon_assoc_ initially, it's updated by the JumpList
+    // update run. If the update run succeeds, it overwrites
+    // recent_closed_icon_assoc_.
+    IconAssociation recent_closed_icon_assoc;
+
+    // A flag indicating if "Most Visited" category should be updated.
+    bool most_visited_should_update = false;
+
+    // A flag indicating if "Recently Closed" category should be updated.
+    bool recent_closed_should_update = false;
 
     // A flag indicating if a JumpList update run is successful.
     bool update_success = false;
@@ -175,7 +197,7 @@ class JumpList : public sessions::TabRestoreServiceObserver,
   void Terminate();
 
   // Updates the application JumpList, which consists of 1) create new icon
-  // files; 2) delete obsolete icon files; 3) notify the OS.
+  // files; 2) notify the OS; 2) delete obsolete icon files.
   // 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.
@@ -183,33 +205,53 @@ class JumpList : public sessions::TabRestoreServiceObserver,
       const base::string16& app_id,
       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,
+      const ShellLinkItemList& recent_closed_pages,
       IncognitoModePrefs::Availability incognito_availability,
       UpdateResults* update_results);
 
   // Updates icon files for |page_list| in |icon_dir|, which consists of
-  // 1) creating at most |slot_limit| new icons which are not in |icon_cache|;
-  // 2) deleting old icons which are not in |icon_cache|.
-  // Returns the number of new icon files created.
-  static int UpdateIconFiles(const base::FilePath& icon_dir,
-                             const ShellLinkItemList& page_list,
-                             size_t slot_limit,
-                             URLIconCache* icon_cache);
+  // 1) If certain safe conditions are not met, clean the folder at |icon_dir|.
+  // If folder cleaning fails, skip step 2. Besides, clear |icon_assoc|,
+  // |icon_cur| and |icon_next|.
+  // 2) Create at most |max_items| icon files which are not in |icon_cur| for
+  // the asynchrounously loaded icons stored in |item_list|. |icon_assoc| and
+  // |icon_next| are updated based on the reusable icons in |icon_cur| and the
+  // newly created icons. Returns the number of new icon files created.
+  static int SafeCreateIconFiles(const base::FilePath& icon_dir,
+                                 const ShellLinkItemList& page_list,
+                                 size_t slot_limit,
+                                 IconAssociation* icon_assoc,
+                                 URLIconCache* icon_cur,
+                                 URLIconCache* icon_next);
 
   // In |icon_dir|, creates at most |max_items| icon files which are not in
-  // |icon_cache| for the asynchrounously loaded icons stored in |item_list|.
-  // |icon_cache| is also updated for newly created icons.
-  // Returns the number of new icon files created.
+  // |icon_cur| for the asynchrounously loaded icons stored in |item_list|.
+  // |icon_assoc| and |icon_next| are updated based on the reusable icons in
+  // |icon_cur| and the newly created icons. Returns the number of new icon
+  // files created.
   static int CreateIconFiles(const base::FilePath& icon_dir,
                              const ShellLinkItemList& item_list,
                              size_t max_items,
-                             URLIconCache* icon_cache);
-
-  // Deletes icon files in |icon_dir| which are not in |icon_cache| anymore.
+                             IconAssociation* icon_assoc,
+                             URLIconCache* icon_cur,
+                             URLIconCache* icon_next);
+
+  // Deletes icon files which are associated with JumpList |version|, based on
+  // the JumpList version and icon association information stored in
+  // |update_results|. Icons in both |most_visited_icon_dir| and
+  // |recent_closed_should_update| are attempted to process.
+  static void DeleteIconFilesUnified(
+      const base::FilePath& most_visited_icon_dir,
+      const base::FilePath& recent_closed_icon_dir,
+      UpdateResults* update_results,
+      JumpListVersion version);
+
+  // Deletes icon files in |icon_dir| which are associated with JumpList
+  // |version|, based on the |icon_assoc| which records the association of
+  // icons and JumpList versions.
   static void DeleteIconFiles(const base::FilePath& icon_dir,
-                              URLIconCache* icon_cache);
+                              IconAssociation* icon_assoc,
+                              JumpListVersion version);
 
   // Tracks FaviconService tasks.
   base::CancelableTaskTracker cancelable_task_tracker_;
@@ -233,13 +275,21 @@ class JumpList : public sessions::TabRestoreServiceObserver,
   ShellLinkItemList most_visited_pages_;
 
   // Items in the "Recently Closed" category of the JumpList.
-  ShellLinkItemList recently_closed_pages_;
+  ShellLinkItemList recent_closed_pages_;
 
   // The icon file paths of the most visited links, indexed by tab url.
   URLIconCache most_visited_icons_;
 
   // The icon file paths of the recently closed links, indexed by tab url.
-  URLIconCache recently_closed_icons_;
+  URLIconCache recent_closed_icons_;
+
+  // The association of icon files in most visited category with the JumpList
+  // versions, defined in enum JumpListVersion.
+  IconAssociation most_visited_icon_assoc_;
+
+  // The association of icon files in recently closed category with the JumpList
+  // versions, defined in enum JumpListVersion.
+  IconAssociation recent_closed_icon_assoc_;
 
   // A flag indicating if TopSites service has notifications.
   bool top_sites_has_pending_notification_ = false;
@@ -251,7 +301,7 @@ class JumpList : public sessions::TabRestoreServiceObserver,
   bool most_visited_should_update_ = false;
 
   // A flag indicating if "Recently Closed" category should be updated.
-  bool recently_closed_should_update_ = false;
+  bool recent_closed_should_update_ = false;
 
   // A flag indicating if there's a JumpList update task already posted or
   // currently running.