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

Issue 2828873006: Revert of Revert "cc: Make scheduler run incoming frame after previous deadline." (Closed)

Created:
3 years, 8 months ago by sunnyps
Modified:
3 years, 8 months ago
Reviewers:
fhorschig, cc-bugs, enne (OOO), scheduler-bugs, chromium-reviews
CC:
chromium-reviews, cc-bugs_chromium.org, scheduler-bugs_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Revert "cc: Make scheduler run incoming frame after previous deadline." (patchset #1 id:1 of https://codereview.chromium.org/2832503005/ ) Reason for revert: Didn't cause the test failures (see crbug.com/713740) Original issue's description: > Revert "cc: Make scheduler run incoming frame after previous deadline." > > This reverts commit d920c93c9c963460b835d19ca4540fe86640b19f. > (Reupload from https://chromium-review.googlesource.com/c/482780/ > due to merge conflicts during rebase.) > > Reason for revert: > > Seems to have caused test failures: > https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Win7__dbg_/9549/layout-test-results/fast/events/pointerevents/pointer-event-consumed-touchstart-in-slop-region-actual.txt > and > https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Win7__dbg_/9549/layout-test-results/fast/spatial-navigation/snav-z-index-pretty-diff.html > > https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Win7%20%28dbg%29/builds/9549 > > Original change's description: > > cc: Make scheduler run incoming frame after previous deadline. > > > > When removing retro frames I made the incoming frame run the previous > > frame's deadline synchronously. This is believed to have regressed > > Event.Latency.OS.TOUCH_MOVED UMA metric. This CL changes the scheduler > > to queue the incoming frame and post a task for it after the previous > > deadline runs. This is a speculative fix for the UMA regression. > > > > R=​enne > > BUG=702372 > > > > CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel > > > > Change-Id: I0d8b0a5df90186b2158e4249540929b9b5ecc70b > > Reviewed-on: https://chromium-review.googlesource.com/478852 > > Reviewed-by: enne <enne@chromium.org>; > > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>; > > Cr-Commit-Position: refs/heads/master@{#465769} > > TBR=enne@chromium.org,sunnyps@chromium.org,chromium-reviews@chromium.org,cc-bugs@chromium.org,scheduler-bugs@chromium.org > # Not skipping any steps as this is a manual revert. > BUG=702372 > CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel > > Review-Url: https://codereview.chromium.org/2832503005 > Cr-Commit-Position: refs/heads/master@{#465972} > Committed: https://chromium.googlesource.com/chromium/src/+/95f34789951a95ea306db1b8e9b568696a3cacaa TBR=cc-bugs@chromium.org,chromium-reviews@chromium.org,enne@chromium.org,scheduler-bugs@chromium.org,fhorschig@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=702372 Review-Url: https://codereview.chromium.org/2828873006 Cr-Commit-Position: refs/heads/master@{#466122} Committed: https://chromium.googlesource.com/chromium/src/+/b0fa1213d8f5bfe59ad4f1ea045960acdd3b1e42

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+162 lines, -78 lines) Patch
M cc/scheduler/begin_frame_tracker.h View 1 chunk +1 line, -2 lines 0 comments Download
M cc/scheduler/begin_frame_tracker.cc View 2 chunks +1 line, -1 line 0 comments Download
M cc/scheduler/scheduler.h View 3 chunks +5 lines, -2 lines 0 comments Download
M cc/scheduler/scheduler.cc View 8 chunks +92 lines, -70 lines 0 comments Download
M cc/scheduler/scheduler_unittest.cc View 3 chunks +63 lines, -3 lines 0 comments Download

Messages

Total messages: 6 (3 generated)
sunnyps
Created Revert of Revert "cc: Make scheduler run incoming frame after previous deadline."
3 years, 8 months ago (2017-04-20 20:51:02 UTC) #2
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/2828873006/1
3 years, 8 months ago (2017-04-20 20:51:48 UTC) #3
commit-bot: I haz the power
3 years, 8 months ago (2017-04-20 20:53:42 UTC) #6
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/b0fa1213d8f5bfe59ad4f1ea0459...

Powered by Google App Engine
This is Rietveld 408576698