Time out jumplist update for very slow or busy OS

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 461c5a4a45b5371c7c64ff66d79060b35a5c7bdd..6a01c08437f3516d535d55f02651489d7953628c 100644
--- a/chrome/browser/win/jumplist.cc
+++ b/chrome/browser/win/jumplist.cc
@@ -20,6 +20,7 @@
 #include "base/task_scheduler/post_task.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/timer/elapsed_timer.h"
 #include "base/trace_event/trace_event.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
@@ -66,6 +67,9 @@ namespace {
 // Delay jumplist updates to allow collapsing of redundant update requests.
 const int kDelayForJumplistUpdateInMS = 3500;
 
+// Timeout jumplist updates for users with extremely slow OS.
+const int kTimeOutForJumplistUpdateInMS = 500;

[grt (UTC plus 2), 2017/04/20 12:48:53]

nit:
constexpr base::TimeDelta kTimeOutForJumplistUpdate =
    base::TimeDelta::FromMilliseconds(500);

[chengx, 2017/04/20 21:01:36]

Done.

+
 // Append the common switches to each shell link.
 void AppendCommonSwitches(ShellLinkItem* shell_link) {
   const char* kSwitchNames[] = { switches::kUserDataDir };
@@ -188,10 +192,19 @@ bool UpdateJumpList(const wchar_t* app_id,
   if (!JumpListUpdater::IsEnabled())
     return true;
 
+  // Records the time cost of starting a JumpListUpdater.
+  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.
+  if (begin_update_timer.Elapsed().InMilliseconds() >=

[grt (UTC plus 2), 2017/04/20 12:48:53]

nit:
  if (begin_update_timer.Elapsed() >= kTimeOutForJumplistUpdate)

[chengx, 2017/04/20 21:01:36]

Done.

+      kTimeOutForJumplistUpdateInMS)
+    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|,
@@ -234,6 +247,9 @@ bool UpdateJumpList(const wchar_t* app_id,
   // TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407.
   SCOPED_UMA_HISTOGRAM_TIMER("WinJumplist.UpdateJumpListDuration");
 
+  // Records the time cost of AddCustomCategory in JumpListUpdater.
+  base::ElapsedTimer add_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.
@@ -250,6 +266,12 @@ bool UpdateJumpList(const wchar_t* app_id,
     return false;
   }
 
+  // Stops jumplist update if AddCustomCategory times out, as it's very likely
+  // the following CommitUpdate call will also take a long time.
+  if (add_category_timer.Elapsed().InMilliseconds() >=

[grt (UTC plus 2), 2017/04/20 12:48:53]

here, too

[chengx, 2017/04/20 21:01:36]

I decided to only keep a timer for BeginUpdate for now to see what happens
first. So this section of code is gone now.

+      kTimeOutForJumplistUpdateInMS)
+    return false;
+
   // Update the "Tasks" category of the JumpList.
   if (!UpdateTaskCategory(&jumplist_updater, incognito_availability))
     return false;