Bug 51668 – cleanupAfterGraphicsCall is never called in WebGLRenderingContext::checkFramebufferStatus

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Takashi Toyoshima]

cleanupAfterGraphicsCall() is called at the end of functions for marking graphic context as dirty.
In WebGLRenderingContext::checkFramebufferStatus(), cleanupAfterGraphicsCall() is called after return.
It mean this cleanAfterGraphicsCall() is never called.
Actually, argument of this function is 'false', and it mean this function do nothing.
But I think this is a trivial oversight, and it cause compile error with some gcc option (for example, -finstrument-functions).
This problem appears in a debug configuration of chromium.
(http://codereview.chromium.org/5991005/)

[Takashi Toyoshima]

Created attachment 77538 [details]
a simple patch which doesn't change the program meaning

[WebKit Review Bot]

Attachment 77538 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebCore/ChangeLog', u'WebCore/html/canvas/WebGLRenderingContext.cpp']" exit_code: 1
WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Takashi Toyoshima]

Created attachment 77539 [details]
revised patch (previous one contained TAB character in ChangeLog and it broke style check)

[Eric Seidel (no email)]

Are you sure this is the right call order?

[Zhenyao Mo]

Comment on attachment 77539 [details]
revised patch (previous one contained TAB character in ChangeLog and it broke style check)

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

> WebCore/html/canvas/WebGLRenderingContext.cpp:565
>  }

You should call m_context->checkFramebufferStatus(target), cache the returned value, cleanupAfterGraphicsCall, then return the cached value.

[Takashi Toyoshima]

Thank you for reviewing the code.
I'll post revised one.

[Takashi Toyoshima]

Created attachment 77600 [details]
Patch

[Kenneth Russell]

Comment on attachment 77600 [details]
Patch

Looks fine per Mo's earlier review.

[Zhenyao Mo]

Comment on attachment 77600 [details]
Patch

Clearing flags on attachment: 77600

Committed r75251: <http://trac.webkit.org/changeset/75251>

[Zhenyao Mo]

All reviewed patches have been landed.  Closing bug.