Last Comment Bug 768351 - "ABORT: Passed bad frame" with mask pointing at data: URL
: "ABORT: Passed bad frame" with mask pointing at data: URL
: assertion, testcase
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- critical (vote)
: mozilla17
Assigned To: Jonathan Watt [:jwatt]
: Jet Villegas (:jet)
Depends on: 776337 795592
Blocks: randomstyles 779403
  Show dependency treegraph
Reported: 2012-06-26 00:54 PDT by Jesse Ruderman
Modified: 2014-02-26 11:47 PST (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (asserts fatally when loaded) (85 bytes, image/svg+xml)
2012-06-26 00:54 PDT, Jesse Ruderman
no flags Details
stack trace (6.18 KB, text/plain)
2012-06-26 00:56 PDT, Jesse Ruderman
no flags Details
patch (1.09 KB, patch)
2012-07-30 11:36 PDT, Jonathan Watt [:jwatt]
no flags Details | Diff | Splinter Review
patch (1.47 KB, patch)
2012-07-30 11:38 PDT, Jonathan Watt [:jwatt]
longsonr: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Jesse Ruderman 2012-06-26 00:54:18 PDT
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
Comment 1 Jesse Ruderman 2012-06-26 00:56:27 PDT
Created attachment 636619 [details]
stack trace
Comment 2 Jonathan Watt [:jwatt] 2012-07-30 11:36:41 PDT
Created attachment 647229 [details] [diff] [review]

We're making the wrong invalidate call for masks applied to outer-<svg>. This fixes that.
Comment 3 Jonathan Watt [:jwatt] 2012-07-30 11:38:57 PDT
Created attachment 647231 [details] [diff] [review]

Now with crashtests.list change.
Comment 4 Robert Longson 2012-07-30 11:41:43 PDT
Comment on attachment 647231 [details] [diff] [review]

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

One set of brackets is surely enough.

r=longsonr with that change
Comment 5 Jonathan Watt [:jwatt] 2012-07-30 12:16:50 PDT
(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.
Comment 6 Robert Longson 2012-07-30 12:29:57 PDT
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.
Comment 7 Matt Brubeck (:mbrubeck) 2012-07-30 15:13:10 PDT
Sorry, I backed this out on inbound because the test fails with an assertion:

++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/
std::vector<nsRefPtr<imgCacheEntry>, std::allocator<nsRefPtr<imgCacheEntry> > >::push_back(nsRefPtr<imgCacheEntry> const&) (/home/cltbld/talos-slave/test/build/firefox/
std::vector<nsRefPtr<imgCacheEntry>, std::allocator<nsRefPtr<imgCacheEntry> > >::push_back(nsRefPtr<imgCacheEntry> const&) (/home/cltbld/talos-slave/test/build/firefox/
std::vector<mozilla::gfx::GradientStop, std::allocator<mozilla::gfx::GradientStop> >::push_back(mozilla::gfx::GradientStop const&) (/home/cltbld/talos-slave/test/build/firefox/
Comment 8 Jonathan Watt [:jwatt] 2012-07-31 03:01:07 PDT
(In reply to Matt Brubeck (:mbrubeck) from comment #7)
> Sorry, I backed this out

Thanks for backing it out.
Comment 9 Jonathan Watt [:jwatt] 2012-07-31 03:01:29 PDT
Comment 10 Ryan VanderMeulen [:RyanVM] 2012-07-31 19:20:29 PDT
Comment 11 Jonathan Watt [:jwatt] 2012-08-01 03:38:50 PDT
This fixed bug 779403.
Comment 12 Jonathan Watt [:jwatt] 2012-08-01 03:40:51 PDT
Comment on attachment 647231 [details] [diff] [review]

[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
Comment 13 Jonathan Watt [:jwatt] 2012-08-01 03:45:13 PDT
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.
Comment 14 Lukas Blakk [:lsblakk] use ?needinfo 2012-08-01 17:15:09 PDT
Tracking and updating the status for 17.
Comment 15 Jonathan Watt [:jwatt] 2012-08-01 17:54:09 PDT
Comment 16 Mihaela Velimiroviciu (:mihaelav) 2012-09-25 04:46:45 PDT
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.
Comment 17 Mihaela Velimiroviciu (:mihaelav) 2012-10-17 02:48:26 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.