Bug 92920

Summary: Fix for drawing invalid layers in WebViewBenchmarkSupportImpl
Product: WebKit Reporter: Daniel Murphy <dmurph>
Component: New BugsAssignee: Daniel Murphy <dmurph>
Status: RESOLVED FIXED    
Severity: Normal CC: alokp, crogers, enne, jamesr, nduca, scheib, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Daniel Murphy 2012-08-01 16:12:55 PDT
Fix for drawing invalid layers in WebViewBenchmarkSupportImpl
Comment 1 Daniel Murphy 2012-08-01 16:14:45 PDT
Created attachment 155916 [details]
Patch
Comment 2 James Robinson 2012-08-01 16:25:20 PDT
What's the bug that this fixes?
Comment 3 Adrienne Walker 2012-08-01 16:26:43 PDT
Comment on attachment 155916 [details]
Patch

Are there any tests for this feature?
Comment 4 Daniel Murphy 2012-08-01 16:26:55 PDT
There was an assertion being thrown when we tried to paint non-content layer, at RenderLayerBacking.cpp:1219.  This only shows up on websites with backing layers like the poster circle.
Comment 5 Daniel Murphy 2012-08-01 16:32:07 PDT
No, there are none.  Testing this specifically seems a bit infeasible.  This provides the drawing for canvas serialization, so when that's working we can do bitmap based tests.
Comment 6 Alok Priyadarshi 2012-08-01 16:38:35 PDT
I think it will be nice to have a simple webkit test for WebViewBenchmarkSupport::paint() since it uses a path different from the actual paint path. Testing it downstream will be hard to maintain.
Comment 7 Nat Duca 2012-08-01 16:41:02 PDT
I think no test is needed for this.

This is not because I think testing is unnecessary. But rather, that testing for this specific bug is busy work. There is a broader question of how to test the support code. And whether its testable.

I propose we land this fix, and then Alokp, Daniel and I deal with the testability discussion offline. It may be that we're going to change this more heavily.
Comment 8 Adrienne Walker 2012-08-01 16:43:49 PDT
(In reply to comment #7)

> This is not because I think testing is unnecessary. But rather, that testing for this specific bug is busy work. There is a broader question of how to test the support code. And whether its testable.

I think that was really my question.  I hadn't ever seen WebViewBenchmarkSupport before and was curious if there was any testing strategy here.
 
> I propose we land this fix, and then Alokp, Daniel and I deal with the testability discussion offline. It may be that we're going to change this more heavily.

Sure.
Comment 9 Adrienne Walker 2012-08-01 16:44:04 PDT
Comment on attachment 155916 [details]
Patch

R=me.
Comment 10 WebKit Review Bot 2012-08-01 17:20:25 PDT
Comment on attachment 155916 [details]
Patch

Clearing flags on attachment: 155916

Committed r124391: <http://trac.webkit.org/changeset/124391>
Comment 11 WebKit Review Bot 2012-08-01 17:20:29 PDT
All reviewed patches have been landed.  Closing bug.