Automatically redraw the display on resume.

Automatically redraw the display on resume.

- OnscreenContextProvider notifies the CommandBufferImpl when the context has
  been lost.
- This in turn causes DisplayImpl::OutputSurfaceLost() to be called, which
  results in it calling back to OnscreenContextProvider to ask it to create a
  new context when suitable.

This patch is heavily based off https://codereview.chromium.org/1116883002/ by
etiennej@chromium.org.

R=jamesr@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/a3b4ddbca7f09447fc281e21f1503c90c8c8be14

[blundell, 2015-05-04 15:11:48]

blundell@chromium.org changed reviewers:
+ jamesr@chromium.org

[blundell, 2015-05-04 15:11:49]

https://codereview.chromium.org/1123623003/diff/1/services/surfaces/display_i...
File services/surfaces/display_impl.cc (right):

https://codereview.chromium.org/1123623003/diff/1/services/surfaces/display_i...
services/surfaces/display_impl.cc:81: cc_id_,
pending_frame_.Clone().To<scoped_ptr<cc::CompositorFrame>>(),
I'm not an expert here, but this feels to me like instead the DisplayImpl should
be passing some notification up the chain in OutputSurfaceLost() that would
allow the higher-level object to kick off a flow that would result in
DisplayImpl::SubmitFrame() being called if desired. Does that make sense, or
does this approach seem correct?

[blundell, 2015-05-04 15:22:04]

It looks like there might be some problems with the apptests on Android that
I'll have to track down (I'm heading out soon). I'd still be interested to get
your thoughts on the overall approach though.

[blundell, 2015-05-04 15:23:26]

On 2015/05/04 15:22:04, blundell wrote:
> It looks like there might be some problems with the apptests on Android that
> I'll have to track down (I'm heading out soon). I'd still be interested to get
> your thoughts on the overall approach though.

(ppi@ pointed out that the failures on that bot are actually chronic and most
likely unrelated to my CL).

[jamesr, 2015-05-04 18:03:22]

I still feel like there's a piece missing in the flow to trigger a new frame. 
With this patch we're keeping a clone of pending_frame_ in DisplayImpl but the
cc::Surface is *also* keeping a clone of the frame around.  We should not need
to persist two copies of this.  However, I think we can treat fixing that as an
optimization.

lgtm

https://codereview.chromium.org/1123623003/diff/1/services/surfaces/display_i...
File services/surfaces/display_impl.cc (right):

https://codereview.chromium.org/1123623003/diff/1/services/surfaces/display_i...
services/surfaces/display_impl.cc:81: cc_id_,
pending_frame_.Clone().To<scoped_ptr<cc::CompositorFrame>>(),
On 2015/05/04 15:11:49, blundell wrote:
> I'm not an expert here, but this feels to me like instead the DisplayImpl
should
> be passing some notification up the chain in OutputSurfaceLost() that would
> allow the higher-level object to kick off a flow that would result in
> DisplayImpl::SubmitFrame() being called if desired. Does that make sense, or
> does this approach seem correct?

Hmm, do we need new frame data in the recover path or do we just need to tell
something that it needs to draw?  The factory_ is preserving the frame data so
we shouldn't need to submit anything again unless the frame contains resources
that need to be regenerated (which in practice won't normally happen since the
view_manager doesn't submit any resources).

The scheduler should know to draw after loss, but it doesn't know to resize
things

[blundell, 2015-05-05 12:47:19]

Patchset #2 (id:20001) has been deleted

[blundell, 2015-05-05 13:12:47]

Hey James,

Thanks for the suggestion! Is this along the lines of what you were thinking? It
gets the job done :).

[blundell, 2015-05-05 13:13:23]

On 2015/05/05 13:12:47, blundell wrote:
> Hey James,
> 
> Thanks for the suggestion! Is this along the lines of what you were thinking?
It
> gets the job done :).

("this" being PS2)

[jamesr, 2015-05-05 23:23:44]

Hmm, the scheduler should know that the context is recreated.  Does it not?

In the current implementation where we create/destroy the cc::Display on context
creation you could probably have the scheduler SetNeedsDraw() on a display being
added

[blundell, 2015-05-06 14:04:39]

The surfaces_scheduler doesn't get any ping about the context being recreated as
far as I can see. Your suggestion works fine though - PTAL.

[jamesr, 2015-05-06 15:04:18]

lgtm

[blundell, 2015-05-11 15:26:40]

Committed patchset #3 (id:60001) manually as
a3b4ddbca7f09447fc281e21f1503c90c8c8be14 (presubmit successful).