Bug 145479 - Crash in GraphicsContext3D::getInternalFramebufferSize
Summary: Crash in GraphicsContext3D::getInternalFramebufferSize
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: Dean Jackson
URL:
Keywords:
Depends on: 145597
Blocks:
  Show dependency treegraph
 
Reported: 2015-05-29 15:49 PDT by Dean Jackson
Modified: 2015-06-03 15:35 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.84 KB, patch)
2015-05-29 16:06 PDT, Dean Jackson
no flags Details | Formatted Diff | Diff
Patch (11.23 KB, patch)
2015-06-02 14:26 PDT, Dean Jackson
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2015-05-29 15:49:33 PDT
Crash in GraphicsContext3D::getInternalFramebufferSize
Comment 1 Dean Jackson 2015-05-29 15:56:56 PDT
<rdar://problem/16461048>
Comment 2 Dean Jackson 2015-05-29 16:06:48 PDT
Created attachment 253928 [details]
Patch
Comment 3 Sam Weinig 2015-05-29 16:14:13 PDT
Comment on attachment 253928 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        This can't be easily tested automatically, because it's a specific
> +        configuration of Safari that uses WebGL in this manner.

We should add the necessary infrastructure to make it testable.
Comment 4 Dean Jackson 2015-06-02 14:26:03 PDT
Created attachment 254103 [details]
Patch
Comment 5 Eric Carlson 2015-06-02 15:05:36 PDT
Comment on attachment 254103 [details]
Patch

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

> LayoutTests/fast/canvas/webgl/useWhilePending.html:18
> +        window.internals.settings.setForcePendingWebGLPolicy(false);

If you reset the property in InternalSettings::resetToConsistentState this won't be necessary.
Comment 6 Dean Jackson 2015-06-02 15:12:02 PDT
Committed r185128: <http://trac.webkit.org/changeset/185128>
Comment 8 Alexey Proskuryakov 2015-06-02 23:11:07 PDT
That was because the test is not expected to work with WebKit1.

However, WebKit2 is broken too, it hits a very scary assertion: <https://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK2%20(Tests)/r185140%20(12167)/fast/canvas/webgl/viewport-unchanged-upon-resize-crash-log.txt>.

This may be a pre-existing bug, but given that:
1) it's a very bad assertion to hit, and
2) it essentially forces us to skip the test, and
3) Sam requested this fix to be tested,

I'm going to roll out.

I suspect that there is an actual issue with uninitialized WebGL contexts when navigating.
Comment 9 WebKit Commit Bot 2015-06-02 23:17:26 PDT
Re-opened since this is blocked by bug 145597
Comment 10 Dean Jackson 2015-06-03 15:35:53 PDT
Re-landed in https://trac.webkit.org/r185172

Alexey was right - it was doing some "bad stuff" with the unitialised context, and hadn't been tested before. This bug has been around for about a year. The fix was pretty clear - to add
renderingContext->suspendIfNeeded();