Bug 80445

Summary: [chromium] Null-check m_layerRenderer in CCLayerTreeHostImpl::finishAllRendering()
Product: WebKit Reporter: James Robinson <jamesr>
Component: New BugsAssignee: James Robinson <jamesr>
Severity: Normal CC: cc-bugs, enne, nduca, vangelis, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
Patch enne: review+

Description James Robinson 2012-03-06 14:56:52 PST
[chromium] Null-check m_layerRenderer in CCLayerTreeHostImpl::finishAllRendering()
Comment 1 James Robinson 2012-03-06 14:57:35 PST
Created attachment 130448 [details]
Comment 2 James Robinson 2012-03-06 14:58:13 PST
This should fix the crash in https://bugs.webkit.org/show_bug.cgi?id=80445.  I'm still not quite sure how to construct a good test for it.
Comment 3 Nat Duca 2012-03-06 15:08:38 PST
Comment on attachment 130448 [details]

That works. You might do a CCLayerTreeHostTest that does a draw inside CCLayerTreeHostTestHooks::layout()... /me forgets, has the renderer been initialized by then?
Comment 4 Adrienne Walker 2012-03-06 15:15:12 PST
Comment on attachment 130448 [details]

I see a clear path where this could happen.  If initializing the layer renderer fails, WebViewImpl will call finishAllRendering in response to didRebindGraphicsContext(false).  What about a test with a fake proxy that fails to initialize a layer renderer and calls finishAllRendering in response? Are you seeing another case where it could happen?
Comment 5 James Robinson 2012-03-06 15:20:21 PST
Yeah, that should do the trick.  Will try it...
Comment 6 James Robinson 2012-03-06 15:45:22 PST
Created attachment 130456 [details]
Comment 7 James Robinson 2012-03-06 15:45:47 PST
I was able to write a more direct unit test - without the CCLayerTreeHostImpl changes, the new unit tests just crashes.
Comment 8 Adrienne Walker 2012-03-06 15:51:36 PST
Comment on attachment 130456 [details]

Thanks for the test.  :)
Comment 9 James Robinson 2012-03-06 17:16:17 PST
Committed r109985: <http://trac.webkit.org/changeset/109985>