Delete JumpListIconsOld and update revised links in every update

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 af8b85740eece98290c83dfe4b35dced83cecdd8..c1585a697bf93408cbbe5a51dcba16eb824a1b92 100644
--- a/chrome/browser/win/jumplist.cc
+++ b/chrome/browser/win/jumplist.cc
@@ -4,8 +4,6 @@
 
 #include "chrome/browser/win/jumplist.h"
 
-#include <Shlwapi.h>
-
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ -260,32 +258,34 @@ void RunUpdateOnFileThread(
   UMA_HISTOGRAM_ENUMERATION("WinJumplist.DeleteStatusJumpListIcons",
                             delete_status, END);
 
-  // If JumpListIcons directory is not empty, skip jumplist update and return
-  // early. If the directory doesn't exist which shouldn't though, try to create
-  // a new JumpListIcons directory. If the creation fails, return early.
+  // If JumpListIcons directory is not empty, skip updating the jumplist.
+  // If the directory doesn't exist which shouldn't though, try to create
+  // a new JumpListIcons directory. If the creation fails, skip updating the
+  // jumplist.
+
+  DirectoryStatus dir_status = NON_EXIST;
   if (base::DirectoryExists(icon_dir)) {

[grt (UTC plus 2), 2017/03/27 08:03:39]

nit: omit braces for single-line conditional and body

[chengx, 2017/03/27 18:32:41]

Done.

-    DirectoryEmptyStatus empty_status =
-        ::PathIsDirectoryEmpty(icon_dir.value().c_str()) ? EMPTY : NON_EMPTY;
-    UMA_HISTOGRAM_ENUMERATION("WinJumplist.EmptyStatusJumpListIcons",
-                              empty_status, EMPTY_STATUS_END);
-    if (empty_status == NON_EMPTY)
-      return;
-  } else if (!base::CreateDirectory(icon_dir)) {
-    return;
+    dir_status = base::IsDirectoryEmpty(icon_dir) ? EMPTY : NON_EMPTY;
   }
 
-  // Create temporary icon files for shortcuts in the "Most Visited" category.
-  CreateIconFiles(icon_dir, local_most_visited_pages);
+  if (dir_status == NON_EXIST && base::CreateDirectory(icon_dir)) {

[grt (UTC plus 2), 2017/03/27 08:03:39]

nit: omit braces

[chengx, 2017/03/27 18:32:41]

Done.

+    dir_status = EMPTY;
+  }
+
+  if (dir_status == EMPTY) {
+    // Create temporary icon files for shortcuts in the "Most Visited" category.
+    CreateIconFiles(icon_dir, local_most_visited_pages);
 
-  // Create temporary icon files for shortcuts in the "Recently Closed"
-  // category.
-  CreateIconFiles(icon_dir, local_recently_closed_pages);
+    // Create temporary icon files for shortcuts in the "Recently Closed"
+    // category.
+    CreateIconFiles(icon_dir, local_recently_closed_pages);
 
-  // We finished collecting all resources needed for updating an application
-  // JumpList. So, create a new JumpList and replace the current JumpList
-  // with it.
-  UpdateJumpList(app_id.c_str(), local_most_visited_pages,
-                 local_recently_closed_pages, incognito_availability);
+    // We finished collecting all resources needed for updating an application
+    // JumpList. So, create a new JumpList and replace the current JumpList
+    // with it.
+    UpdateJumpList(app_id.c_str(), local_most_visited_pages,
+                   local_recently_closed_pages, incognito_availability);
+  }

[grt (UTC plus 2), 2017/03/27 08:03:39]

in the "else" case, the jumplist retains the old most visited and recently
closed links, but the icons may be missing, correct? is it better to update it
with the revised links but no icons?

[chengx, 2017/03/27 18:32:41]

Correct, in this case, the jumplist retains the old most visited and recently
closed links, but the icons may be missing.

Agreed that we should update the revised links even we are not creating new
icons for them.

[grt (UTC plus 2), 2017/03/27 19:18:47]

how does the shell handle this -- is Chrome's icon used, or is there a blank
space where the icon should be, or something else?

[chengx, 2017/03/27 20:26:08]

In this case, Chrome's icon will be used. 

The case where a blank space is used is when the icons in JumpListIcons are
deleted after the update. This time, when right clicking the Chrome icon in the
task bar, a blanks space will be used as the icons are missing now. 

 
   // Post a background task to delete JumpListIconsOld folder if it exists and
   // log the delete results to UMA.