Cancel coming updates when certain JumpList APIs time out to fail

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 "base/base_paths.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ skipping 58 matching lines @@
 
 // The delay before updating the JumpList to prevent update storms.
 constexpr base::TimeDelta kDelayForJumplistUpdate =
     base::TimeDelta::FromMilliseconds(3500);
 
 // The maximum allowed time for JumpListUpdater::BeginUpdate. Updates taking
 // longer than this are discarded to prevent bogging down slow machines.
 constexpr base::TimeDelta kTimeOutForJumplistBeginUpdate =
     base::TimeDelta::FromMilliseconds(500);
 
-// The maximum allowed time for updating both "most visited" and "recently
-// closed" categories via JumpListUpdater::AddCustomCategory.
+// The maximum allowed time for adding most visited pages custom category via
+// JumpListUpdater::AddCustomCategory.
 constexpr base::TimeDelta kTimeOutForAddCustomCategory =
-    base::TimeDelta::FromMilliseconds(500);
+    base::TimeDelta::FromMilliseconds(320);
 
 // The maximum allowed time for JumpListUpdater::CommitUpdate.
 constexpr base::TimeDelta kTimeOutForJumplistCommitUpdate =
     base::TimeDelta::FromMilliseconds(1000);
 
 // Appends 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();
@@ skipping 20 matching lines @@
   if (!base::CreateTemporaryFileInDir(icon_dir, &path))
     return false;
 
   // Create an icon file from the favicon attached to the given |page|, and
   // save it as the temporary file.
   gfx::ImageFamily image_family;
   if (!image_skia.isNull()) {
     std::vector<float> supported_scales = image_skia.GetSupportedScales();
     for (auto& scale : supported_scales) {
       gfx::ImageSkiaRep image_skia_rep = image_skia.GetRepresentation(scale);
-      if (!image_skia_rep.is_null())
+      if (!image_skia_rep.is_null()) {
         image_family.Add(
             gfx::Image::CreateFrom1xBitmap(image_skia_rep.sk_bitmap()));
+      }
     }
   }
 
   if (!IconUtil::CreateIconFileFromImageFamily(image_family, path,
                                                IconUtil::NORMAL_WRITE)) {
     // Delete the file created by CreateTemporaryFileInDir as it won't be used.
     base::DeleteFile(path, false);
     return false;
   }
 
@@ skipping 575 matching lines @@
     bool most_visited_should_update,
     bool recently_closed_should_update,
     IncognitoModePrefs::Availability incognito_availability,
     UpdateTransaction* update_transaction) {
   DCHECK(update_transaction);
 
   JumpListUpdater jumplist_updater(app_id);
 
   base::ElapsedTimer begin_update_timer;
 
-  if (!jumplist_updater.BeginUpdate())
-    return;
+  bool begin_success = jumplist_updater.BeginUpdate();
 
-  // 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 JumpListUpdater::BeginUpdate takes longer than the maximum allowed time,
+  // abort the current update as it's very likely the following steps will also
+  // take a long time, and skip the next |kUpdatesToSkipUnderHeavyLoad| updates.
   if (begin_update_timer.Elapsed() >= kTimeOutForJumplistBeginUpdate) {
     update_transaction->update_timeout = true;
     return;
   }
 
+  if (!begin_success)
+    return;
+
   // Record the desired number of icons created in this JumpList update.
   int icons_created = 0;
 
   URLIconCache most_visited_icons_next;
   URLIconCache recently_closed_icons_next;
 
   // Update the icons for "Most Visisted" category of the JumpList if needed.
   if (most_visited_should_update) {
     icons_created += UpdateIconFiles(
         most_visited_icon_dir, most_visited_pages, kMostVisitedItems,
         &update_transaction->most_visited_icons, &most_visited_icons_next);
   }
 
   // Update the icons for "Recently Closed" category of the JumpList if needed.
   if (recently_closed_should_update) {
     icons_created += UpdateIconFiles(
         recently_closed_icon_dir, recently_closed_pages, kRecentlyClosedItems,
         &update_transaction->recently_closed_icons,
         &recently_closed_icons_next);
   }
 
   base::ElapsedTimer add_custom_category_timer;
 
   // 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, kMostVisitedItems)) {
+  bool add_category_success = jumplist_updater.AddCustomCategory(
+      l10n_util::GetStringUTF16(IDS_NEW_TAB_MOST_VISITED), most_visited_pages,
+      kMostVisitedItems);
+
+  // If AddCustomCategory takes longer than the maximum allowed time, abort the
+  // current update and skip the next |kUpdatesToSkipUnderHeavyLoad| updates.
+  //
+  // We only time adding custom category for most visited pages because
+  // 1. If processing the first category times out or fails, there is no need to
+  //    process the second category. In this case, we are not able to time both
+  //    categories. Then we need to select one category from the two.
+  // 2. Most visited category is selected because it always has 5 items except
+  //    for a new Chrome user who has not closed 5 distinct websites yet. In
+  //    comparison, the number of items in recently closed category is much less
+  //    stable. It has 3 items only after an user closes 3 websites in one
+  //    session. This means the runtime of AddCustomCategory API should be fixed
+  //    for most visited category, but not for recently closed category. So a
+  //    fixed timeout threshold is only valid for most visited category.
+  if (add_custom_category_timer.Elapsed() >= kTimeOutForAddCustomCategory) {
+    update_transaction->update_timeout = true;
     return;
   }
 
+  if (!add_category_success)
+    return;
+
   base::TimeDelta most_visited_category_time =
       add_custom_category_timer.Elapsed();
 
   // Update the "Recently Closed" category of the JumpList.
   if (!jumplist_updater.AddCustomCategory(
           l10n_util::GetStringUTF16(IDS_RECENTLY_CLOSED), recently_closed_pages,
           kRecentlyClosedItems)) {
     return;
   }
 
@@ skipping 113 matching lines @@
                                const URLIconCache& icons_cache) {
   // Put all cached icon file paths into a set.
   base::flat_set<base::FilePath> cached_files;
   cached_files.reserve(icons_cache.size());
 
   for (const auto& url_path_pair : icons_cache)
     cached_files.insert(url_path_pair.second);
 
   DeleteNonCachedFiles(icon_dir, cached_files);
 }