Bug 92920 - Fix for drawing invalid layers in WebViewBenchmarkSupportImpl
Summary: Fix for drawing invalid layers in WebViewBenchmarkSupportImpl
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: Daniel Murphy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-01 16:12 PDT by Daniel Murphy
Modified: 2012-08-01 19:11 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.57 KB, patch)
2012-08-01 16:14 PDT, Daniel Murphy
no flags Details | Formatted Diff | Diff

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