Bug 80445

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

James Robinson
Reported 2012-03-06 14:56:52 PST
[chromium] Null-check m_layerRenderer in CCLayerTreeHostImpl::finishAllRendering()
Attachments
Patch (1.37 KB, patch)
2012-03-06 14:57 PST, James Robinson 		no flags
DetailsFormatted DiffDiff
Patch (3.57 KB, patch)
2012-03-06 15:45 PST, James Robinson 		enne: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
James Robinson
Comment 1 2012-03-06 14:57:35 PST
Created attachment 130448 [details] Patch
James Robinson
Comment 2 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.
Nat Duca
Comment 3 2012-03-06 15:08:38 PST
Comment on attachment 130448 [details] Patch That works. You might do a CCLayerTreeHostTest that does a draw inside CCLayerTreeHostTestHooks::layout()... /me forgets, has the renderer been initialized by then?
Adrienne Walker
Comment 4 2012-03-06 15:15:12 PST
Comment on attachment 130448 [details] Patch 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?
James Robinson
Comment 5 2012-03-06 15:20:21 PST
Yeah, that should do the trick. Will try it...
James Robinson
Comment 6 2012-03-06 15:45:22 PST
Created attachment 130456 [details] Patch
James Robinson
Comment 7 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.
Adrienne Walker
Comment 8 2012-03-06 15:51:36 PST
Comment on attachment 130456 [details] Patch Thanks for the test. :)
James Robinson
Comment 9 2012-03-06 17:16:17 PST
Committed r109985: <http://trac.webkit.org/changeset/109985>
Note You need to log in before you can comment on or make changes to this bug.