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..7642498752ea436cdfdff3b3c6f6186e4da70ab9 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"
@@ -64,7 +65,12 @@ using JumpListData = JumpList::JumpListData;
 namespace {
 
 // Delay jumplist updates to allow collapsing of redundant update requests.
-const int kDelayForJumplistUpdateInMS = 3500;
+constexpr base::TimeDelta kDelayForJumplistUpdateInMS =
+    base::TimeDelta::FromMilliseconds(3500);
+
+// Timeout jumplist updates for users with extremely slow OS.
+constexpr base::TimeDelta kTimeOutForJumplistUpdateInMS =
+    base::TimeDelta::FromMilliseconds(500);
 
 // Append the common switches to each shell link.
 void AppendCommonSwitches(ShellLinkItem* shell_link) {
@@ -188,10 +194,18 @@ 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() >= 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|,
@@ -218,22 +232,13 @@ bool UpdateJumpList(const wchar_t* app_id,
   // 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());

[Ilya Sherman, 2017/04/20 21:24:24]

Should this metric be marked as obsolete as well?

[chengx, 2017/04/20 21:49:04]

Yes, it's obsolete. Now it's fixed in histograms.xml.

+  bool is_dir_existed_and_empty =
+      base::DirectoryExists(icon_dir) && base::IsDirectoryEmpty(icon_dir);
 
-    // Create icon files for shortcuts in the "Most Visited" category.
+  // Create icon files for shortcuts in the "Most Visited" category.
+  if (is_dir_existed_and_empty)
     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");
-
   // Update the "Most Visited" category of the JumpList if it exists.
   // This update request is applied into the JumpList when we commit this
   // transaction.
@@ -243,6 +248,10 @@ bool UpdateJumpList(const wchar_t* app_id,
     return false;
   }
 
+  // Create icon files for shortcuts in the "Recently Closed" category.
+  if (is_dir_existed_and_empty)
+    CreateIconFiles(icon_dir, recently_closed_pages, recently_closed_items);
+
   // Update the "Recently Closed" category of the JumpList.
   if (!jumplist_updater.AddCustomCategory(
           l10n_util::GetStringUTF16(IDS_RECENTLY_CLOSED),
@@ -595,9 +604,7 @@ void JumpList::PostRunUpdate() {
   if (timer_.IsRunning()) {
     timer_.Reset();
   } else {
-    timer_.Start(FROM_HERE,
-                 base::TimeDelta::FromMilliseconds(kDelayForJumplistUpdateInMS),
-                 this,
+    timer_.Start(FROM_HERE, kDelayForJumplistUpdateInMS, this,
                  &JumpList::DeferredRunUpdate);
   }
 }