Make ui::Compositor use ui::Scheduler

Make ui::Compositor use ui::Scheduler

Taken from enne's CL 535733002 and rebased. It has been taken out of CL 134623005.

BUG=329552
Committed: https://crrev.com/36b7fc7f8b05ea627873e58a162c1c26784e472d
Cr-Commit-Position: refs/heads/master@{#298779}

Committed: https://crrev.com/b821b71ff0166e250ae4b30b56c1b7b6d3bd5db6
Cr-Commit-Position: refs/heads/master@{#306954}

Committed: https://crrev.com/5efa0a1c4381c7e68e22285a99d601431060e5c3
Cr-Commit-Position: refs/heads/master@{#313766}

[weiliangc, 2014-10-08 17:48:13]

weiliangc@chromium.org changed reviewers:
+ ben@chromium.org, danakj@chromium.org

[weiliangc, 2014-10-08 17:50:03]

This is from enne's patch with same name, which is taken out of CL "Make
SingleThreadProxy a SchedulerClient".

ui/compositor: danakj
everything else: ben

[weiliangc, 2014-10-08 17:53:36]

weiliangc@chromium.org changed reviewers:
+ sky@chromium.org

[weiliangc, 2014-10-08 17:54:06]

Add sky@ for review everything else other than ui/compositor.

[danakj, 2014-10-08 17:58:10]

LGTM

[sky, 2014-10-08 19:04:49]

LGTM

[weiliangc, 2014-10-08 19:06:02]

The CQ bit was checked by weiliangc@chromium.org

[commit-bot: I haz the power, 2014-10-08 19:07:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/638653003/20001

[commit-bot: I haz the power, 2014-10-09 04:44:29]

Committed patchset #2 (id:20001) as eec8e4e3e3f50d3a0094dc82bc9638ee7c0ce2cf

[commit-bot: I haz the power, 2014-10-09 04:45:50]

Patchset 2 (id:??) landed as
https://crrev.com/36b7fc7f8b05ea627873e58a162c1c26784e472d
Cr-Commit-Position: refs/heads/master@{#298779}

[maniscalco, 2014-10-09 16:25:11]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/643693002/ by maniscalco@chromium.org.

The reason for reverting is: Speculatively reverting as I suspect it may be the
cause of test failures like this:

[16079:16079:1009/052004:FATAL:compositor.cc(303)] Check failed: !IsLocked().
#0 0x0000eb62e4dc base::debug::StackTrace::StackTrace()
#1 0x0000eb6be0ed logging::LogMessage::~LogMessage()
#2 0x0000eb1b9cf0 ui::Compositor::DidCommit()
#3 0x0000ebcb20e5 cc::LayerTreeHost::CommitComplete()
#4 0x0000ebd30331 cc::SingleThreadProxy::DoCommit()

http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29...
.

[weiliangc, 2014-10-14 20:41:48]

Investigated crash causing revert:
Compositor lock was grabbed after finishing BeginFrame and before DoCommit,
which is too late to stop the current commit, and should go ahead without crash.
Instead DCHECK at BeginMainFrame, since if lock is grabbed, commit should be
deferred and won't end up in BeginMainFrame().

[danakj, 2014-10-14 20:43:01]

Yep that LGTM. There were a few other bugs also around this. Where are we at
with them?

[weiliangc, 2014-10-14 20:48:18]

On 2014/10/14 at 20:43:01, danakj wrote:
> Yep that LGTM. There were a few other bugs also around this. Where are we at
with them?

This is the bug caused the revert. (All cases are IsLocked() DCHECK inside
DidCommit(), though at different tests)

From before from enne's patch:
- layer_unittest: fixed.
- double free: disappeared.
- input latency regression: STP is probably the culprit.

[piman, 2014-10-14 20:50:10]

piman@chromium.org changed reviewers:
+ piman@chromium.org

[piman, 2014-10-14 20:50:11]

https://codereview.chromium.org/638653003/diff/180001/ui/compositor/composito...
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/638653003/diff/180001/ui/compositor/composito...
ui/compositor/compositor.cc:306: // through it anyway.
I think that's a problem... It means we'll flash depending on timing of input vs
scheduler (e.g. on resize), won't it?

[danakj, 2014-10-14 20:52:49]

https://codereview.chromium.org/638653003/diff/180001/ui/compositor/composito...
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/638653003/diff/180001/ui/compositor/composito...
ui/compositor/compositor.cc:306: // through it anyway.
On 2014/10/14 20:50:11, piman (Very slow to review) wrote:
> I think that's a problem... It means we'll flash depending on timing of input
vs
> scheduler (e.g. on resize), won't it?

Once beginframe has happened the main thread has already decided on what the
frame will look like.

I guess on the threaded case it blocks though... but here it doesn't block. So
it could change things in between. Maybe this is a larger problem.

[weiliangc, 2014-12-04 18:33:02]

- Rebased on top of the main_thread_low_latency mode.
- CommitComplete is called synchronously.
^ These fixes browser tests browser compositor resize lock being grabbed
problem.

Remaining:
- power monitor is not set up for unittest which caused the win trybots fails.
Scheduler DCHECK on power monitor in win, and before tests using compositor
doesn't use scheduler so didn't trigger this before.

[weiliangc, 2014-12-04 20:34:33]

On 2014/12/04 at 18:33:02, weiliangc wrote:
> - Rebased on top of the main_thread_low_latency mode.
> - CommitComplete is called synchronously.
> ^ These fixes browser tests browser compositor resize lock being grabbed
problem.
> 
> Remaining:
> - power monitor is not set up for unittest which caused the win trybots fails.
Scheduler DCHECK on power monitor in win, and before tests using compositor
doesn't use scheduler so didn't trigger this before.

Do not use disable_hi_res_timer_tasks_on_battery when it is test and there is no
power monitor set up. This is done through InitializeContextFactoryForTest.

[danakj, 2014-12-04 21:05:03]

https://codereview.chromium.org/638653003/diff/260001/ui/aura/bench/bench_mai...
File ui/aura/bench/bench_main.cc (right):

https://codereview.chromium.org/638653003/diff/260001/ui/aura/bench/bench_mai...
ui/aura/bench/bench_main.cc:301: new ui::InProcessContextFactory(false));
give boolean literals a name with a temp variable please

[weiliangc, 2014-12-04 21:06:49]

https://codereview.chromium.org/638653003/diff/260001/content/browser/composi...
File content/browser/compositor/test/no_transport_image_transport_factory.cc
(right):

https://codereview.chromium.org/638653003/diff/260001/content/browser/composi...
content/browser/compositor/test/no_transport_image_transport_factory.cc:17: :
context_factory_(new ui::InProcessContextFactory(true)),
Is there a better way to do this too? This can't really have a temp variable?

[danakj, 2014-12-04 21:07:54]

https://codereview.chromium.org/638653003/diff/260001/content/browser/composi...
File content/browser/compositor/test/no_transport_image_transport_factory.cc
(right):

https://codereview.chromium.org/638653003/diff/260001/content/browser/composi...
content/browser/compositor/test/no_transport_image_transport_factory.cc:17: :
context_factory_(new ui::InProcessContextFactory(true)),
On 2014/12/04 21:06:49, weiliangc wrote:
> Is there a better way to do this too? This can't really have a temp variable?

Can throw a comment in here above it?

[weiliangc, 2014-12-04 21:48:55]

On 2014/12/04 at 21:07:54, danakj wrote:
>
https://codereview.chromium.org/638653003/diff/260001/content/browser/composi...
> File content/browser/compositor/test/no_transport_image_transport_factory.cc
(right):
> 
>
https://codereview.chromium.org/638653003/diff/260001/content/browser/composi...
> content/browser/compositor/test/no_transport_image_transport_factory.cc:17: :
context_factory_(new ui::InProcessContextFactory(true)),
> On 2014/12/04 21:06:49, weiliangc wrote:
> > Is there a better way to do this too? This can't really have a temp
variable?
> 
> Can throw a comment in here above it?

Done.

[danakj, 2014-12-04 22:22:10]

LGTM

https://codereview.chromium.org/638653003/diff/300001/cc/trees/single_thread_...
File cc/trees/single_thread_proxy.cc (right):

https://codereview.chromium.org/638653003/diff/300001/cc/trees/single_thread_...
cc/trees/single_thread_proxy.cc:431: // commit_blocking_task_runner would make
sure all tasks posted during
nit: |commit_blocking_task_runner_|

https://codereview.chromium.org/638653003/diff/300001/content/browser/composi...
File content/browser/compositor/test/no_transport_image_transport_factory.cc
(right):

https://codereview.chromium.org/638653003/diff/300001/content/browser/composi...
content/browser/compositor/test/no_transport_image_transport_factory.cc:17: //
The context factory created here is for test, thus passing in true in
nit: is for unit tests

[weiliangc, 2014-12-04 22:45:16]

The CQ bit was checked by weiliangc@chromium.org

[commit-bot: I haz the power, 2014-12-04 22:47:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/638653003/320001

[weiliangc, 2014-12-04 22:49:28]

The CQ bit was unchecked by weiliangc@chromium.org

[piman, 2014-12-04 23:05:24]

https://codereview.chromium.org/638653003/diff/340001/ui/compositor/test/in_p...
File ui/compositor/test/in_process_context_factory.h (right):

https://codereview.chromium.org/638653003/diff/340001/ui/compositor/test/in_p...
ui/compositor/test/in_process_context_factory.h:26: InProcessContextFactory(bool
context_factory_for_test);
nit: explicit

[piman, 2014-12-04 23:07:12]

lgtm

[weiliangc, 2014-12-05 01:12:18]

The CQ bit was checked by weiliangc@chromium.org

[commit-bot: I haz the power, 2014-12-05 01:13:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/638653003/360001

[commit-bot: I haz the power, 2014-12-05 01:54:24]

Committed patchset #13 (id:360001)

[commit-bot: I haz the power, 2014-12-05 01:55:12]

Patchset 13 (id:??) landed as
https://crrev.com/b821b71ff0166e250ae4b30b56c1b7b6d3bd5db6
Cr-Commit-Position: refs/heads/master@{#306954}

[Ilya Sherman, 2014-12-06 01:00:02]

A revert of this CL (patchset #13 id:360001) has been created in
https://codereview.chromium.org/784653002/ by isherman@chromium.org.

The reason for reverting is: This appears to be causing crashes on the Windows
perf bots.  See the linked bug for a stack trace.

BUG=439649.

[weiliangc, 2015-01-29 19:30:32]

The CQ bit was checked by weiliangc@chromium.org

[commit-bot: I haz the power, 2015-01-29 19:30:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/638653003/380001

[commit-bot: I haz the power, 2015-01-29 19:57:22]

Committed patchset #14 (id:380001)

[commit-bot: I haz the power, 2015-01-29 19:58:45]

Patchset 14 (id:??) landed as
https://crrev.com/5efa0a1c4381c7e68e22285a99d601431060e5c3
Cr-Commit-Position: refs/heads/master@{#313766}

[please use gerrit instead, 2015-01-30 01:13:51]

Hi,

This appears to have broken browser_trests on xp. Please look into this and let
sheriffs know whether the issue is due to this patch or not. If browser_tests
continue to fail, this patch may be one of the first on the chopping block for a
revert. Thank you!

(view as text)
ChromeRenderProcessHostTest.ProcessPerTab (run #1):
[ RUN      ] ChromeRenderProcessHostTest.ProcessPerTab
[3580:3484:0129/144524:WARNING:data_reduction_proxy_settings.cc(345)] SPDY proxy
OFF at startup
[3580:3920:0129/144524:WARNING:raw_channel.cc(210)] Shutting down RawChannel
with write buffer nonempty
[3580:3920:0129/144524:WARNING:raw_channel.cc(210)] Shutting down RawChannel
with write buffer nonempty
[3580:3920:0129/144525:WARNING:raw_channel.cc(210)] Shutting down RawChannel
with write buffer nonempty
[3580:3920:0129/144525:WARNING:raw_channel.cc(210)] Shutting down RawChannel
with write buffer nonempty
[3580:3484:0129/144525:WARNING:pref_notifier_impl.cc(27)] pref observer found at
shutdown printing.enabled
[3580:3484:0129/144525:WARNING:pref_notifier_impl.cc(27)] pref observer found at
shutdown plugins.allow_outdated
[3580:3484:0129/144525:WARNING:pref_notifier_impl.cc(27)] pref observer found at
shutdown plugins.always_authorize
Backtrace:
	(No symbol) [0xE3DA2000]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x02D8E628+33417800]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x029C9A34+29466196]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x02D8E651+33417841]
	ovly_debug_event [0x00CF6E6A+4596442]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x032B37C3+38812643]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x032B384B+38812779]
	ovly_debug_event [0x00D0B556+4680134]
	ovly_debug_event [0x00D0BD1A+4682122]
	ovly_debug_event [0x00D0BE2B+4682395]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x0343A758+40414072]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x017F1E13+10756659]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01A18BCE+13012974]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01A05F64+12936068]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01A05F89+12936105]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01A07862+12942466]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E5012A+656714]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E504BB+657627]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x017872F7+10319639]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x017CF0B3+10613971]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03B4BED4+47825652]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03A3F2DE+46724862]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03A3F196+46724534]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03A3E4F3+46721299]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x030F0445+36964453]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E0BD2B+377163]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x010981F4+3048980]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01098369+3049353]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01098939+3050841]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01098686+3050150]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E938FD+933149]
	ovly_debug_event [0x00A726B8+1956648]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x030E93B8+36935640]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E39CC6+565478]
	ovly_debug_event [0x00A72720+1956752]
	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x031C90DA+37852410]
	RegisterWaitForInputIdle [0x7C816037+73]

https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20(1)

[weiliangc, 2015-01-30 02:02:44]

On 2015/01/30 01:13:51, Rouslan Solomakhin wrote:
> Hi,
> 
> This appears to have broken browser_trests on xp. Please look into this and
let
> sheriffs know whether the issue is due to this patch or not. If browser_tests
> continue to fail, this patch may be one of the first on the chopping block for
a
> revert. Thank you!
> 
> (view as text)
> ChromeRenderProcessHostTest.ProcessPerTab (run #1):
> [ RUN      ] ChromeRenderProcessHostTest.ProcessPerTab
> [3580:3484:0129/144524:WARNING:data_reduction_proxy_settings.cc(345)] SPDY
proxy
> OFF at startup
> [3580:3920:0129/144524:WARNING:raw_channel.cc(210)] Shutting down RawChannel
> with write buffer nonempty
> [3580:3920:0129/144524:WARNING:raw_channel.cc(210)] Shutting down RawChannel
> with write buffer nonempty
> [3580:3920:0129/144525:WARNING:raw_channel.cc(210)] Shutting down RawChannel
> with write buffer nonempty
> [3580:3920:0129/144525:WARNING:raw_channel.cc(210)] Shutting down RawChannel
> with write buffer nonempty
> [3580:3484:0129/144525:WARNING:pref_notifier_impl.cc(27)] pref observer found
at
> shutdown printing.enabled
> [3580:3484:0129/144525:WARNING:pref_notifier_impl.cc(27)] pref observer found
at
> shutdown plugins.allow_outdated
> [3580:3484:0129/144525:WARNING:pref_notifier_impl.cc(27)] pref observer found
at
> shutdown plugins.always_authorize
> Backtrace:
> 	(No symbol) [0xE3DA2000]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x02D8E628+33417800]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x029C9A34+29466196]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x02D8E651+33417841]
> 	ovly_debug_event [0x00CF6E6A+4596442]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x032B37C3+38812643]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x032B384B+38812779]
> 	ovly_debug_event [0x00D0B556+4680134]
> 	ovly_debug_event [0x00D0BD1A+4682122]
> 	ovly_debug_event [0x00D0BE2B+4682395]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x0343A758+40414072]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x017F1E13+10756659]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01A18BCE+13012974]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01A05F64+12936068]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01A05F89+12936105]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01A07862+12942466]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E5012A+656714]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E504BB+657627]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x017872F7+10319639]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x017CF0B3+10613971]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03B4BED4+47825652]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03A3F2DE+46724862]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03A3F196+46724534]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03A3E4F3+46721299]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x030F0445+36964453]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E0BD2B+377163]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x010981F4+3048980]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01098369+3049353]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01098939+3050841]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01098686+3050150]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E938FD+933149]
> 	ovly_debug_event [0x00A726B8+1956648]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x030E93B8+36935640]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E39CC6+565478]
> 	ovly_debug_event [0x00A72720+1956752]
> 	RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x031C90DA+37852410]
> 	RegisterWaitForInputIdle [0x7C816037+73]
> 
> https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20(1)

CL 781163003 is depended on this.