Fix to not create jumplist icon files that aren't used by shell

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 102 matching lines @@
 
   // Add this icon file to the list and return its absolute path.
   // The IShellLink::SetIcon() function needs the absolute path to an icon.
   *icon_path = path;
   return true;
 }
 
 // Helper method for RunUpdate to create icon files for the asynchrounously
 // loaded icons.
 void CreateIconFiles(const base::FilePath& icon_dir,
-                     const ShellLinkItemList& item_list) {
+                     const ShellLinkItemList& item_list,
+                     size_t max_items) {
   // TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407.
   SCOPED_UMA_HISTOGRAM_TIMER("WinJumplist.CreateIconFilesDuration");
 
   for (ShellLinkItemList::const_iterator item = item_list.begin();
-      item != item_list.end(); ++item) {
+       item != item_list.end() && max_items > 0; ++item, --max_items) {
     base::FilePath icon_path;
     if (CreateIconFile((*item)->icon_image(), icon_dir, &icon_path))
       (*item)->set_icon(icon_path.value(), 0);
   }
 }
 
 // Updates the "Tasks" category of the JumpList.
 bool UpdateTaskCategory(
     JumpListUpdater* jumplist_updater,
     IncognitoModePrefs::Availability incognito_availability) {
@@ skipping 31 matching lines @@
     incognito->set_title(incognito_title);
     incognito->set_icon(chrome_path.value(), icon_resources::kIncognitoIndex);
     items.push_back(incognito);
   }
 
   return jumplist_updater->AddTasks(items);
 }
 
 // Updates the application JumpList.
 bool UpdateJumpList(const wchar_t* app_id,
+                    const base::FilePath& icon_dir,
                     const ShellLinkItemList& most_visited_pages,
                     const ShellLinkItemList& recently_closed_pages,
                     IncognitoModePrefs::Availability incognito_availability) {
-  // TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407.
-  SCOPED_UMA_HISTOGRAM_TIMER("WinJumplist.UpdateJumpListDuration");
-
   // JumpList is implemented only on Windows 7 or later.
   // So, we should return now when this function is called on earlier versions
   // of Windows.
   if (!JumpListUpdater::IsEnabled())
     return true;
 
   JumpListUpdater jumplist_updater(app_id);
   if (!jumplist_updater.BeginUpdate())
     return false;
 
   // We allocate 60% of the given JumpList slots to "most-visited" items
   // 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.
   const int kMostVisited = 60;
   const int kRecentlyClosed = 40;
   const int kTotal = kMostVisited + kRecentlyClosed;
   size_t most_visited_items =
       MulDiv(jumplist_updater.user_max_items(), kMostVisited, kTotal);
   size_t recently_closed_items =
       jumplist_updater.user_max_items() - most_visited_items;
   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();
   }
 
+  // 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
+  // case, Chrome's icon will be used for the new links.
+
+  if (base::DirectoryExists(icon_dir) && base::IsDirectoryEmpty(icon_dir)) {
+    // TODO(chengx): Remove this UMA metric after fixing http://crbug.com/40407.
+    UMA_HISTOGRAM_COUNTS_100(
+        "WinJumplist.CreateIconFilesCount",
+        most_visited_pages.size() + recently_closed_pages.size());
+
+    // Create icon files for shortcuts in the "Most Visited" category.
+    CreateIconFiles(icon_dir, most_visited_pages, most_visited_items);
+
+    // Create icon files for shortcuts in the "Recently Closed" category.
+    CreateIconFiles(icon_dir, recently_closed_pages, recently_closed_items);
+  }
+
+  // TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407.
+  SCOPED_UMA_HISTOGRAM_TIMER("WinJumplist.UpdateJumpListDuration");

[gab, 2017/04/18 19:21:11]

This changed the timing of this histogram without renaming it, typically we
would have made this "WinJumplist.UpdateJumpListDuration2" in this change.

Now, after commit, though I guess it's fine if you'll always filter by version
while investigating and remove it later.

[chengx, 2017/04/18 19:31:47]

I am now heavily using version filters (thanks for pointing me to that
functionality), so I think this is fine for debugging purpose.

+
   // Update the "Most Visited" category of the JumpList if it exists.
   // This update request is applied into the JumpList when we commit this
   // transaction.
   if (!jumplist_updater.AddCustomCategory(
           l10n_util::GetStringUTF16(IDS_NEW_TAB_MOST_VISITED),
           most_visited_pages, most_visited_items)) {
     return false;
   }
 
   // Update the "Recently Closed" category of the JumpList.
@@ skipping 28 matching lines @@
     // Make sure we are not out of date: if icon_urls_ is not empty, then
     // another notification has been received since we processed this one
     if (!data->icon_urls_.empty())
       return;
 
     // Make local copies of lists so we can release the lock.
     local_most_visited_pages = data->most_visited_pages_;
     local_recently_closed_pages = data->recently_closed_pages_;
   }
 
-  // If JumpListIcons directory doesn't exist or is not empty, skip updating the
-  // jumplist icons. The jumplist links should be updated anyway, as it doesn't
-  // involve disk IO.
-  if (base::DirectoryExists(icon_dir) && base::IsDirectoryEmpty(icon_dir)) {
-    // TODO(chengx): Remove the UMA histogram after fixing
-    // http://crbug.com/40407.
-    UMA_HISTOGRAM_COUNTS_100(
-        "WinJumplist.CreateIconFilesCount",
-        local_most_visited_pages.size() + local_recently_closed_pages.size());
-
-    // Create icon files for shortcuts in the "Most Visited" category.
-    CreateIconFiles(icon_dir, local_most_visited_pages);
-
-    // Create icon files for shortcuts in the "Recently Closed"
-    // category.
-    CreateIconFiles(icon_dir, local_recently_closed_pages);
-  }
-
   // 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,
+  UpdateJumpList(app_id.c_str(), icon_dir, local_most_visited_pages,
                  local_recently_closed_pages, incognito_availability);
 }
 
 }  // namespace
 
 JumpList::JumpListData::JumpListData() {}
 
 JumpList::JumpListData::~JumpListData() {}
 
 JumpList::JumpList(Profile* profile)
@@ skipping 335 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);
 }