Retire some metrics and update file util methods for JumpList

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 46 matching lines @@
 #include "ui/gfx/image/image_family.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_skia_rep.h"
 #include "url/gurl.h"
 
 using content::BrowserThread;
 using JumpListData = JumpList::JumpListData;
 
 namespace {
 
-// Delay jumplist updates to allow collapsing of redundant update requests.
+// The delay before updating the JumpList to prevent update storms.
 constexpr base::TimeDelta kDelayForJumplistUpdate =
     base::TimeDelta::FromMilliseconds(3500);
 
-// Timeout jumplist updates for users with extremely slow OS.
+// The maximum allowed time for JumpListUpdater::BeginUpdate. Updates taking
+// longer than this are discarded to prevent bogging down slow machines.
 constexpr base::TimeDelta kTimeOutForJumplistUpdate =
     base::TimeDelta::FromMilliseconds(500);
 
 // Append the common switches to each shell link.
 void AppendCommonSwitches(ShellLinkItem* shell_link) {
   const char* kSwitchNames[] = { switches::kUserDataDir };
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
   shell_link->GetCommandLine()->CopySwitchesFrom(command_line,
                                                  kSwitchNames,
@@ skipping 37 matching lines @@
     base::DeleteFile(path, false);
     return false;
   }
 
   // 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.
+// Creates icon files for the asynchrounously loaded icons.
 void CreateIconFiles(const base::FilePath& icon_dir,
                      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() && 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 icon files in |icon_dir|, which includes deleting old icons and
+// creating at most |slot_limit| new icons for |page_list|.
+void UpdateIconFiles(const base::FilePath& icon_dir,
+                     const ShellLinkItemList& page_list,
+                     size_t slot_limit) {
+  DeleteDirectoryContentAndLogRuntime(icon_dir, kFileDeleteLimit);
+
+  // Create new icons only when the directory exists and is empty.
+  if (base::CreateDirectory(icon_dir) && base::IsDirectoryEmpty(icon_dir))
+    CreateIconFiles(icon_dir, page_list, slot_limit);
+}
+
 // Updates the "Tasks" category of the JumpList.
 bool UpdateTaskCategory(
     JumpListUpdater* jumplist_updater,
     IncognitoModePrefs::Availability incognito_availability) {
   base::FilePath chrome_path;
   if (!PathService::Get(base::FILE_EXE, &chrome_path))
     return false;
 
   int icon_index = install_static::GetIconResourceIndex();
 
@@ skipping 38 matching lines @@
 bool UpdateJumpList(const wchar_t* app_id,
                     const base::FilePath& icon_dir,
                     const ShellLinkItemList& most_visited_pages,
                     const ShellLinkItemList& recently_closed_pages,
                     bool most_visited_pages_have_updates,
                     bool recently_closed_pages_have_updates,
                     IncognitoModePrefs::Availability incognito_availability) {
   if (!JumpListUpdater::IsEnabled())
     return true;
 
-  // Records the time cost of starting a JumpListUpdater.
+  JumpListUpdater jumplist_updater(app_id);
+
   base::ElapsedTimer begin_update_timer;
 
-  JumpListUpdater jumplist_updater(app_id);
   if (!jumplist_updater.BeginUpdate())
     return false;
 
-  // Stops jumplist update if JumpListUpdater's start times out, as it's very
-  // likely the following update steps will also take a long time.
+  // Discard this JumpList update if JumpListUpdater::BeginUpdate takes longer
+  // than the maximum allowed time, as it's very likely the following update
+  // steps will also take a long time.
   if (begin_update_timer.Elapsed() >= kTimeOutForJumplistUpdate)
     return false;
 
   // The default maximum number of items to display in JumpList is 10.
   // https://msdn.microsoft.com/library/windows/desktop/dd378398.aspx
   // The "Most visited" category title always takes 1 of the JumpList slots if
   // |most_visited_pages| isn't empty.
   // The "Recently closed" category title will also take 1 if
   // |recently_closed_pages| isn't empty.
   // For the remaining slots, we allocate 5/8 (i.e., 5 slots if both categories
@@ skipping 17 matching lines @@
       MulDiv(user_max_items_adjusted, kMostVisited, kTotal);
   size_t recently_closed_items = user_max_items_adjusted - 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();
   }
 
   // Record the desired number of icons to create in this JumpList update.
   int icons_to_create = 0;
 
+  // Update the icons for "Most Visisted" category of the JumpList if needed.
   if (most_visited_pages_have_updates) {
-    // Delete the content in JumpListIconsMostVisited folder and log the results
-    // to UMA.
     base::FilePath icon_dir_most_visited = icon_dir.DirName().Append(
         icon_dir.BaseName().value() + FILE_PATH_LITERAL("MostVisited"));
 
-    DeleteDirectoryContentAndLogResults(icon_dir_most_visited,
-                                        kFileDeleteLimit);
+    UpdateIconFiles(icon_dir_most_visited, most_visited_pages,
+                    most_visited_items);
 
-    // If the directory doesn't exist (we have tried to create it in
-    // DeleteDirectoryContentAndLogResults) 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_most_visited) &&
-        base::IsDirectoryEmpty(icon_dir_most_visited)) {
-      // Create icon files for shortcuts in the "Most Visited" category.
-      CreateIconFiles(icon_dir_most_visited, most_visited_pages,
-                      most_visited_items);
-
-      icons_to_create +=
-          std::min(most_visited_pages.size(), most_visited_items);
-    }
+    icons_to_create += std::min(most_visited_pages.size(), most_visited_items);
   }
 
+  // Update the icons for "Recently Closed" category of the JumpList if needed.
   if (recently_closed_pages_have_updates) {
-    // Delete the content in JumpListIconsRecentClosed folder and log the
-    // results to UMA.
     base::FilePath icon_dir_recent_closed = icon_dir.DirName().Append(
         icon_dir.BaseName().value() + FILE_PATH_LITERAL("RecentClosed"));
 
-    DeleteDirectoryContentAndLogResults(icon_dir_recent_closed,
-                                        kFileDeleteLimit);
+    UpdateIconFiles(icon_dir_recent_closed, recently_closed_pages,
+                    recently_closed_items);
 
-    if (base::DirectoryExists(icon_dir_recent_closed) &&
-        base::IsDirectoryEmpty(icon_dir_recent_closed)) {
-      // Create icon files for shortcuts in the "Recently Closed" category.
-      CreateIconFiles(icon_dir_recent_closed, recently_closed_pages,
-                      recently_closed_items);
-
-      icons_to_create +=
-          std::min(recently_closed_pages.size(), recently_closed_items);
-    }
+    icons_to_create +=
+        std::min(recently_closed_pages.size(), recently_closed_items);
   }
 
   // TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407.
   UMA_HISTOGRAM_COUNTS_100("WinJumplist.CreateIconFilesCount", icons_to_create);
 
   // TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407.
   SCOPED_UMA_HISTOGRAM_TIMER("WinJumplist.UpdateJumpListDuration");
 
   // Update the "Most Visited" category of the JumpList if it exists.
   // This update request is applied into the JumpList when we commit this
@@ skipping 46 matching lines @@
         data->recently_closed_pages_have_updates_;
 
     // Clear the flags to reflect that we'll take actions on these updates.
     data->most_visited_pages_have_updates_ = false;
     data->recently_closed_pages_have_updates_ = false;
   }
 
   if (!most_visited_pages_have_updates && !recently_closed_pages_have_updates)
     return;
 
-  // Updates the application JumpList. If it fails, reset the flags to true if
+  // Update the application JumpList. If it fails, reset the flags to true if
   // they were so that the corresponding JumpList categories will be tried to
   // update again in the next run.
   if (!UpdateJumpList(
           app_id.c_str(), icon_dir, local_most_visited_pages,
           local_recently_closed_pages, most_visited_pages_have_updates,
           recently_closed_pages_have_updates, incognito_availability)) {
     base::AutoLock auto_lock(data->list_lock_);
     if (most_visited_pages_have_updates)
       data->most_visited_pages_have_updates_ = true;
     if (recently_closed_pages_have_updates)
@@ skipping 325 matching lines @@
   // Post a task to update the JumpList, which consists of 1) delete old icons,
   // 2) create new icons, 3) notify the OS.
   update_jumplist_task_runner_->PostTask(
       FROM_HERE, base::Bind(&RunUpdateJumpList, incognito_availability, app_id_,
                             icon_dir_, base::RetainedRef(jumplist_data_)));
 
   // Post a task to delete JumpListIcons folder as it's no longer needed.
   // Now we have JumpListIconsMostVisited folder and JumpListIconsRecentClosed
   // folder instead.
   delete_jumplisticons_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&DeleteDirectoryAndLogResults, icon_dir_, kFileDeleteLimit));
+      FROM_HERE, base::Bind(&DeleteDirectory, icon_dir_, kFileDeleteLimit));
 
   // Post a task to delete JumpListIconsOld folder as it's no longer needed.
   base::FilePath icon_dir_old = icon_dir_.DirName().Append(
       icon_dir_.BaseName().value() + FILE_PATH_LITERAL("Old"));
 
   delete_jumplisticons_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&DeleteDirectoryAndLogResults,
-                            std::move(icon_dir_old), kFileDeleteLimit));
+      FROM_HERE,
+      base::Bind(&DeleteDirectory, std::move(icon_dir_old), kFileDeleteLimit));
 }
 
 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);
 }