Bug 89797 – [chromium] Add the workaround of IOSurface-related corruption during readback on Mac OS X.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Yasuhiro Matsuda]

In https://bugs.webkit.org/show_bug.cgi?id=79735, a workaround of IOSurface-related corruption during readback was added for Mac OS X 10.7.
While looking into the thumbnail corruption in Chromium (http://code.google.com/p/chromium/issues/detail?id=132000), it turned out this work around is needed for 10.6.8 as well.

[Yasuhiro Matsuda]

Created attachment 149149 [details]
Patch

[James Robinson]

Comment on attachment 149149 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=149149&action=review

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:1368
> +        // On Mac OS X 10.6.8+, calling glReadPixels against an FBO whose color attachment is an

do we know that this is broke from 1.6.7 -> 1.6.8?  Should we just do this unconditionally on mac?

[Kenneth Russell]

Comment on attachment 149149 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=149149&action=review

>> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:1368
>> +        // On Mac OS X 10.6.8+, calling glReadPixels against an FBO whose color attachment is an
> 
> do we know that this is broke from 1.6.7 -> 1.6.8?  Should we just do this unconditionally on mac?

I doubt that that is known for sure. Agree, we should just do this on all OS versions now -- it's OK to leave the function needsIOSurfaceReadbackWorkaround, but let's just make it unconditionally return true (IOSurfaces were introduced in 10.6, so it looks like we need the workaround all the time for now, unless we can confirm the issue was fixed in 10.8).

[Yasuhiro Matsuda]

Created attachment 149354 [details]
Patch

[James Robinson]

Comment on attachment 149354 [details]
Patch

R=me

[WebKit Review Bot]

Comment on attachment 149354 [details]
Patch

Rejecting attachment 149354 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 11844 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
47>At revision 11844.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/13093297

[James Robinson]

Oh, I probably gave you merge conflicts with http://trac.webkit.org/changeset/121204.  Would you mind rebasing your patch and re-uploading?

[Yasuhiro Matsuda]

Created attachment 149418 [details]
Patch

[Yasuhiro Matsuda]

(In reply to comment #7)
> Oh, I probably gave you merge conflicts with http://trac.webkit.org/changeset/121204.  Would you mind rebasing your patch and re-uploading?

Resolved the conflict and uploaded a new patch.
Could you set commit-queue flag again if it looks good?

[WebKit Review Bot]

Comment on attachment 149418 [details]
Patch

Clearing flags on attachment: 149418

Committed r121267: <http://trac.webkit.org/changeset/121267>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

Did you file a radar on the IOSurface issue? What's the radar number?

[James Robinson]

(In reply to comment #12)
> Did you file a radar on the IOSurface issue? What's the radar number?

From the inline comment: "http://crbug.com/99393. <rdar://problem/10949687>"