1278199 - Assertion failure: int4[0] == mViewportX && int4[1] == mViewportY && int4[2] == mViewportWidth && int4[3] == mViewportHeight,

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Raymond Forbes[:rforbes]]

Attachment: testcase

Code in question is here.

WebGLContext::AssertCachedGlobalState()
{
#ifdef DEBUG
    MakeContextCurrent();

    GetAndFlushUnderlyingGLErrors();

    ////////////////

    // Draw state
    MOZ_ASSERT(gl->fIsEnabled(LOCAL_GL_DEPTH_TEST) == mDepthTestEnabled);

Assertion failure: int4[0] == mViewportX && int4[1] == mViewportY && int4[2] == mViewportWidth && int4[3] == mViewportHeight, at c:/src/mozilla/mozilla-central/dom/canvas/WebGLContextUtils.cpp:801
#01: mozilla::WebGLContext::SetDimensions (c:\src\mozilla\mozilla-central\dom\canvas\webglcontext.cpp:1005)
#02: mozilla::WebGLContext::TryToRestoreContext (c:\src\mozilla\mozilla-central\dom\canvas\webglcontext.cpp:1602)
#03: mozilla::UpdateContextLossStatusTask::Run (c:\src\mozilla\mozilla-central\dom\canvas\webglcontext.cpp:1628)
#04: nsThread::ProcessNextEvent (c:\src\mozilla\mozilla-central\xpcom\threads\nsthread.cpp:1029)
#05: NS_ProcessNextEvent (c:\src\mozilla\mozilla-central\xpcom\glue\nsthreadutils.cpp:290)
#06: mozilla::ipc::MessagePump::Run (c:\src\mozilla\mozilla-central\ipc\glue\messagepump.cpp:100)
#07: MessageLoop::RunInternal (c:\src\mozilla\mozilla-central\ipc\chromium\src\base\message_loop.cc:235)
#08: MessageLoop::RunHandler (c:\src\mozilla\mozilla-central\ipc\chromium\src\base\message_loop.cc:229)
#09: MessageLoop::Run (c:\src\mozilla\mozilla-central\ipc\chromium\src\base\message_loop.cc:209)
#10: nsBaseAppShell::Run (c:\src\mozilla\mozilla-central\widget\nsbaseappshell.cpp:158)
#11: nsAppShell::Run (c:\src\mozilla\mozilla-central\widget\windows\nsappshell.cpp:264)
#12: nsAppStartup::Run (c:\src\mozilla\mozilla-central\toolkit\components\startup\nsappstartup.cpp:285)
#13: XREMain::XRE_mainRun (c:\src\mozilla\mozilla-central\toolkit\xre\nsapprunner.cpp:4374)
#14: XREMain::XRE_main (c:\src\mozilla\mozilla-central\toolkit\xre\nsapprunner.cpp:4478)
#15: XRE_main (c:\src\mozilla\mozilla-central\toolkit\xre\nsapprunner.cpp:4586)
#16: do_main (c:\src\mozilla\mozilla-central\browser\app\nsbrowserapp.cpp:242)
#17: NS_internal_main (c:\src\mozilla\mozilla-central\browser\app\nsbrowserapp.cpp:382)
#18: wmain (c:\src\mozilla\mozilla-central\toolkit\xre\nswindowswmain.cpp:130)
#19: invoke_main (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:79)
#20: __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:255)
#21: __scrt_common_main (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:300)
#22: wmainCRTStartup (f:\dd\vctools\crt\vcstartup\src\startup\exe_wmain.cpp:17)
#23: BaseThreadInitThunk[C:\Windows\SYSTEM32\KERNEL32.DLL +0x138f4]
#24: RtlUnicodeStringToInteger[C:\Windows\SYSTEM32\ntdll.dll +0x65de3]
#25: RtlUnicodeStringToInteger[C:\Windows\SYSTEM32\ntdll.dll +0x65dae]

Comment 1

[Daniel Veditz [:dveditz]]

Jeff: is this an assertion we should worry about wrt security? If not is there a way to fix or suppress it so it doesn't trigger false positives in our testing tools?

Comment 2

[Kelsey Gilbert [:jgilbert]]

(In reply to Daniel Veditz [:dveditz] from comment #1)
> Jeff: is this an assertion we should worry about wrt security? If not is
> there a way to fix or suppress it so it doesn't trigger false positives in
> our testing tools?

This should just be a correctness bug with no security repercussions.

Comment 3

[Dzmitry Malyshau [:kvark]]

Attachment: Reset viewport offset on SetDimensions and clamp to MAX_VIEWPORT_DIMS.

Comment 4

[Dzmitry Malyshau [:kvark]]

(pending) try results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=20a3861a84a6057fa911c70e22050c99909a38a2

Comment 5

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8825547 [details] [diff] [review]
Reset viewport offset on SetDimensions and clamp to MAX_VIEWPORT_DIMS.

Review of attachment 8825547 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/WebGLContextGL.cpp
@@ +2139,5 @@
>  
>      if (width < 0 || height < 0)
>          return ErrorInvalidValue("viewport: negative size");
>  
> +    width = std::min(width, (GLsizei)mImplMaxViewportDims[0]);

std::min(uint32_t(width), mImpl...) is more correct, though for our expected ranges here it shouldn't matter.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Should we be landing the testcase as a crashtest as well?

Comment 7

[Kelsey Gilbert [:jgilbert]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)
> Should we be landing the testcase as a crashtest as well?

Not for this, no.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/84c7a36da084
Reset viewport offset on SetDimensions and clamp to MAX_VIEWPORT_DIMS. r=jgilbert

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/84c7a36da084

Comment 10

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8825547 [details] [diff] [review]
Reset viewport offset on SetDimensions and clamp to MAX_VIEWPORT_DIMS.

Approval Request Comment
[Feature/Bug causing the regression]: n/a
[User impact if declined]: Spurious assert when running DEBUG builds
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: no
[Why is the change risky/not risky?]: Practically DEBUG-only.
[String changes made/needed]:

Comment 11

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8825547 [details] [diff] [review]
Reset viewport offset on SetDimensions and clamp to MAX_VIEWPORT_DIMS.

webgl viewport fix for aurora52

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d306f88dfd3a