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

Issue 1050833002: cc: Remove background ticking from LTHI. (Closed)

Created:
5 years, 8 months ago by mithro-old
Modified:
5 years, 8 months ago
CC:
cc-bugs_chromium.org, chromium-reviews, scheduler-bugs_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

cc: Remove background ticking from LTHI. Currently LTHI "background ticks" animations once every second. This background ticking doesn't have any js detectable effect, nor user visible effects, which means it is just wasting CPU cycles (and thus power). BUG=371747 R=jdduke,brianderson,danakj Committed: https://crrev.com/4c7c7690739a19c417701e38264a460eb563da66 Cr-Commit-Position: refs/heads/master@{#324389}

Patch Set 1 #

Total comments: 8

Patch Set 2 : Fixing for review comments. #

Patch Set 3 : Rebase onto master now that the split out CLs have been landed. #

Patch Set 4 : Another attempt at rebasing. #

Total comments: 3

Patch Set 5 : Rebase onto master. #

Patch Set 6 : Rebase and small fix. #

Patch Set 7 : Rebase onto master. #

Patch Set 8 : Rebase onto master. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -358 lines) Patch
M cc/scheduler/scheduler.h View 1 2 3 4 3 chunks +0 lines, -5 lines 0 comments Download
M cc/scheduler/scheduler.cc View 1 2 3 4 5 chunks +10 lines, -38 lines 0 comments Download
M cc/scheduler/scheduler_state_machine.cc View 1 2 3 4 5 6 7 2 chunks +6 lines, -9 lines 0 comments Download
M cc/scheduler/scheduler_state_machine_unittest.cc View 1 2 3 4 5 6 7 3 chunks +27 lines, -6 lines 0 comments Download
M cc/scheduler/scheduler_unittest.cc View 1 2 3 4 2 chunks +7 lines, -2 lines 0 comments Download
M cc/test/scheduler_test_common.h View 1 chunk +0 lines, -2 lines 0 comments Download
M cc/test/scheduler_test_common.cc View 1 chunk +0 lines, -15 lines 0 comments Download
M cc/trees/layer_tree_host_unittest_animation.cc View 1 2 chunks +0 lines, -281 lines 0 comments Download

Messages

Total messages: 17 (4 generated)
mithro-old
Hi everyone, This patch removes background ticking. I'm expecting the bots to explode as I ...
5 years, 8 months ago (2015-04-01 10:37:26 UTC) #1
jdduke (slow)
https://codereview.chromium.org/1050833002/diff/1/cc/scheduler/scheduler.cc File cc/scheduler/scheduler.cc (left): https://codereview.chromium.org/1050833002/diff/1/cc/scheduler/scheduler.cc#oldcode184 cc/scheduler/scheduler.cc:184: frame_source_->SetActiveSource(background_frame_source_); Why not set the active source to null ...
5 years, 8 months ago (2015-04-01 19:21:21 UTC) #2
jdduke (slow)
On 2015/04/01 19:21:21, jdduke wrote: Oh, and thanks for tackling this!
5 years, 8 months ago (2015-04-01 19:21:49 UTC) #3
sunnyps
On 2015/04/01 10:37:26, mithro wrote: > Hi everyone, > > This patch removes background ticking. ...
5 years, 8 months ago (2015-04-01 19:55:38 UTC) #4
Ian Vollick
On 2015/04/01 19:55:38, sunnyps wrote: > On 2015/04/01 10:37:26, mithro wrote: > > Hi everyone, ...
5 years, 8 months ago (2015-04-01 19:57:46 UTC) #5
brianderson
https://codereview.chromium.org/1050833002/diff/1/cc/scheduler/scheduler.cc File cc/scheduler/scheduler.cc (right): https://codereview.chromium.org/1050833002/diff/1/cc/scheduler/scheduler.cc#newcode156 cc/scheduler/scheduler.cc:156: void Scheduler::UpdateActiveFrameSource() { Delete? https://codereview.chromium.org/1050833002/diff/1/cc/scheduler/scheduler_state_machine.cc File cc/scheduler/scheduler_state_machine.cc (right): https://codereview.chromium.org/1050833002/diff/1/cc/scheduler/scheduler_state_machine.cc#newcode681 ...
5 years, 8 months ago (2015-04-01 21:58:09 UTC) #6
mithro-old
PTAL. https://codereview.chromium.org/1050833002/diff/1/cc/scheduler/scheduler.cc File cc/scheduler/scheduler.cc (left): https://codereview.chromium.org/1050833002/diff/1/cc/scheduler/scheduler.cc#oldcode184 cc/scheduler/scheduler.cc:184: frame_source_->SetActiveSource(background_frame_source_); On 2015/04/01 19:21:21, jdduke wrote: > Why ...
5 years, 8 months ago (2015-04-02 01:03:59 UTC) #8
sunnyps
Left a comment. https://codereview.chromium.org/1050833002/diff/80001/cc/scheduler/scheduler_state_machine.cc File cc/scheduler/scheduler_state_machine.cc (right): https://codereview.chromium.org/1050833002/diff/80001/cc/scheduler/scheduler_state_machine.cc#newcode745 cc/scheduler/scheduler_state_machine.cc:745: return false; Better move this to ...
5 years, 8 months ago (2015-04-07 22:55:04 UTC) #10
brianderson
lgtm https://codereview.chromium.org/1050833002/diff/80001/cc/scheduler/scheduler_state_machine.cc File cc/scheduler/scheduler_state_machine.cc (right): https://codereview.chromium.org/1050833002/diff/80001/cc/scheduler/scheduler_state_machine.cc#newcode745 cc/scheduler/scheduler_state_machine.cc:745: return false; On 2015/04/07 22:55:04, sunnyps wrote: > ...
5 years, 8 months ago (2015-04-08 01:21:59 UTC) #11
mithro-old
https://codereview.chromium.org/1050833002/diff/80001/cc/scheduler/scheduler_state_machine.cc File cc/scheduler/scheduler_state_machine.cc (right): https://codereview.chromium.org/1050833002/diff/80001/cc/scheduler/scheduler_state_machine.cc#newcode745 cc/scheduler/scheduler_state_machine.cc:745: return false; On 2015/04/08 01:21:59, brianderson wrote: > On ...
5 years, 8 months ago (2015-04-08 23:40:51 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1050833002/160001
5 years, 8 months ago (2015-04-09 06:58:26 UTC) #15
commit-bot: I haz the power
Committed patchset #8 (id:160001)
5 years, 8 months ago (2015-04-09 07:05:09 UTC) #16
commit-bot: I haz the power
5 years, 8 months ago (2015-04-09 07:06:14 UTC) #17
Message was sent while issue was closed.
Patchset 8 (id:??) landed as
https://crrev.com/4c7c7690739a19c417701e38264a460eb563da66
Cr-Commit-Position: refs/heads/master@{#324389}

Powered by Google App Engine
This is Rietveld 408576698