1277027 - Fix a few places in WebGL/Canvas2D where graphics MOZ_CRASH does not have a GFX prefixed message

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

There are a few files in dom/canvas that use MOZ_CRASH, but don't have GFX: prefix, like the majority of the graphics code.  Makes it easier to do a quick search in the crash reports.  See also bug 1272767.

Comment 1

[Ernest Yim [:ernest]]

Attachment: Bug 1277027 - fix MOZ_CRASH to have GFX prefix in webGL code

Review commit: https://reviewboard.mozilla.org/r/57288/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57288/

Comment 2

[Ernest Yim [:ernest]]

Comment on attachment 8759293 [details]
Bug 1277027 - fix MOZ_CRASH to have GFX prefix in webGL code

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57288/diff/1-2/

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8759293 [details]
Bug 1277027 - fix MOZ_CRASH to have GFX prefix in webGL code

https://reviewboard.mozilla.org/r/57288/#review54118

Comment 4

[Ernest Yim [:ernest]]

Will need someone to help me land, if/when this is approved.

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

Nice, thanks Ernest.

I wouldn't mind you doing a pass through MOZ_RELEASE_ASSERT calls we make, in gfx/ and dom/canvas/ and making sure they all have a string description (second parameter), with the GFX: prefix.  Pretty sure none of them do right now.  In this bug is fine.

Comment 6

[Ernest Yim [:ernest]]

Comment on attachment 8759293 [details]
Bug 1277027 - fix MOZ_CRASH to have GFX prefix in webGL code

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57288/diff/2-3/

Comment 7

[Benoit Girard (:BenWa)]

I feel like we need a policy for putting so many string in the binary. I also added a lot via the failure ID and over time these strings really add up.

Comment 8

[Benoit Girard (:BenWa)]

Comment on attachment 8759293 [details]
Bug 1277027 - fix MOZ_CRASH to have GFX prefix in webGL code

https://reviewboard.mozilla.org/r/57288/#review54424

Looks good, just a few string nitpicks

::: dom/canvas/WebGLContext.cpp:675
(Diff revision 3)
>                                    nsACString* const out_failureId)
>  {
>      std::queue<gl::SurfaceCaps> fallbackCaps;
>      PopulateCapFallbackQueue(baseCaps, &fallbackCaps);
>  
> -    MOZ_RELEASE_ASSERT(!gl);
> +    MOZ_RELEASE_ASSERT(!gl, "GFX: Create context flags set.");

Already have a context

::: dom/canvas/WebGLContext.cpp:2109
(Diff revision 3)
>      gl::GLContext* gl = webgl->GL();
>      gl->MakeCurrent();
>  
>      auto compression = usage->format->compression;
>      if (compression) {
> -        MOZ_RELEASE_ASSERT(!xOffset && !yOffset && !zOffset);
> +        MOZ_RELEASE_ASSERT(!xOffset && !yOffset && !zOffset, "GFX: texture offsets not zeroed.");

Can't zero compressed texture with offsets

Comment 9

[Ernest Yim [:ernest]]

Comment on attachment 8759293 [details]
Bug 1277027 - fix MOZ_CRASH to have GFX prefix in webGL code

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57288/diff/3-4/

Comment 10

[Ernest Yim [:ernest]]

if no more issues, could you please try and/or land this for me?

Comment 11

[Benoit Girard (:BenWa)]

Try submited, let me know when it's ready to land.

Comment 12

[Ernest Yim [:ernest]]

Comment on attachment 8759293 [details]
Bug 1277027 - fix MOZ_CRASH to have GFX prefix in webGL code

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57288/diff/4-5/

Comment 13

[Ernest Yim [:ernest]]

Could you please resubmit try?

Comment 14

[Benoit Girard (:BenWa)]

Done, if you can't submit from mozreview you should still be able to push using:
hg push -f try

Just looking to save you the extra delay of me pushing it.

Comment 15

[Ernest Yim [:ernest]]

Comment on attachment 8759293 [details]
Bug 1277027 - fix MOZ_CRASH to have GFX prefix in webGL code

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57288/diff/5-6/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/17276a2bbd6a
fix MOZ_CRASH to have GFX prefix in webGL code r=BenWa,milan

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/17276a2bbd6a