Bug 80445 - [chromium] Null-check m_layerRenderer in CCLayerTreeHostImpl::finishAllRendering()
Summary: [chromium] Null-check m_layerRenderer in CCLayerTreeHostImpl::finishAllRender...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-06 14:56 PST by James Robinson
Modified: 2012-03-06 17:16 PST (History)
5 users (show)

See Also:

Attachments
Patch (1.37 KB, patch)
2012-03-06 14:57 PST, James Robinson 		no flags Details | Formatted Diff | Diff
Patch (3.57 KB, patch)
2012-03-06 15:45 PST, James Robinson 		enne: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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]
Patch
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]
Patch

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]
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?
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]
Patch
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]
Patch

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