"ABORT: Passed bad frame" with mask pointing at data: URL

VERIFIED FIXED in Firefox 16

Status

()

Core
SVG
--
critical
VERIFIED FIXED
5 years ago
3 years ago

People

(Reporter: Jesse Ruderman, Assigned: jwatt)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla17
x86_64
Mac OS X
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox15 unaffected, firefox16+ verified, firefox17 verified)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 636618 [details]
testcase (asserts fatally when loaded)

###!!! ABORT: Passed bad frame!: 'aFrame->IsFrameOfType(nsIFrame::eSVG) && !(aFrame->GetStateBits() & NS_STATE_IS_OUTER_SVG)', file layout/svg/base/src/nsSVGUtils.cpp, line 614
(Reporter)

Comment 1

5 years ago
Created attachment 636619 [details]
stack trace
(Assignee)

Updated

5 years ago
Assignee: nobody → jwatt
(Assignee)

Comment 2

5 years ago
Created attachment 647229 [details] [diff] [review]
patch

We're making the wrong invalidate call for masks applied to outer-<svg>. This fixes that.
Attachment #647229 - Flags: review?(roc)
(Assignee)

Comment 3

5 years ago
Created attachment 647231 [details] [diff] [review]
patch

Now with crashtests.list change.
Attachment #647229 - Attachment is obsolete: true
Attachment #647229 - Flags: review?(roc)
Attachment #647231 - Flags: review?(roc)

Comment 4

5 years ago
Comment on attachment 647231 [details] [diff] [review]
patch

>+  if ((mFrame->GetStateBits() & NS_FRAME_SVG_LAYOUT)) {

One set of brackets is surely enough.

r=longsonr with that change
Attachment #647231 - Flags: review?(roc) → review+
(Assignee)

Comment 5

5 years ago
(In reply to Robert Longson from comment #4)
> r=longsonr with that change

Thanks. It's common convention to flag bitwise-and with an extra pair of parenthesis though, and I think some compilers warn if you don't. For that reason I checked it in with the extra parenthesis.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ac323ff812e
Target Milestone: --- → mozilla17

Comment 6

5 years ago
The only warn if you use compare to something e.g. if (a & b == c) as that's probably not going to work as intended.
Sorry, I backed this out on inbound because the test fails with an assertion:
https://hg.mozilla.org/integration/mozilla-inbound/rev/205889645965

https://tbpl.mozilla.org/php/getParsedLog.php?id=13978494&tree=Mozilla-Inbound

++DOMWINDOW == 87 (0xb271b3c) [serial = 3858] [outer = 0xac582a0]
###!!! ASSERTION: Should not use nsSVGIntegrationUtils on this SVG frame: '!svgChildFrame || (NS_SVGDisplayListPaintingEnabled() && !(aFrame->GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD))', file ../../../../../layout/svg/base/src/nsSVGIntegrationUtils.cpp, line 390
std::map<ogg_packet*, long, std::less<ogg_packet*>, std::allocator<std::pair<ogg_packet* const, long> > >::operator[](ogg_packet* const&) (/home/cltbld/talos-slave/test/build/firefox/libxul.so)
std::vector<nsRefPtr<imgCacheEntry>, std::allocator<nsRefPtr<imgCacheEntry> > >::push_back(nsRefPtr<imgCacheEntry> const&) (/home/cltbld/talos-slave/test/build/firefox/libxul.so)
std::vector<nsRefPtr<imgCacheEntry>, std::allocator<nsRefPtr<imgCacheEntry> > >::push_back(nsRefPtr<imgCacheEntry> const&) (/home/cltbld/talos-slave/test/build/firefox/libxul.so)
std::vector<mozilla::gfx::GradientStop, std::allocator<mozilla::gfx::GradientStop> >::push_back(mozilla::gfx::GradientStop const&) (/home/cltbld/talos-slave/test/build/firefox/libxul.so)
Target Milestone: mozilla17 → ---
(Assignee)

Comment 8

5 years ago
(In reply to Matt Brubeck (:mbrubeck) from comment #7)
> Sorry, I backed this out

Thanks for backing it out.
(Assignee)

Comment 9

5 years ago
Pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/cb2bc08167c7
Target Milestone: --- → mozilla17
(Assignee)

Updated

5 years ago
Depends on: 776337
https://hg.mozilla.org/mozilla-central/rev/cb2bc08167c7
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

5 years ago
This fixed bug 779403.
Blocks: 779403
status-firefox15: --- → unaffected
status-firefox16: --- → affected
status-firefox17: --- → affected
tracking-firefox16: --- → ?
(Assignee)

Comment 12

5 years ago
Comment on attachment 647231 [details] [diff] [review]
patch

[Approval Request Comment]

The real bug we want to fix on aurora is bug 779403, but this bug's patch fixed that bug, so requesting approval on this patch.

Bug caused by (feature/regressing bug #): 738192
User impact if declined: SVG masking is broken
Testing completed (on m-c, etc.): patch has been on m-c for several days
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none
Attachment #647231 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 13

5 years ago
Err, for "Risk to taking this patch (and alternatives if risky)" I seem to have only processed the "alternatives" part. The risk is very low though, but yeah, there aren't really any viable alternatives.
Tracking and updating the status for 17.
status-firefox17: affected → fixed
tracking-firefox16: ? → +
Attachment #647231 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 15

5 years ago
Pushed https://hg.mozilla.org/releases/mozilla-aurora/rev/74324fb8f642
status-firefox16: affected → fixed
Keywords: verifyme
I have tried this on:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:16.0) Gecko/20120917 Firefox/16.0 debug build

using the test case provided and there is no assertion, no abord message.

Changing the flag to Verified on Firefox 16.
status-firefox16: fixed → verified
Depends on: 795592
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/17.0 Firefox/17.0 (latest beta debug buid)

Verified the fix on the above build: there is no assertion/abort message when opening the testcase attached in bug description.
Status: RESOLVED → VERIFIED
status-firefox17: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.