Fall back to software mode if Aura can't create a GL context

Fall back to software mode if Aura can't create a GL context

Wire up fallback from Compositor::CreateOutputSurface to
ContextFactory::CreateOutputSurface, so that we force using
the software compositor after failing to create a GL context
enough times.

BUG=319267
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238382

[ccameron, 2013-11-28 02:45:37]

I was a bit surprised that there was no mechanism for forcing a transition to
software mode (both killing off the processes and disabling acceleration).

Lmk if this seems like a reasonable place to do it, or if this seems to
draconian.

[piman, 2013-12-03 01:06:44]

On 2013/11/28 02:45:37, ccameron1 wrote:
> I was a bit surprised that there was no mechanism for forcing a transition to
> software mode (both killing off the processes and disabling acceleration).
> 
> Lmk if this seems like a reasonable place to do it, or if this seems to
> draconian.

The code that's supposed to handle the fallback is in
GpuProcessTransportFactory::CreateOutputSurface
Do you know why it doesn't trigger?

[ccameron, 2013-12-03 02:33:26]

On 2013/12/03 01:06:44, piman wrote:
> On 2013/11/28 02:45:37, ccameron1 wrote:
> > I was a bit surprised that there was no mechanism for forcing a transition
to
> > software mode (both killing off the processes and disabling acceleration).
> > 
> > Lmk if this seems like a reasonable place to do it, or if this seems to
> > draconian.
> 
> The code that's supposed to handle the fallback is in
> GpuProcessTransportFactory::CreateOutputSurface
> Do you know why it doesn't trigger?

Oh -- the call to GpuProcessTransportFactory::CreateContextCommon creates a
context, but creating a context doesn't mean it's valid -- you have to do a
makeContextCurrent() on it to actually trigger the context creation (via
MaybeInitializeGL et al), and the failure if creating contexts fails.

Updated the patch to just call makeContextCurrent in
GpuProcessTransportFactory::CreateContextCommon.

This may have performance implications for start-up -- is the current
architecture of delayed initialization intentional?

[piman, 2013-12-03 03:14:21]

On 2013/12/03 02:33:26, ccameron1 wrote:
> On 2013/12/03 01:06:44, piman wrote:
> > On 2013/11/28 02:45:37, ccameron1 wrote:
> > > I was a bit surprised that there was no mechanism for forcing a transition
> to
> > > software mode (both killing off the processes and disabling acceleration).
> > > 
> > > Lmk if this seems like a reasonable place to do it, or if this seems to
> > > draconian.
> > 
> > The code that's supposed to handle the fallback is in
> > GpuProcessTransportFactory::CreateOutputSurface
> > Do you know why it doesn't trigger?
> 
> Oh -- the call to GpuProcessTransportFactory::CreateContextCommon creates a
> context, but creating a context doesn't mean it's valid -- you have to do a
> makeContextCurrent() on it to actually trigger the context creation (via
> MaybeInitializeGL et al), and the failure if creating contexts fails.
> 
> Updated the patch to just call makeContextCurrent in
> GpuProcessTransportFactory::CreateContextCommon.
> 
> This may have performance implications for start-up -- is the current
> architecture of delayed initialization intentional?

Oh, I see... we don't know yet at the ContextProvider level, because we haven't
made current.
The main reason we don't makeContextCurrent is that you can only do that on the
thread where you're going to use it. So this would break on Chrome OS, where we
use the thread for the UI compositor.
We could make current in CreateOutputSurface (using
ContextProvider::BindToCurrentThread) if we're not using the thread, and test
the result. But yes, it could have startup perf implications. +jbauman/sievers
in case they have a good insight here.

[ccameron, 2013-12-03 03:16:38]

On 2013/12/03 03:14:21, piman wrote:
> On 2013/12/03 02:33:26, ccameron1 wrote:
> > On 2013/12/03 01:06:44, piman wrote:
> > > On 2013/11/28 02:45:37, ccameron1 wrote:
> > > > I was a bit surprised that there was no mechanism for forcing a
transition
> > to
> > > > software mode (both killing off the processes and disabling
acceleration).
> > > > 
> > > > Lmk if this seems like a reasonable place to do it, or if this seems to
> > > > draconian.
> > > 
> > > The code that's supposed to handle the fallback is in
> > > GpuProcessTransportFactory::CreateOutputSurface
> > > Do you know why it doesn't trigger?
> > 
> > Oh -- the call to GpuProcessTransportFactory::CreateContextCommon creates a
> > context, but creating a context doesn't mean it's valid -- you have to do a
> > makeContextCurrent() on it to actually trigger the context creation (via
> > MaybeInitializeGL et al), and the failure if creating contexts fails.
> > 
> > Updated the patch to just call makeContextCurrent in
> > GpuProcessTransportFactory::CreateContextCommon.
> > 
> > This may have performance implications for start-up -- is the current
> > architecture of delayed initialization intentional?
> 
> Oh, I see... we don't know yet at the ContextProvider level, because we
haven't
> made current.
> The main reason we don't makeContextCurrent is that you can only do that on
the
> thread where you're going to use it. So this would break on Chrome OS, where
we
> use the thread for the UI compositor.
> We could make current in CreateOutputSurface (using
> ContextProvider::BindToCurrentThread) if we're not using the thread, and test
> the result. But yes, it could have startup perf implications. +jbauman/sievers
> in case they have a good insight here.

Yes, I noticed that there were some thread checks that blew up if I wasn't
careful about where to put that.

Btw, WRT the performance implications, it may be that the approach in PS1
(retroactively murdering the GPU process) may be the way to go -- it assumes
that everything works the way that it usually does (common case, etc), runs
forward, and snaps things back (albeit a bit violently) if we find that things
aren't working as expected).

[jbauman, 2013-12-03 03:19:47]

On 2013/12/03 02:33:26, ccameron1 wrote:
> On 2013/12/03 01:06:44, piman wrote:
> > On 2013/11/28 02:45:37, ccameron1 wrote:
> > > I was a bit surprised that there was no mechanism for forcing a transition
> to
> > > software mode (both killing off the processes and disabling acceleration).
> > > 
> > > Lmk if this seems like a reasonable place to do it, or if this seems to
> > > draconian.
> > 
> > The code that's supposed to handle the fallback is in
> > GpuProcessTransportFactory::CreateOutputSurface
> > Do you know why it doesn't trigger?
> 
> Oh -- the call to GpuProcessTransportFactory::CreateContextCommon creates a
> context, but creating a context doesn't mean it's valid -- you have to do a
> makeContextCurrent() on it to actually trigger the context creation (via
> MaybeInitializeGL et al), and the failure if creating contexts fails.
> 
> Updated the patch to just call makeContextCurrent in
> GpuProcessTransportFactory::CreateContextCommon.
> 
> This may have performance implications for start-up -- is the current
> architecture of delayed initialization intentional?

The way we handle this in the renderer is that after the compositor fails to use
an output surface a few times, then it requests the creation of a "fallback"
output surface that would be created in software. In this case, I think that
would involve plumbing the fallback parameter of
ui::Compositor::CreateOutputSurface into
GpuProcessTransportFactory::CreateOutputSurface and using that to fall back to
software.

[ccameron, 2013-12-03 03:45:41]

On 2013/12/03 03:19:47, jbauman wrote:
> On 2013/12/03 02:33:26, ccameron1 wrote:
> > On 2013/12/03 01:06:44, piman wrote:
> > > On 2013/11/28 02:45:37, ccameron1 wrote:
> > > > I was a bit surprised that there was no mechanism for forcing a
transition
> > to
> > > > software mode (both killing off the processes and disabling
acceleration).
> > > > 
> > > > Lmk if this seems like a reasonable place to do it, or if this seems to
> > > > draconian.
> > > 
> > > The code that's supposed to handle the fallback is in
> > > GpuProcessTransportFactory::CreateOutputSurface
> > > Do you know why it doesn't trigger?
> > 
> > Oh -- the call to GpuProcessTransportFactory::CreateContextCommon creates a
> > context, but creating a context doesn't mean it's valid -- you have to do a
> > makeContextCurrent() on it to actually trigger the context creation (via
> > MaybeInitializeGL et al), and the failure if creating contexts fails.
> > 
> > Updated the patch to just call makeContextCurrent in
> > GpuProcessTransportFactory::CreateContextCommon.
> > 
> > This may have performance implications for start-up -- is the current
> > architecture of delayed initialization intentional?
> 
> The way we handle this in the renderer is that after the compositor fails to
use
> an output surface a few times, then it requests the creation of a "fallback"
> output surface that would be created in software. In this case, I think that
> would involve plumbing the fallback parameter of
> ui::Compositor::CreateOutputSurface into
> GpuProcessTransportFactory::CreateOutputSurface and using that to fall back to
> software.

Oh. That's very reasonable... updated patch.

[piman, 2013-12-03 04:20:45]

OK, LGTM. Not sure if there's some edge cases on Chrome OS where we'll try to
fallback too early (which we can't), but I guess crash reports will tell us

[jbauman, 2013-12-03 05:42:03]

lgtm

[ccameron, 2013-12-03 10:27:58]

Thanks!

[commit-bot: I haz the power, 2013-12-03 10:39:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ccameron@chromium.org/93593002/40001

[commit-bot: I haz the power, 2013-12-03 13:00:11]

Change committed as 238382