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

Daniel Murphy
Reported 2012-08-01 16:12:55 PDT
Fix for drawing invalid layers in WebViewBenchmarkSupportImpl
Attachments
Patch (1.57 KB, patch)
2012-08-01 16:14 PDT, Daniel Murphy 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Daniel Murphy
Comment 1 2012-08-01 16:14:45 PDT
Created attachment 155916 [details] Patch
James Robinson
Comment 2 2012-08-01 16:25:20 PDT
What's the bug that this fixes?
Adrienne Walker
Comment 3 2012-08-01 16:26:43 PDT
Comment on attachment 155916 [details] Patch Are there any tests for this feature?
Daniel Murphy
Comment 4 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.
Daniel Murphy
Comment 5 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.
Alok Priyadarshi
Comment 6 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.
Nat Duca
Comment 7 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.
Adrienne Walker
Comment 8 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.
Adrienne Walker
Comment 9 2012-08-01 16:44:04 PDT
Comment on attachment 155916 [details] Patch R=me.
WebKit Review Bot
Comment 10 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>
WebKit Review Bot
Comment 11 2012-08-01 17:20:29 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.