Various logical fixes for jumplist

This CL consists of the following three changes:

1) Move DeleteDirectoryContentAndLogResults() so that it runs only if
   jumplist_updater.BeginUpdate() succeeds, otherwise it's pointless.

2) Return early if any AddShellLink call fails. This is suspected to
   cause long execution time for jumplist_updater.

3) Decrease kFileDeleteLimit from 60 to 30. A user without jumplist
   issues should have at most 10 icon files in JumpListIcons folder as
   in crrev.com/2816113002. So changing kFileDeleteLimit to 30 won't
   affect "healthy" users. It'll give a better experience for users
   who have corrupted jumplist folders as the disk IO per jumplist
   update becomes less.

BUG=40407, 179576

Review-Url: https://codereview.chromium.org/2824103003
Cr-Commit-Position: refs/heads/master@{#465689}

chrome/browser/win/jumplist.cc

@@ -196,6 +196,8 @@ bool UpdateJumpList(const wchar_t* app_id,
   // and 40% to "recently-closed" items, respectively.
   // Nevertheless, if there are not so many items in |recently_closed_pages|,
   // we give the remaining slots to "most-visited" items.
+  // The default maximum number of items to display in JumpList is 10.
+  // https://msdn.microsoft.com/en-us/library/windows/desktop/dd378398(v=vs.85).aspx
   const int kMostVisited = 60;
   const int kRecentlyClosed = 40;
   const int kTotal = kMostVisited + kRecentlyClosed;
@@ -208,6 +210,9 @@ bool UpdateJumpList(const wchar_t* app_id,
     recently_closed_items = recently_closed_pages.size();
   }
 
+  // Delete the content in JumpListIcons folder and log the results to UMA.
+  DeleteDirectoryContentAndLogResults(icon_dir, kFileDeleteLimit);
+
   // If JumpListIcons directory doesn't exist (we have tried to create it
   // already) or is not empty, skip updating the jumplist icons. The jumplist
   // links should be updated anyway, as it doesn't involve disk IO. In this
@@ -277,9 +282,6 @@ void RunUpdateJumpList(IncognitoModePrefs::Availability incognito_availability,
     local_recently_closed_pages = data->recently_closed_pages_;
   }
 
-  // Delete the content in JumpListIcons folder and log the results to UMA.
-  DeleteDirectoryContentAndLogResults(icon_dir, kFileDeleteLimit);
-
   // 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.

chrome/browser/win/jumplist_file_util.h

@@ -8,7 +8,7 @@
 #include "base/files/file_path.h"
 
 // Maximum number of icon files allowed to delete per jumplist update.
-const int kFileDeleteLimit = 60;
+const int kFileDeleteLimit = 30;
 
 // Folder delete status enumeration, used in Delete* methods below.
 // This is used for UMA. Do not delete entries, and keep in sync with

chrome/browser/win/jumplist_updater.cc

@@ -190,7 +190,8 @@ bool JumpListUpdater::AddTasks(const ShellLinkItemList& link_items) {
   // Add items to the "Task" category.
   for (ShellLinkItemList::const_iterator it = link_items.begin();
        it != link_items.end(); ++it) {
-    AddShellLink(collection, application_path.value(), *it);
+    if (!AddShellLink(collection, application_path.value(), *it))
+      return false;
   }
 
   // We can now add the new list to the JumpList.
@@ -236,8 +237,8 @@ bool JumpListUpdater::AddCustomCategory(const std::wstring& category_name,
 
   for (ShellLinkItemList::const_iterator item = link_items.begin();
        item != link_items.end() && max_items > 0; ++item, --max_items) {
-    scoped_refptr<ShellLinkItem> link(*item);
-    AddShellLink(collection, application_path.value(), link);
+    if (!AddShellLink(collection, application_path.value(), *item))
+      return false;
   }
 
   // We can now add the new list to the JumpList.