Use a cc::Display for layout tests.

Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
Committed: https://crrev.com/a40dd4485a253e119162b5d2fb733ef8b7c53b56
Cr-Commit-Position: refs/heads/master@{#402360}

[danakj, 2016-06-18 01:12:46]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

R=piman@chromium.org, sievers
BUG=311404
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[danakj, 2016-06-18 01:13:04]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel
==========

[danakj, 2016-06-18 01:13:07]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[danakj, 2016-06-18 01:13:13]

The CQ bit was unchecked by danakj@chromium.org

[danakj, 2016-06-18 01:13:31]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel;tryserver.blink:android_blink_rel
==========

[danakj, 2016-06-18 01:13:35]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-18 01:13:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2075343003/1

[danakj, 2016-06-18 01:14:14]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel;tryserver.blink:android_blink_rel
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, etc.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel;tryserver.blink:android_blink_rel
==========

[danakj, 2016-06-18 01:14:43]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, etc.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel;tryserver.blink:android_blink_rel
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel;tryserver.blink:android_blink_rel
==========

[commit-bot: I haz the power, 2016-06-18 01:14:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-18 01:14:58]

Dry run: Try jobs failed on following builders:
  android_blink_rel on tryserver.blink (JOB_FAILED, no build URL)

[danakj, 2016-06-18 01:15:11]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel;tryserver.blink:android_blink_rel
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel
==========

[danakj, 2016-06-18 01:16:19]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel
==========

[danakj, 2016-06-18 01:23:17]

I thought about moving all the context creation into the LayerTestDepsImpl. Then
the early out for tests could be at the top of CreateCompositorOutputSurface,
and wouldnt have to deal with the ifs about software etc, hoping we ignore them.

But there's a bunch of logic about async worker contexts that I'd have to
duplicate for making the worker and sharing the context or not. So I didn't.
WDYT?

[piman, 2016-06-20 16:45:07]

lgtm

https://codereview.chromium.org/2075343003/diff/1/content/test/layouttest_sup...
File content/test/layouttest_support.cc (right):

https://codereview.chromium.org/2075343003/diff/1/content/test/layouttest_sup...
content/test/layouttest_support.cc:218: */
nit: remove

[danakj, 2016-06-20 23:21:00]

There's some strange/intruiging tiny pixel differences compared to before, such
as:
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...

Little rounding things, which is fine. But why would it be at all different..
O_o

https://codereview.chromium.org/2075343003/diff/1/content/test/layouttest_sup...
File content/test/layouttest_support.cc (right):

https://codereview.chromium.org/2075343003/diff/1/content/test/layouttest_sup...
content/test/layouttest_support.cc:218: */
On 2016/06/20 16:45:07, piman wrote:
> nit: remove

Done.

[enne (OOO), 2016-06-20 23:27:58]

On 2016/06/20 at 23:21:00, danakj wrote:
> There's some strange/intruiging tiny pixel differences compared to before,
such as:
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...
> 
> Little rounding things, which is fine. But why would it be at all different..
O_o

The only thing I can think is that SurfaceAggregator is now involved where it
wasn't before.

[danakj, 2016-06-21 00:00:21]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-21 00:00:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2075343003/10001

[commit-bot: I haz the power, 2016-06-21 00:39:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-21 00:39:26]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[danakj, 2016-06-21 01:11:04]

Aha. WebView surface expansion testing leaked into layouttests, that's the pixel
differences. I've made us stop doing that in layout tests in the last patch set.

There's still timeouts with threaded layout tests (may they go away) to figure
out.

[danakj, 2016-06-21 01:11:14]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-21 01:11:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2075343003/30001

[danakj, 2016-06-21 01:11:56]

I should point out we're still using default RendererSettings in layout tests
instead of plumbing them from the renderer compositor (if that would even make
sense! they should match the ui::Compositor instead). So.. idk what to do with
that. Leaving it as is for now.

[commit-bot: I haz the power, 2016-06-21 01:56:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-21 01:56:07]

Dry run: Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/8...)

[danakj, 2016-06-21 21:47:19]

2 other pixel failures were:
css3/filters/backdrop-filter-rendering.html
css3/filters/backdrop-filter-rendering-no-background.html

I think this points to bug 612238 where backdrop filters are just not working.

They work with mailbox output surface because it binds a texture as the default
framebuffer, so reading GL_RGBA works. But with a cc::Display, drawing to an
(offscreen) context's default framebuffer, there's no alpha channel, so it
fails.

I'm addressing this in another CL.

[danakj, 2016-06-23 01:11:04]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-23 01:11:36]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2075343003/70001

[danakj, 2016-06-23 01:42:12]

The latest patchset changes PixelTestDelegatedOutputSurface to use a
DelayBasedBeginFrameSource instead of a BackToBackBeginFrameSource. This fixes
most (all i think?) of the timeouts in virtual/threaded/ since those tests just
spam the compositor with a BackToBack BFS, which keeps the system too busy to
get through all the IPC back and forth to end the test cleanly. DelayBased BFS
gives more idle time for the tests to end. (heh)

This whole virtual test suite is flaky as heck! It takes many retries to get
thru all the tests.

There remain at least 3 tests with pixel differences, and they are ref-tests, so
that's extra sad cuz I can't just rebaseline them:
virtual/threaded/animations/composited-animations-rotate-zero-degrees.htmlpass
virtual/threaded/animations/composited-animations-simple.html
virtual/threaded/animations/composited-animations-translate-rotate-scale.html

The deltas are all a few pixel differences under transforms. I will have to
investigate more.

There is also a crash appearing that looks like:
Received signal 11 SEGV_MAPERR 000000000220
#0 0x7f93ff99a947 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#1 0x7f93f5cb4330 <unknown>
#2 0x7f93fd0b5399 cc::Scheduler::DidSwapBuffersComplete()
#3 0x7f93fd13182c cc::ProxyImpl::DidSwapBuffersCompleteOnImplThread()
#4 0x7f93fb21d2f1 cc::Surface::~Surface()
#5 0x7f93fb22b747 cc::SurfaceManager::GarbageCollectSurfaces()
#6 0x7f93fb22b22b cc::SurfaceManager::Destroy()
#7 0x7f93fb2289d8 cc::SurfaceFactory::Destroy()
#8 0x0000004bbfd0 cc::PixelTestDelegatingOutputSurface::DetachFromClient()
#9 0x7f93fd0f5a67 cc::LayerTreeHostImpl::~LayerTreeHostImpl()
#10 0x7f93fd0f6119 cc::LayerTreeHostImpl::~LayerTreeHostImpl()
#11 0x7f93fd12f7e5 cc::ProxyImpl::~ProxyImpl()
#12 0x7f93fd12f999 cc::ProxyImpl::~ProxyImpl()

This is us shutting down the LTHI. During shutdown in layout tests, there is a
SurfaceFactory, etc. The surface for the renderer may have been queued into the
Display, but not drawn yet. Since these are threaded tests, and they just go on
their own constantly.

In that case, the Surface has a draw_callback_ that it will call once drawn. It
also calls it from its destructor:
    draw_callback_.Run(SurfaceDrawStatus::DRAW_SKIPPED);

That goes to the compositor which is mid shutdown and the scheduler is already
destroyed. This is fixed by invalidating the weakptr in
PixelTestDelegatingOutputSurface sooner, so that the DrawCallback doesn't do
anything during shutdown. I'll upload another patchset for that.

[danakj, 2016-06-23 01:42:29]

danakj@chromium.org changed reviewers:
+ enne@chromium.org

[danakj, 2016-06-23 01:48:16]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-23 01:48:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2075343003/90001

[danakj, 2016-06-23 01:49:42]

These are the remaining pixel failures (again, everything is flaky tho yay):

https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...

There's 2 more than I didn't get locally. I did confirm that the difference
between the first SwapBuffers and the second in the LTHI is the same before and
after this CL now, with the DelayBasedBFS.

[commit-bot: I haz the power, 2016-06-23 02:51:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-23 02:51:46]

Dry run: Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/8...)

[danakj, 2016-06-23 20:59:46]

OK I undid my change to the weakptrs in PixelTestDelegatingOutputSurface, and
instead I release the OutputSurface off LTHI when ProxyImpl goes to shutdown, so
that it stops calling to its client during destruction of things.

Back to the pixel failure mines.

[danakj, 2016-06-23 21:00:53]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-23 21:01:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2075343003/110001

[danakj, 2016-06-23 21:13:05]

The pixel differences with
virtual/threaded/animations/rotate-transform-equivalent.html reproduce if I use
a default RendererSettings instead of the one given to LTH, so I guess we do
need to plumb those settings to the Display also!

[commit-bot: I haz the power, 2016-06-23 21:45:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-23 21:45:34]

Dry run: Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/8...)

[danakj, 2016-06-23 22:04:26]

Confirmed that getting the settings we give to LayerTreeHost(Impl) to the
Display does in fact fix the remainder of the tests for me locally.

PTAL

[danakj, 2016-06-23 22:04:35]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-23 22:04:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2075343003/130001

[danakj, 2016-06-23 22:18:49]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-23 22:19:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2075343003/150001

[commit-bot: I haz the power, 2016-06-23 22:54:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-23 22:54:40]

Dry run: Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/68015)

[enne (OOO), 2016-06-23 23:17:55]

lgtm

https://codereview.chromium.org/2075343003/diff/130001/cc/trees/proxy_impl.cc
File cc/trees/proxy_impl.cc (right):

https://codereview.chromium.org/2075343003/diff/130001/cc/trees/proxy_impl.cc...
cc/trees/proxy_impl.cc:120: layer_tree_host_impl_->ReleaseOutputSurface();
Please do this in single thread proxy too for consistency, if you could?

https://codereview.chromium.org/2075343003/diff/150001/cc/test/pixel_test_del...
File cc/test/pixel_test_delegating_output_surface.h (right):

https://codereview.chromium.org/2075343003/diff/150001/cc/test/pixel_test_del...
cc/test/pixel_test_delegating_output_surface.h:69:
scoped_refptr<ContextProvider> display_context_provider_;
I hate temporary members like this, but I don't see how to do it differently.

[jbauman, 2016-06-23 23:19:09]

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

[jbauman, 2016-06-23 23:19:10]

lgtm

[danakj, 2016-06-23 23:21:04]

https://codereview.chromium.org/2075343003/diff/130001/cc/trees/proxy_impl.cc
File cc/trees/proxy_impl.cc (right):

https://codereview.chromium.org/2075343003/diff/130001/cc/trees/proxy_impl.cc...
cc/trees/proxy_impl.cc:120: layer_tree_host_impl_->ReleaseOutputSurface();
On 2016/06/23 23:17:55, enne wrote:
> Please do this in single thread proxy too for consistency, if you could?

Done.

https://codereview.chromium.org/2075343003/diff/150001/cc/test/pixel_test_del...
File cc/test/pixel_test_delegating_output_surface.h (right):

https://codereview.chromium.org/2075343003/diff/150001/cc/test/pixel_test_del...
cc/test/pixel_test_delegating_output_surface.h:69:
scoped_refptr<ContextProvider> display_context_provider_;
On 2016/06/23 23:17:55, enne wrote:
> I hate temporary members like this, but I don't see how to do it differently.

I think we can maybe do this once OutputSurface and CompositorFrameSink are
different, then maybe OutputSurface can take this at BindToClient time.

[danakj, 2016-06-23 23:28:00]

Blah, linux works but 2 more failures on mac:

virtual/threaded/animations/transition-and-animation-1.html
virtual/threaded/animations/transition-and-animation-2.html

https://storage.googleapis.com/chromium-layout-test-archives/mac_blink_rel/68...

They aren't pixel tests this time, so its extra fun/mysterious. And only on mac.

Also some webview tests are failing somehow. So +bo FYI cuz apparently I'm doing
something to webview.

[danakj, 2016-06-23 23:28:03]

Blah, linux works but 2 more failures on mac:

virtual/threaded/animations/transition-and-animation-1.html
virtual/threaded/animations/transition-and-animation-2.html

https://storage.googleapis.com/chromium-layout-test-archives/mac_blink_rel/68...

They aren't pixel tests this time, so its extra fun/mysterious. And only on mac.

Also some webview tests are failing somehow. So +bo FYI cuz apparently I'm doing
something to webview.

[danakj, 2016-06-24 18:33:10]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-06-24 18:33:10]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org,
enne@chromium.org, jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2075343003/#ps210001
(title: "layouttests-display: rebase")

[commit-bot: I haz the power, 2016-06-24 18:33:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-24 19:46:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-24 19:46:31]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[danakj, 2016-06-24 23:23:59]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel;tryserver.blink:mac_blink_rel
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
==========

[danakj, 2016-06-24 23:24:18]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-06-24 23:24:19]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org,
jbauman@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2075343003/#ps230001
(title: "layouttests-display: webview")

[commit-bot: I haz the power, 2016-06-24 23:25:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danakj, 2016-06-24 23:31:28]

Oh, bo, there's a change in
content/renderer/android/synchronous_compositor_proxy.cc now.

The output surface is detatched/destroyed before the proxy now, so it DCHECKs
it's null in the destructor instead of setting it to null (for a 2nd time),
which was DCHECKing.

[danakj, 2016-06-24 23:31:44]

danakj@chromium.org changed reviewers:
+ boliu@chromium.org

[danakj, 2016-06-24 23:41:56]

The CQ bit was unchecked by danakj@chromium.org

[boliu, 2016-06-24 23:44:45]

lgtm

https://codereview.chromium.org/2075343003/diff/230001/content/renderer/andro...
File content/renderer/android/synchronous_compositor_proxy.cc (right):

https://codereview.chromium.org/2075343003/diff/230001/content/renderer/andro...
content/renderer/android/synchronous_compositor_proxy.cc:58:
DCHECK_NE(output_surface_, output_surface);
alternative is to remove this DCHECK, since it's clearly wrong given this is
called from the destructor also

but if cc has stronger guarantees now about OS going away first, then this is
good too

[danakj, 2016-06-25 00:28:54]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[danakj, 2016-06-25 00:29:03]

OK last patch set has a DCHECK in ~LTHI that the output surface is gone already
also.

And I had to change a lot of cc unittests to release the output surface before
destroying the LTHI. And it exposed some dangling layer pointers in the
occlusion tracker tests.

[danakj, 2016-06-25 00:29:08]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-06-25 00:29:09]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org,
boliu@chromium.org, jbauman@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2075343003/#ps250001
(title: "layouttests-display: dcheckdestructor")

[commit-bot: I haz the power, 2016-06-25 00:29:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-25 01:55:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-25 01:55:18]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[danakj, 2016-06-27 17:10:45]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2016-06-27 17:11:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danakj, 2016-06-27 17:11:47]

Patchset #14 (id:250001) has been deleted

[danakj, 2016-06-27 17:12:33]

The CQ bit was unchecked by danakj@chromium.org

[danakj, 2016-06-27 17:12:40]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2016-06-27 17:13:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-27 18:31:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-27 18:31:15]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[danakj, 2016-06-27 18:53:05]

The CQ bit was checked by danakj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 18:53:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danakj, 2016-06-27 19:59:13]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-06-27 19:59:13]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org,
boliu@chromium.org, jbauman@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2075343003/#ps270001
(title: "layouttests-display2: moreneverfix")

[commit-bot: I haz the power, 2016-06-27 19:59:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danakj, 2016-06-27 20:00:16]

On 2016/06/25 00:29:03, danakj wrote:
> OK last patch set has a DCHECK in ~LTHI that the output surface is gone
already
> also.

This requires a ton of cc test changes. I'm going to do it separately.

> And I had to change a lot of cc unittests to release the output surface before
> destroying the LTHI. And it exposed some dangling layer pointers in the
> occlusion tracker tests.

[commit-bot: I haz the power, 2016-06-27 21:58:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-27 21:58:19]

Failed to apply patch for content/renderer/gpu/render_widget_compositor.cc:
While running git apply --index -3 -p1;
  error: patch failed: content/renderer/gpu/render_widget_compositor.cc:449
  Falling back to three-way merge...
  Applied patch to 'content/renderer/gpu/render_widget_compositor.cc' with
conflicts.
  U content/renderer/gpu/render_widget_compositor.cc

Patch:       content/renderer/gpu/render_widget_compositor.cc
Index: content/renderer/gpu/render_widget_compositor.cc
diff --git a/content/renderer/gpu/render_widget_compositor.cc
b/content/renderer/gpu/render_widget_compositor.cc
index
99649f658b8a1bb2fda03952221c8727523c7f4a..0c49515356840a94b8656cfa4e86cbe285a58965
100644
--- a/content/renderer/gpu/render_widget_compositor.cc
+++ b/content/renderer/gpu/render_widget_compositor.cc
@@ -221,6 +221,7 @@ RenderWidgetCompositor::RenderWidgetCompositor(
     : num_failed_recreate_attempts_(0),
       delegate_(delegate),
       compositor_deps_(compositor_deps),
+      threaded_(!!compositor_deps_->GetCompositorImplThreadTaskRunner()),
       never_visible_(false),
       layout_and_paint_async_callback_(nullptr),
       remote_proto_channel_receiver_(nullptr),
@@ -228,16 +229,54 @@ RenderWidgetCompositor::RenderWidgetCompositor(
 
 void RenderWidgetCompositor::Initialize(float device_scale_factor) {
   base::CommandLine* cmd = base::CommandLine::ForCurrentProcess();
+  cc::LayerTreeSettings settings =
+      GenerateLayerTreeSettings(*cmd, compositor_deps_, device_scale_factor);
+  cc::LayerTreeHost::InitParams params;
+  params.client = this;
+  params.shared_bitmap_manager = compositor_deps_->GetSharedBitmapManager();
+  params.gpu_memory_buffer_manager =
+      compositor_deps_->GetGpuMemoryBufferManager();
+  params.settings = &settings;
+  params.task_graph_runner = compositor_deps_->GetTaskGraphRunner();
+  params.main_task_runner =
+      compositor_deps_->GetCompositorMainThreadTaskRunner();
+  if (settings.use_external_begin_frame_source) {
+    params.external_begin_frame_source =
+        delegate_->CreateExternalBeginFrameSource();
+  }
+  params.animation_host = cc::AnimationHost::CreateMainInstance();
 
+  if (cmd->HasSwitch(switches::kUseRemoteCompositing)) {
+    DCHECK(!threaded_);
+    params.image_serialization_processor =
+        compositor_deps_->GetImageSerializationProcessor();
+    layer_tree_host_ = cc::LayerTreeHost::CreateRemoteServer(this, &params);
+  } else if (!threaded_) {
+    // Single-threaded layout tests.
+    layer_tree_host_ = cc::LayerTreeHost::CreateSingleThreaded(this, &params);
+  } else {
+    layer_tree_host_ = cc::LayerTreeHost::CreateThreaded(
+        compositor_deps_->GetCompositorImplThreadTaskRunner(), &params);
+  }
+  DCHECK(layer_tree_host_);
+}
+
+RenderWidgetCompositor::~RenderWidgetCompositor() = default;
+
+// static
+cc::LayerTreeSettings RenderWidgetCompositor::GenerateLayerTreeSettings(
+    const base::CommandLine& cmd,
+    CompositorDependencies* compositor_deps,
+    float device_scale_factor) {
   cc::LayerTreeSettings settings;
 
   // For web contents, layer transforms should scale up the contents of layers
   // to keep content always crisp when possible.
   settings.layer_transforms_should_scale_layer_contents = true;
 
-  if (cmd->HasSwitch(switches::kDisableGpuVsync)) {
+  if (cmd.HasSwitch(switches::kDisableGpuVsync)) {
     std::string display_vsync_string =
-        cmd->GetSwitchValueASCII(switches::kDisableGpuVsync);
+        cmd.GetSwitchValueASCII(switches::kDisableGpuVsync);
     if (display_vsync_string == "gpu") {
       settings.renderer_settings.disable_display_vsync = true;
     } else if (display_vsync_string == "beginframe") {
@@ -248,37 +287,33 @@ void RenderWidgetCompositor::Initialize(float
device_scale_factor) {
     }
   }
   settings.main_frame_before_activation_enabled =
-      cmd->HasSwitch(cc::switches::kEnableMainFrameBeforeActivation);
+      cmd.HasSwitch(cc::switches::kEnableMainFrameBeforeActivation);
 
+  // TODO(danakj): This should not be a setting O_O; it should change when the
+  // device scale factor on LayerTreeHost changes.
   settings.default_tile_size = CalculateDefaultTileSize(device_scale_factor);
-  if (cmd->HasSwitch(switches::kDefaultTileWidth)) {
+  if (cmd.HasSwitch(switches::kDefaultTileWidth)) {
     int tile_width = 0;
-    GetSwitchValueAsInt(*cmd,
-                        switches::kDefaultTileWidth,
-                        1,
-                        std::numeric_limits<int>::max(),
-                        &tile_width);
+    GetSwitchValueAsInt(cmd, switches::kDefaultTileWidth, 1,
+                        std::numeric_limits<int>::max(), &tile_width);
     settings.default_tile_size.set_width(tile_width);
   }
-  if (cmd->HasSwitch(switches::kDefaultTileHeight)) {
+  if (cmd.HasSwitch(switches::kDefaultTileHeight)) {
     int tile_height = 0;
-    GetSwitchValueAsInt(*cmd,
-                        switches::kDefaultTileHeight,
-                        1,
-                        std::numeric_limits<int>::max(),
-                        &tile_height);
+    GetSwitchValueAsInt(cmd, switches::kDefaultTileHeight, 1,
+                        std::numeric_limits<int>::max(), &tile_height);
     settings.default_tile_size.set_height(tile_height);
   }
 
   int max_untiled_layer_width = settings.max_untiled_layer_size.width();
-  if (cmd->HasSwitch(switches::kMaxUntiledLayerWidth)) {
-    GetSwitchValueAsInt(*cmd, switches::kMaxUntiledLayerWidth, 1,
+  if (cmd.HasSwitch(switches::kMaxUntiledLayerWidth)) {
+    GetSwitchValueAsInt(cmd, switches::kMaxUntiledLayerWidth, 1,
                         std::numeric_limits<int>::max(),
                         &max_untiled_layer_width);
   }
   int max_untiled_layer_height = settings.max_untiled_layer_size.height();
-  if (cmd->HasSwitch(switches::kMaxUntiledLayerHeight)) {
-    GetSwitchValueAsInt(*cmd, switches::kMaxUntiledLayerHeight, 1,
+  if (cmd.HasSwitch(switches::kMaxUntiledLayerHeight)) {
+    GetSwitchValueAsInt(cmd, switches::kMaxUntiledLayerHeight, 1,
                         std::numeric_limits<int>::max(),
                         &max_untiled_layer_height);
   }
@@ -287,50 +322,50 @@ void RenderWidgetCompositor::Initialize(float
device_scale_factor) {
                                            max_untiled_layer_height);
 
   settings.gpu_rasterization_msaa_sample_count =
-      compositor_deps_->GetGpuRasterizationMSAASampleCount();
+      compositor_deps->GetGpuRasterizationMSAASampleCount();
   settings.gpu_rasterization_forced =
-      compositor_deps_->IsGpuRasterizationForced();
+      compositor_deps->IsGpuRasterizationForced();
   settings.gpu_rasterization_enabled =
-      compositor_deps_->IsGpuRasterizationEnabled();
+      compositor_deps->IsGpuRasterizationEnabled();
   settings.async_worker_context_enabled =
-      compositor_deps_->IsAsyncWorkerContextEnabled();
+      compositor_deps->IsAsyncWorkerContextEnabled();
 
-  settings.can_use_lcd_text = compositor_deps_->IsLcdTextEnabled();
+  settings.can_use_lcd_text = compositor_deps->IsLcdTextEnabled();
   settings.use_distance_field_text =
-      compositor_deps_->IsDistanceFieldTextEnabled();
-  settings.use_zero_copy = compositor_deps_->IsZeroCopyEnabled();
-  settings.use_partial_raster = compositor_deps_->IsPartialRasterEnabled();
+      compositor_deps->IsDistanceFieldTextEnabled();
+  settings.use_zero_copy = compositor_deps->IsZeroCopyEnabled();
+  settings.use_partial_raster = compositor_deps->IsPartialRasterEnabled();
   settings.enable_elastic_overscroll =
-      compositor_deps_->IsElasticOverscrollEnabled();
+      compositor_deps->IsElasticOverscrollEnabled();
   settings.renderer_settings.use_gpu_memory_buffer_resources =
-      compositor_deps_->IsGpuMemoryBufferCompositorResourcesEnabled();
+      compositor_deps->IsGpuMemoryBufferCompositorResourcesEnabled();
   settings.use_image_texture_targets =
-      compositor_deps_->GetImageTextureTargets();
+      compositor_deps->GetImageTextureTargets();
   settings.image_decode_tasks_enabled =
-      compositor_deps_->AreImageDecodeTasksEnabled();
-
-  if (cmd->HasSwitch(cc::switches::kTopControlsShowThreshold)) {
-      std::string top_threshold_str =
-          cmd->GetSwitchValueASCII(cc::switches::kTopControlsShowThreshold);
-      double show_threshold;
-      if (base::StringToDouble(top_threshold_str, &show_threshold) &&
-          show_threshold >= 0.f && show_threshold <= 1.f)
-        settings.top_controls_show_threshold = show_threshold;
+      compositor_deps->AreImageDecodeTasksEnabled();
+
+  if (cmd.HasSwitch(cc::switches::kTopControlsShowThreshold)) {
+    std::string top_threshold_str =
+        cmd.GetSwitchValueASCII(cc::switches::kTopControlsShowThreshold);
+    double show_threshold;
+    if (base::StringToDouble(top_threshold_str, &show_threshold) &&
+        show_threshold >= 0.f && show_threshold <= 1.f)
+      settings.top_controls_show_threshold = show_threshold;
   }
 
-  if (cmd->HasSwitch(cc::switches::kTopControlsHideThreshold)) {
-      std::string top_threshold_str =
-          cmd->GetSwitchValueASCII(cc::switches::kTopControlsHideThreshold);
-      double hide_threshold;
-      if (base::StringToDouble(top_threshold_str, &hide_threshold) &&
-          hide_threshold >= 0.f && hide_threshold <= 1.f)
-        settings.top_controls_hide_threshold = hide_threshold;
+  if (cmd.HasSwitch(cc::switches::kTopControlsHideThreshold)) {
+    std::string top_threshold_str =
+        cmd.GetSwitchValueASCII(cc::switches::kTopControlsHideThreshold);
+    double hide_threshold;
+    if (base::StringToDouble(top_threshold_str, &hide_threshold) &&
+        hide_threshold >= 0.f && hide_threshold <= 1.f)
+      settings.top_controls_hide_threshold = hide_threshold;
   }
 
-  settings.use_layer_lists = cmd->HasSwitch(cc::switches::kEnableLayerLists);
+  settings.use_layer_lists = cmd.HasSwitch(cc::switches::kEnableLayerLists);
 
   settings.renderer_settings.allow_a…
(message too large)

[danakj, 2016-06-27 22:16:08]

The CQ bit was checked by danakj@chromium.org

[danakj, 2016-06-27 22:16:09]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org,
boliu@chromium.org, jbauman@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2075343003/#ps290001
(title: "layouttests-display2: rebase")

[danakj, 2016-06-27 22:16:15]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
==========

[commit-bot: I haz the power, 2016-06-27 22:16:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-28 01:14:23]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
==========

[commit-bot: I haz the power, 2016-06-28 01:14:25]

Committed patchset #15 (id:290001)

[commit-bot: I haz the power, 2016-06-28 01:17:25]

Description was changed from

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404
==========

to

==========
Use a cc::Display for layout tests.

This gives a delegating OutputSurface to LayerTreeHost for layout tests,
which delegates to an in-process cc::Display instance. It does so using
the same code we now use for cc pixel tests, which is the
PixelTestDelegatingOutputSurface class.

This makes LayoutTests use the same code paths in the renderer
compositor that we ship to users! Hooray paying down 3-year-old
technical debt. After this, LayerTreeHostImpl will no longer need
to use a DirectRenderer ever (modulo WebView).

I wanted to keep the Display using a separate ContextProvider from cc,
to test texture transport mostly as it exists between the renderer and
the out-of-process Display that we ship to users. So the output surface
must take a ContextProvider from its creator to allow cc pixel tests
to use in-process GL, and layout tests to use out-of-process GL. So
the LayoutTestDependenciesImpl creates a ContextProviderCommandBuffer
for the cc::Display to use, keeping the smallest code footprint
possible inside RenderThreadImpl.

TODO(danakj): Delete GLFrameData, MailboxOutputSurface, mailbox support
in GLRenderer, etc.

R=piman@chromium.org, sievers
BUG=311404

Committed: https://crrev.com/a40dd4485a253e119162b5d2fb733ef8b7c53b56
Cr-Commit-Position: refs/heads/master@{#402360}
==========

[commit-bot: I haz the power, 2016-06-28 01:17:27]

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