Fix crash in GotAcceleratedIOSurfaceFrame

Fix crash in GotAcceleratedIOSurfaceFrame

The root cause here is that it was assumed that creation of a
CompositingIOSurfaceMac and CompositingIOSurfaceContext
cannot not fail when, in fact, it can. This resulted in a NULL
reference later on.

Fix part of this by making CompositingIOSurfaceLayer's init function
fail if the CompositingIOSurfaceContext fails.

Fix the remaining part of this by removing the NULL references from
BrowserCompositorViewMacInternal::GotAcceleratedIOSurfaceFrame,
and make the function call AcceleratedLayerHitError if any errors are
encountered, which will cause the layer to re-attempt the frame with
a new context.

Also, split AcceleratedLayerDidDrawFrame into two functions,
AcceleratedLayerDidDrawFrame and AcceleratedLayerHitError, rather
than parameterizing success using a bool, since that naming was
deceptive.

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

[ccameron, 2014-08-07 21:14:27]

Somewhere along the line here, I assumed that the IOSurface/Context creation
functions could not fail (well, if they failed it would OOM or something equally
destructive). Not so.

Simple fix, and some refactoring, to make things easier to read.

[Ken Russell (switch to Gerrit), 2014-08-07 22:11:44]

LGTM

https://codereview.chromium.org/447113004/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/compositing_iosurface_layer_mac.mm (right):

https://codereview.chromium.org/447113004/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/compositing_iosurface_layer_mac.mm:188: LOG(ERROR)
<< "Failed create CompositingIOSurfaceContext";
I wonder whether here and throughout these classes VLOG should be used instead
of LOG to reduce the amount of output emitted by default, though I realize these
are only displayed in fairly serious error conditions.

[ccameron, 2014-08-07 22:28:25]

Thanks!

https://codereview.chromium.org/447113004/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/compositing_iosurface_layer_mac.mm (right):

https://codereview.chromium.org/447113004/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/compositing_iosurface_layer_mac.mm:188: LOG(ERROR)
<< "Failed create CompositingIOSurfaceContext";
On 2014/08/07 22:11:44, Ken Russell wrote:
> I wonder whether here and throughout these classes VLOG should be used instead
> of LOG to reduce the amount of output emitted by default, though I realize
these
> are only displayed in fairly serious error conditions.

If anything, I'd like to make these errors more visible -- I'd like them to
appear in about:gpu, if possible -- is there a way to do that in the browser
process?

[Ken Russell (switch to Gerrit), 2014-08-07 22:34:01]

On 2014/08/07 22:28:25, ccameron1 wrote:
> Thanks!
> 
>
https://codereview.chromium.org/447113004/diff/1/content/browser/renderer_hos...
> File content/browser/renderer_host/compositing_iosurface_layer_mac.mm (right):
> 
>
https://codereview.chromium.org/447113004/diff/1/content/browser/renderer_hos...
> content/browser/renderer_host/compositing_iosurface_layer_mac.mm:188:
LOG(ERROR)
> << "Failed create CompositingIOSurfaceContext";
> On 2014/08/07 22:11:44, Ken Russell wrote:
> > I wonder whether here and throughout these classes VLOG should be used
instead
> > of LOG to reduce the amount of output emitted by default, though I realize
> these
> > are only displayed in fairly serious error conditions.
> 
> If anything, I'd like to make these errors more visible -- I'd like them to
> appear in about:gpu, if possible -- is there a way to do that in the browser
> process?

I don't remember -- please study the code.

[ccameron, 2014-08-07 22:42:56]

On 2014/08/07 22:34:01, Ken Russell wrote:
> On 2014/08/07 22:28:25, ccameron1 wrote:
> > Thanks!
> > 
> >
>
https://codereview.chromium.org/447113004/diff/1/content/browser/renderer_hos...
> > File content/browser/renderer_host/compositing_iosurface_layer_mac.mm
(right):
> > 
> >
>
https://codereview.chromium.org/447113004/diff/1/content/browser/renderer_hos...
> > content/browser/renderer_host/compositing_iosurface_layer_mac.mm:188:
> LOG(ERROR)
> > << "Failed create CompositingIOSurfaceContext";
> > On 2014/08/07 22:11:44, Ken Russell wrote:
> > > I wonder whether here and throughout these classes VLOG should be used
> instead
> > > of LOG to reduce the amount of output emitted by default, though I realize
> > these
> > > are only displayed in fairly serious error conditions.
> > 
> > If anything, I'd like to make these errors more visible -- I'd like them to
> > appear in about:gpu, if possible -- is there a way to do that in the browser
> > process?
> 
> I don't remember -- please study the code.

Found it by tracking where we report GPU process being killed --
GpuDataManagerImpl::GetInstance()->AddLogMessage does it.

I'll create a separate patch to add CA-related Log messages (there a few I'll
want to catch).

[ccameron, 2014-08-08 08:37:05]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-08-08 08:38:09]

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

[commit-bot: I haz the power, 2014-08-08 11:06:46]

Change committed as 288328