Delete JumpListIconsOld and update revised links in every update

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 <Shlwapi.h>
-
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
 #include "base/sequenced_task_runner.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 234 matching lines @@
   }
 
   // Delete the contents in JumpListIcons directory and log the delete status
   // to UMA.
   FolderDeleteResult delete_status =
       DeleteDirectoryContent(icon_dir, kFileDeleteLimit);
 
   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 (base::DirectoryExists(icon_dir)) {
-    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;
+  // If JumpListIcons directory is not empty, skip updating the jumplist icons.
+  // 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 icons. The jumplist links should be updated anyway, as it doesn't
+  // involve disk IO.
+
+  DirectoryStatus dir_status = NON_EXIST;
+  if (base::DirectoryExists(icon_dir))
+    dir_status = base::IsDirectoryEmpty(icon_dir) ? EMPTY : NON_EMPTY;
+
+  if (dir_status == NON_EXIST && base::CreateDirectory(icon_dir))
+    dir_status = EMPTY;
+

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

UMA WinJumplist.DirectoryStatusJumpListIcons here?

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

Ah, my bad. Now it is back.

+  if (dir_status == EMPTY) {
+    // Create temporary icon files for shortcuts in the "Most Visited" category.

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

the files aren't really temporary, are they? their names may be random, but they
need to persist as long as the shell references them.

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

Those are old comments existed for a very long time. But you are right, they
need to persist as long as they are referenced by the shell. I have remove
“temporary”.

+    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 "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);
-
-  // We finished collecting all resources needed for updating an application
-  // JumpList. So, create a new JumpList and replace the current JumpList
-  // with it.
+  // Create a new JumpList and replace the current JumpList with it. The
+  // jumplist links are updated anyway, while the jumplist icons may not as
+  // mentioned above.
   UpdateJumpList(app_id.c_str(), local_most_visited_pages,
                  local_recently_closed_pages, incognito_availability);
 
   // Post a background task to delete JumpListIconsOld folder if it exists and
   // log the delete results to UMA.
   base::FilePath icon_dir_old = icon_dir.DirName().Append(
       icon_dir.BaseName().value() + FILE_PATH_LITERAL("Old"));
 
   if (base::DirectoryExists(icon_dir_old)) {
     sequenced_task_runner->PostTask(
@@ skipping 326 matching lines @@
 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);
 }