Bug 75589 - [Chromium] Do not recreate texture updater for image layer if one exists.
Summary: [Chromium] Do not recreate texture updater for image layer if one exists.
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 Sievers
URL:
Keywords:
: 76185 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-01-04 17:21 PST by Daniel Sievers
Modified: 2012-01-12 10:53 PST (History)
7 users (show)

See Also:


Attachments
Patch (1.66 KB, patch)
2012-01-04 17:22 PST, Daniel Sievers
no flags Details | Formatted Diff | Diff
Patch (27.15 KB, patch)
2012-01-06 15:01 PST, Daniel Sievers
no flags Details | Formatted Diff | Diff
Patch (35.04 KB, patch)
2012-01-06 16:56 PST, Daniel Sievers
no flags Details | Formatted Diff | Diff
Patch (26.32 KB, patch)
2012-01-09 13:43 PST, Daniel Sievers
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 Sievers 2012-01-04 17:21:08 PST
[Chromium] Do not recreate texture updater for image layer if one exists.
Comment 1 Daniel Sievers 2012-01-04 17:22:04 PST
Created attachment 121190 [details]
Patch
Comment 2 James Robinson 2012-01-04 17:23:20 PST
Comment on attachment 121190 [details]
Patch

Any way to test this? Perhaps a layout test that shows the image corruption?
Comment 3 Daniel Sievers 2012-01-06 15:01:30 PST
Created attachment 121502 [details]
Patch
Comment 4 Daniel Sievers 2012-01-06 15:05:09 PST
I had no luck with reparenting through JavaScript, because it ended up creating a new ImageLayer in that case, which doesn't show the bug.

See attached patch, growing the layer (which inserts the scrollbar layer) however shows the problem.

The initial layoutController.display() before changing the layer size seems to be necessary also to make it fail. Not sure if that is the right directory for the resource .jpg. I copied it from the top-level compositing/resources dir, although
I saw another test reference it from the platform dir relatively, but when I ran new-run-webkit-tests that wouldn't work to find it.

(In reply to comment #2)
> (From update of attachment 121190 [details])
> Any way to test this? Perhaps a layout test that shows the image corruption?
Comment 5 WebKit Review Bot 2012-01-06 16:11:37 PST
Comment on attachment 121502 [details]
Patch

Attachment 121502 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11170117

New failing tests:
platform/chromium/compositing/img-layer-grow.html
Comment 6 Daniel Sievers 2012-01-06 16:56:58 PST
Created attachment 121523 [details]
Patch
Comment 7 James Robinson 2012-01-06 17:57:14 PST
You should not need to copy any resources, a relative include should work fine. Are you sure you got the path right?

Please use mock scrollbars or css-style them if they are going to be in the visible test output.

add a layoutTestController.dumpAsText(true) to suppress the normal render tree output, it's not needed for this test.
Comment 8 Daniel Sievers 2012-01-09 13:43:02 PST
Created attachment 121721 [details]
Patch
Comment 9 James Robinson 2012-01-09 15:00:22 PST
Comment on attachment 121721 [details]
Patch

Nice, R=me
Comment 10 Daniel Sievers 2012-01-10 11:33:17 PST
James, can you cq+? Thanks!

(In reply to comment #9)
> (From update of attachment 121721 [details])
> Nice, R=me
Comment 11 James Robinson 2012-01-10 11:35:01 PST
Comment on attachment 121721 [details]
Patch

Sure. In the future you can request this by setting the commit-queue flag to "?" or by running webkit-patch upload --request-commit
Comment 12 WebKit Review Bot 2012-01-10 11:52:53 PST
Comment on attachment 121721 [details]
Patch

Clearing flags on attachment: 121721

Committed r104610: <http://trac.webkit.org/changeset/104610>
Comment 13 WebKit Review Bot 2012-01-10 11:52:58 PST
All reviewed patches have been landed.  Closing bug.
Comment 14 Shawn Singh 2012-01-12 10:53:18 PST
*** Bug 76185 has been marked as a duplicate of this bug. ***