Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(14)

Issue 2516493002: requestIdleCallback: Yield for high priority work (Closed)

Created:
4 years, 1 month ago by Sami
Modified:
4 years, 1 month ago
CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, rwlbuis, sof
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

requestIdleCallback: Yield for high priority work When a high priority task is pending (e.g., user input) while an idle callback is running, we should reduce the remaining idle period length to zero to signal the idle callback to exit. This can help reduce touch latency. BUG=653352 Committed: https://crrev.com/c96e8e1003ed954a54110e0ec652e46ef7a962cb Cr-Commit-Position: refs/heads/master@{#433621}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Added a test #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+91 lines, -7 lines) Patch
M third_party/WebKit/Source/core/BUILD.gn View 1 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/IdleDeadline.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/dom/IdleDeadline.cpp View 1 2 chunks +9 lines, -1 line 1 comment Download
A third_party/WebKit/Source/core/dom/IdleDeadlineTest.cpp View 1 1 chunk +74 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h View 1 3 chunks +2 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/platform/testing/TestingPlatformSupport.cpp View 1 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 34 (22 generated)
Sami
PTAL. I'm not sure we have a great way to test this...Maybe a SimTest would ...
4 years, 1 month ago (2016-11-17 22:57:48 UTC) #5
rmcilroy
Awesome, thanks! LGTM with optional suggestion. https://codereview.chromium.org/2516493002/diff/1/third_party/WebKit/Source/core/dom/IdleDeadline.cpp File third_party/WebKit/Source/core/dom/IdleDeadline.cpp (right): https://codereview.chromium.org/2516493002/diff/1/third_party/WebKit/Source/core/dom/IdleDeadline.cpp#newcode23 third_party/WebKit/Source/core/dom/IdleDeadline.cpp:23: } nit - ...
4 years, 1 month ago (2016-11-18 09:13:51 UTC) #8
Sami
Thanks! Added a test. Jochen, mind stamping this please? https://codereview.chromium.org/2516493002/diff/1/third_party/WebKit/Source/core/dom/IdleDeadline.cpp File third_party/WebKit/Source/core/dom/IdleDeadline.cpp (right): https://codereview.chromium.org/2516493002/diff/1/third_party/WebKit/Source/core/dom/IdleDeadline.cpp#newcode23 third_party/WebKit/Source/core/dom/IdleDeadline.cpp:23: ...
4 years, 1 month ago (2016-11-21 11:28:03 UTC) #14
jochen (gone - plz use gerrit)
lgtm
4 years, 1 month ago (2016-11-21 12:03:11 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2516493002/20001
4 years, 1 month ago (2016-11-21 13:23:05 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/276755)
4 years, 1 month ago (2016-11-21 15:28:42 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2516493002/20001
4 years, 1 month ago (2016-11-21 15:29:07 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/276866)
4 years, 1 month ago (2016-11-21 19:33:36 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2516493002/20001
4 years, 1 month ago (2016-11-21 19:35:36 UTC) #27
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 1 month ago (2016-11-21 20:14:17 UTC) #30
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/c96e8e1003ed954a54110e0ec652e46ef7a962cb Cr-Commit-Position: refs/heads/master@{#433621}
4 years, 1 month ago (2016-11-21 20:18:51 UTC) #32
esprehn
4 years, 1 month ago (2016-11-21 20:32:28 UTC) #34
Message was sent while issue was closed.
https://codereview.chromium.org/2516493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/IdleDeadline.cpp (right):

https://codereview.chromium.org/2516493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/IdleDeadline.cpp:20: timeRemaining = 0;
Early return in blink.

if (timeRemaining < 0)
  return 0;

if (....)
  return 0;

return 1000.0 * PerformanceBase::clampTimeResolution(timeRemaining);

Powered by Google App Engine
This is Rietveld 408576698