Cancel the next 10 updates if there's a timeout in jumplist updater

chrome/browser/win/jumplist.cc

Index: chrome/browser/win/jumplist.cc
diff --git a/chrome/browser/win/jumplist.cc b/chrome/browser/win/jumplist.cc
index 59377fbee9fd99d9f1bbbf9b8f9c3ca01883a3ea..ea798dbf63d7f9e87377ad8d3e0ba926215fa3cf 100644
--- a/chrome/browser/win/jumplist.cc
+++ b/chrome/browser/win/jumplist.cc
@@ -64,15 +64,28 @@ using JumpListData = JumpList::JumpListData;
 
 namespace {
 
+// The maximum allowed number of updates to skip to alleviate the machine when a
+// previous update was too slow.
+constexpr int kMaxUpdatesToSkip = 10;

[gab, 2017/05/12 18:59:59]

kUpdatesToSkipUnderHeavyLoad

(i.e. this isn't a "max", it *is* the number of updates you'll skip)

[chengx, 2017/05/12 19:23:57]

Agreed. Done.

+
 // 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 kTimeOutForJumplistUpdate =
+constexpr base::TimeDelta kTimeOutForJumplistBeginUpdate =
+    base::TimeDelta::FromMilliseconds(500);
+
+// The maximum allowed time for updating both "most visited" and "recently
+// closed" categories via JumpListUpdater::AddCustomCategory.
+constexpr base::TimeDelta kTimeOutForAddCustomCategory =
     base::TimeDelta::FromMilliseconds(500);
 
+// 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 };
@@ -523,6 +536,11 @@ void JumpList::TopSitesChanged(history::TopSites* top_sites,
 }
 
 void JumpList::DeferredTopSitesChanged() {
+  if (updates_to_skip_ > 0) {
+    --updates_to_skip_;
+    return;
+  }
+
   scoped_refptr<history::TopSites> top_sites =
       TopSitesFactory::GetForProfile(profile_);
   if (top_sites) {
@@ -534,6 +552,11 @@ void JumpList::DeferredTopSitesChanged() {
 }
 
 void JumpList::DeferredTabRestoreServiceChanged() {
+  if (updates_to_skip_ > 0) {
+    --updates_to_skip_;
+    return;
+  }
+
   // Create a list of ShellLinkItems from the "Recently Closed" pages.
   // As noted above, we create a ShellLinkItem objects with the following
   // parameters.
@@ -620,8 +643,10 @@ bool JumpList::UpdateJumpList(
   // 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)
+  if (begin_update_timer.Elapsed() >= kTimeOutForJumplistBeginUpdate) {
+    updates_to_skip_ = kMaxUpdatesToSkip;
     return false;
+  }
 
   // The default maximum number of items to display in JumpList is 10.
   // https://msdn.microsoft.com/library/windows/desktop/dd378398.aspx
@@ -686,6 +711,8 @@ bool JumpList::UpdateJumpList(
   // TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407.
   SCOPED_UMA_HISTOGRAM_TIMER("WinJumplist.UpdateJumpListDuration");
 
+  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.
@@ -702,12 +729,26 @@ bool JumpList::UpdateJumpList(
     return false;
   }
 
+  // If JumpListUpdater::AddCustomCategory takes longer than the maximum allowed
+  // time, mark the flag but continue with this update.

[gab, 2017/05/12 19:00:00]

Explain why you continue with the update in this case

Also, it's no longer a "flag"

[chengx, 2017/05/12 19:23:57]

I've updated the comments to explain why this update should be continued in
particular. Thanks!

+  if (add_custom_category_timer.Elapsed() >= kTimeOutForAddCustomCategory)
+    updates_to_skip_ = kMaxUpdatesToSkip;
+
   // Update the "Tasks" category of the JumpList.
   if (!UpdateTaskCategory(&jumplist_updater, incognito_availability))
     return false;
 
+  base::ElapsedTimer commit_update_timer;
+
   // Commit this transaction and send the updated JumpList to Windows.
-  return jumplist_updater.CommitUpdate();
+  bool commit_result = jumplist_updater.CommitUpdate();
+
+  // If JumpListUpdater::CommitUpdate takes longer than the maximum allowed
+  // time, mark the flag but continue with this update.

[gab, 2017/05/12 19:00:00]

ditto

[chengx, 2017/05/12 19:23:58]

Done.

+  if (commit_update_timer.Elapsed() >= kTimeOutForJumplistCommitUpdate)
+    updates_to_skip_ = kMaxUpdatesToSkip;
+
+  return commit_result;
 }
 
 void JumpList::RunUpdateJumpList(