1545842 - Downgrade a few release asserts to diagnostic asserts on beta.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

The only case where we know that this assert breaks is in nsIconChannel, which is not exposed to content.

So it doesn't seem to be easy to potentially exploit it, and the crashes in release may be a bit excessive.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1545842 - Downgrade RestyleManager assertions to DIAGNOSTIC_ASSERT in beta. r=tnikkel

The only case where we know that this assert breaks right now is in
nsIconChannel.

nsIconChannel is used for moz-icon://, which is not exposed to content.

So it doesn't seem to be easy to potentially get into a broken state that
content could exploit, and thus crashes in release may be a bit excessive.

Downgrade the assertion for now.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9059586 [details]
Bug 1545842 - Downgrade RestyleManager assertions to DIAGNOSTIC_ASSERT in beta. r=tnikkel

Beta/Release Uplift Approval Request

• User impact if declined: Crashes (bug 1530190).
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: We have no particular STR for bug 1530190.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just downgrades an assertion so as to not crash on release.
• String changes made/needed: none

Comment 3

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9059586 [details]
Bug 1545842 - Downgrade RestyleManager assertions to DIAGNOSTIC_ASSERT in beta. r=tnikkel

Downgrading of an assertion to not crash in release, uplift approved for 67 beta 13, thanks.

Comment 4

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bdd03a2c4e54

Comment 5

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

Emilio, given that bug 1530190 landed for 69, should we land the (safer?) patch here for 68?

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Yes, should I re-request beta-uplift here?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Err...

Comment 8

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

Bit confusing because this already has beta approval from the 67 cycle, so I'll use the whiteboard. Sheriffs, please consider this a=jcristau for beta 68.

Comment 9

[Oana Pop-Rus]

https://hg.mozilla.org/releases/mozilla-beta/rev/5d6b91a3b536