Factor draw functionality out of DefaultPlatformDisplay into FrameGenerator.

Factor draw functionality out of DefaultPlatformDisplay into FrameGenerator.

Introducing a new class FrameGenerator, which is responsible for redrawing the
display by submitting CompositorFrames to the owned DisplayCompositor.
FrameGenerator is owned by PlatformDisplay.

BUG=NONE

Committed: https://crrev.com/6ccecb6657bcc8c670c0685ee36078fa4befa16b
Cr-Commit-Position: refs/heads/master@{#403969}

[mfomitchev, 2016-06-30 19:13:28]

Patchset #2 (id:20001) has been deleted

[mfomitchev, 2016-06-30 20:21:48]

Patchset #2 (id:40001) has been deleted

[mfomitchev, 2016-06-30 20:29:50]

mfomitchev@chromium.org changed reviewers:
+ fsamuel@google.com, sky@chromium.org

[mfomitchev, 2016-06-30 20:29:50]

PTAL. Factoring the draw functionality into a separate class as discussed in
crrev.com/2099893002.

[Fady Samuel, 2016-06-30 20:45:34]

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

[Fady Samuel, 2016-06-30 20:45:35]

Thanks for this refactor Mikhail. This makes it a lot easier to reason about and
unit test. Roughly, FrameGenerator is the thing that talks to Mus Gpu. LGTM

[sky, 2016-06-30 23:40:20]

https://codereview.chromium.org/2112993002/diff/120001/components/mus/ws/fram...
File components/mus/ws/frame_generator_delegate.h (right):

https://codereview.chromium.org/2112993002/diff/120001/components/mus/ws/fram...
components/mus/ws/frame_generator_delegate.h:22: virtual
~FrameGeneratorDelegate(){};
nit: no ';', space after '()' and move to protected section.

https://codereview.chromium.org/2112993002/diff/120001/components/mus/ws/plat...
File components/mus/ws/platform_display.cc (right):

https://codereview.chromium.org/2112993002/diff/120001/components/mus/ws/plat...
components/mus/ws/platform_display.cc:249:
frame_generator_->OnAcceleratedWidgetAvailable(widget);
Could we delay creation of frame_generator until we get a valid widget? That
would seem to avoid allowing FrameGenerator to be in a weird state.

[mfomitchev, 2016-07-06 15:53:09]

https://codereview.chromium.org/2112993002/diff/120001/components/mus/ws/fram...
File components/mus/ws/frame_generator_delegate.h (right):

https://codereview.chromium.org/2112993002/diff/120001/components/mus/ws/fram...
components/mus/ws/frame_generator_delegate.h:22: virtual
~FrameGeneratorDelegate(){};
On 2016/06/30 23:40:20, sky wrote:
> nit: no ';', space after '()' and move to protected section.

Done.

https://codereview.chromium.org/2112993002/diff/120001/components/mus/ws/plat...
File components/mus/ws/platform_display.cc (right):

https://codereview.chromium.org/2112993002/diff/120001/components/mus/ws/plat...
components/mus/ws/platform_display.cc:249:
frame_generator_->OnAcceleratedWidgetAvailable(widget);
On 2016/06/30 23:40:20, sky wrote:
> Could we delay creation of frame_generator until we get a valid widget? That
> would seem to avoid allowing FrameGenerator to be in a weird state.

That would mean that we'd need to add gpu_state and surfaces_state fields to the
PlatformDisplay class (so that we can pass them to FrameGenerator's constructor
in OnAcceleratedWidgetAvailable()), which seems a bit unfortunate. Also, while
we'd remove two checks for display_compositor_ in FrameGenerator, we'd need to
add four checks for frame_generator_ in DefaultPlatformDisplay. So I am not sure
we'd be simplifying the code, but I might be overlooking something, so if you
still think we should do it - let me know, and I will make the change.

[sky, 2016-07-06 19:34:19]

What you have is good, LGTM

[mfomitchev, 2016-07-06 19:49:24]

The CQ bit was checked by mfomitchev@chromium.org

[mfomitchev, 2016-07-06 19:49:25]

The patchset sent to the CQ was uploaded after l-g-t-m from fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/2112993002/#ps180001
(title: "fix namespace comments")

[commit-bot: I haz the power, 2016-07-06 19:50:04]

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-07-06 23:18:57]

Committed patchset #8 (id:180001)

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

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-06 23:20:26]

Description was changed from

==========
Factor draw functionality out of DefaultPlatformDisplay into FrameGenerator.

Introducing a new class FrameGenerator, which is responsible for redrawing the
display by submitting CompositorFrames to the owned DisplayCompositor.
FrameGenerator is owned by PlatformDisplay.

BUG=NONE
==========

to

==========
Factor draw functionality out of DefaultPlatformDisplay into FrameGenerator.

Introducing a new class FrameGenerator, which is responsible for redrawing the
display by submitting CompositorFrames to the owned DisplayCompositor.
FrameGenerator is owned by PlatformDisplay.

BUG=NONE

Committed: https://crrev.com/6ccecb6657bcc8c670c0685ee36078fa4befa16b
Cr-Commit-Position: refs/heads/master@{#403969}
==========

[commit-bot: I haz the power, 2016-07-06 23:20:27]

Patchset 8 (id:??) landed as
https://crrev.com/6ccecb6657bcc8c670c0685ee36078fa4befa16b
Cr-Commit-Position: refs/heads/master@{#403969}