Closed
Bug 767056
Opened 12 years ago
Closed 12 years ago
crash in nsRegion::Or (when I test Bug 766956, browser crashes when resize browser)
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: alice0775, Assigned: jwatt)
References
()
Details
(Keywords: crash, regression, reproducible)
Crash Data
Attachments
(1 file)
(deleted),
patch
|
roc
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is
report bp-1933edf5-00de-4de3-bfef-549ea2120621 .
=============================================================
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/10e019421e6b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0a1 ID:20120621053048
http://hg.mozilla.org/releases/mozilla-aurora/rev/d557426b0c8d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/20120621 Firefox/15.0a2 ID:20120621042006
when I test Bug 766956, browser crashes when resize browser
Steps to Reproduce:
1. Open http://granite.sru.edu/~ddailey/svg/B/BBox0.svg
2. Resize browser width by dragging window side border quickly
3. Repeat step2 10-20 times
Actual Results:
Browser crashes
Expected Results:
No crash
Regression window(m-c)
Not crash:
http://hg.mozilla.org/mozilla-central/rev/f2b2b99108a2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120517030029
Crash:
http://hg.mozilla.org/mozilla-central/rev/895e12563245
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120517110214
Pusjlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f2b2b99108a2&tochange=895e12563245
Regression window(m-i)
Not crash:
http://hg.mozilla.org/integration/mozilla-inbound/rev/37f2536e975e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120516204627
Crash:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2c3647738e81
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120516230826
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=37f2536e975e&tochange=2c3647738e81
Suspected: Bug 734082, Bug 614732
Reporter | ||
Updated•12 years ago
|
Summary: crash in nsRegion::Or → crash in nsRegion::Or (when I test Bug 766956, browser crashes when resize browser)
Reporter | ||
Comment 1•12 years ago
|
||
In local build
Last Good:d3b11e443f04
First Bad;05a339620439
This crash is triggered by Bug 734082
Blocks: 734082
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → jwatt
status-firefox15:
--- → affected
status-firefox16:
--- → affected
status-firefox17:
--- → affected
tracking-firefox15:
--- → ?
tracking-firefox16:
--- → ?
tracking-firefox17:
--- → ?
Component: SVG → Style System (CSS)
Assignee | ||
Updated•12 years ago
|
Component: Style System (CSS) → SVG
Updated•12 years ago
|
Comment 2•12 years ago
|
||
New reproducible crash in FF15, so tracking for upcoming versions. We'll likely backout bug 734082 or just wontfix if it comes down to the wire.
Assignee | ||
Updated•12 years ago
|
OS: Windows 7 → All
Hardware: x86 → All
Assignee | ||
Comment 3•12 years ago
|
||
In nsSVGOuterSVGFrame::Reflow we call nsSVGOuterSVGFrame::NotifyViewportOrTransformChanged if the size of the nsSVGOuterSVGFrame has changed. That in turn calls ChildrenOnlyTransformChanged on its children, which can then result in nsSVGUtils::ScheduleReflowSVG being called on descendants of the nsSVGOuterSVGFrame.
When we've entered nsSVGOuterSVGFrame::Reflow due to a resize, we have the issue that we reflow the reflow root for the nsSVGOuterSVGFrame, but then nsSVGUtils::ScheduleReflowSVG pushes that reflow root back onto PresShell::mDirtyRoots while we're under nsSVGOuterSVGFrame::Reflow. This would lead to an infinite reflow loop, except that NotifyViewportOrTransformChanged is only called if the size of the nsSVGOuterSVGFrame changes, which prevents that from happening.
What is happening when things go wrong here is that when the nsSVGOuterSVGFrame is reflowed it is first reflowed at a size assuming no scrollbars, then reflowed at a size that makes space for scrollbars. It's this oscillation that is particular to resizing that is causing us to always hit the |newViewportSize != svgElem->GetViewportSize()| block in nsSVGOuterSVGFrame::Reflow. We then end up with a stack overflow that contains the following four frames repeated:
PresShell::ProcessReflowCommands(bool)
PresShell::FlushPendingNotifications(mozFlushType)
PresShell::HandlePostedReflowCallbacks(bool)
PresShell::DidDoReflow(bool)
There's no need for nsSVGUtils::ScheduleReflowSVG to call PresShell()->FrameNeedsReflow() while we're under nsSVGOuterSVGFrame::Reflow since nsSVGOuterSVGFrame::Reflow goes on to call ReflowSVG on its anonymous child before returning. The simplest fix here is to stop it from doing that.
Assignee | ||
Comment 4•12 years ago
|
||
Hmm. We're already doing that.
Assignee | ||
Comment 5•12 years ago
|
||
Why does this list nothing?
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d3b11e443f04&tochange=05a339620439
Comment 6•12 years ago
|
||
> Why does this list nothing?
Because pushlog range listing sadly lists _pushes_ that are in the range, not changesets. And the two changesets delimiting the range are part of the same push.
Comment 7•12 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #2)
> New reproducible crash in FF15, so tracking for upcoming versions. We'll
> likely backout bug 734082 or just wontfix if it comes down to the wire.
At lsblakk's request, I tried backing out bug 734082 locally, but it's not a clean backout.
"hg backout" required manual file-merging, so I cancelled out of that and did the following:
hg up -r 05a339620439 # Go back in time to the cset we want to back out
hg revert revert -r d3b11e443f04 . # Revert to its parent
hg qnew backout-734082 # Save that reverting as a patch
hg qpop # Pop off the patch
hg up -r default # Go back to tip
hg qpush # Try to reapply the backout patch
The backout patch touches 33 files, and it had 15 hunks that failed to apply if I uses 8 lines of context, and 7 hunks that fail to apply if I use 3 lines of context. Fuzz fixed some, but not all, of those hunks (and with only 3 lines of context, I don't really trust patch-fuzz anyway).
So if we fix this by backing out bug 734082, it'll require some manual merge conflict resolution in at least a few hunks.
Comment 8•12 years ago
|
||
(sorry -- meant to say that previous comment was entirely about the beta branch.)
Comment 9•12 years ago
|
||
Thanks dholbert. Since bug 734082 can't be cleanly backed out I'm wontfixing this for 15 and hopefully jwatt can get this cleaned up on Aurora before merge day (8/27). Thankfully this reproducible regression is a very low volume crash at the moment, but let's get this fixed for 16.
Assignee | ||
Comment 10•12 years ago
|
||
The fundamental problem here is that we have a natural infinite loop due to the way SVG is sized and the way that adding/removing scrollbars changes the dimensions of the viewport into which the SVG is fit.
By way of example, lets say we have an SVG document containing the following markup:
<svg width="100%" viewBox="0 0 100 100">
<!-- ... -->
</svg>
The width attribute being set to 100% means that we should size the SVG to give it a width 100% of the viewport. The height property defaults to 'auto' and the viewBox attribute gives the SVG an intrinsic ratio of 1:1, so the SVG/CSS rules say to set the height of the SVG to the same value as its width.
Let's assume that adding a scrollbar reduces the size of the viewport by 10px.
To fit this SVG into a viewport that without scrollbars is 100px wide by 95px high we'd go through the following steps:
1) viewport = 100x95:
2) svg = 100x100
3) vertical scrollbar needed
4) viewport now = 90x95 after adding vertical scrollbar
5) svg now = 90x90
6) the vertical scrollbar is not needed
7) goto step 1, and loop to death
Comment 11•12 years ago
|
||
Why didn't we have the loop before bug 734082 landed?
Assignee | ||
Comment 12•12 years ago
|
||
The code in nsHTMLScrollFrame::ReflowContents is basically responsible for carrying out the "reflow with various scrollbars/no-scrollbars permutations" steps to try and find a suitable sizing solution. It's final fallback solution (the last TryLayout() call) is to reflow with both vertical and horizontal scrollbars enabled (assuming overflow in each respective direction is not hidden). That's basically what we want here - for sizing attempts to stop with scrollbars added even though they're unnecessary (since not having them produces a size that leaves content outside the viewport without a means to scroll to it).
As far as I can tell, the reason we go into a loop is because under some of the nsSVGOuterSVGFrame::Reflow() calls (triggered under nsHTMLScrollFrame::ReflowContents() as it tries reflowing at various sizes) we now hit the new nsLayoutUtils::PostRestyleEvent() call in nsSVGSVGElement::ChildrenOnlyTransformChanged(), which is passing nsChangeHint_UpdateOverflow. The stack for that:
nsSVGSVGElement::ChildrenOnlyTransformChanged
nsSVGOuterSVGFrame::NotifyViewportOrTransformChanged
nsSVGOuterSVGFrame::Reflow
nsContainerFrame::ReflowChild
nsCanvasFrame::Reflow
nsContainerFrame::ReflowChild
nsHTMLScrollFrame::ReflowScrolledFrame
nsHTMLScrollFrame::ReflowContents
nsHTMLScrollFrame::Reflow
nsContainerFrame::ReflowChild
ViewportFrame::Reflow
PresShell::DoReflow
PresShell::ProcessReflowCommands
When PresShell::DoReflow above returns, we then end up processing this restyle event under PresShell::DidDoReflow, and that in turn causes us to schedule a reflow under nsGfxScrollFrameInner::UpdateOverflow:
PresShell::FrameNeedsReflow
nsGfxScrollFrameInner::UpdateOverflow
nsHTMLScrollFrame::UpdateOverflow
nsCSSFrameConstructor::ProcessRestyledFrames
mozilla::css::RestyleTracker::ProcessOneRestyle
mozilla::css::RestyleTracker::DoProcessRestyles
mozilla::css::RestyleTracker::ProcessRestyles
nsCSSFrameConstructor::ProcessPendingRestyles
PresShell::FlushPendingNotifications
PresShell::HandlePostedReflowCallbacks
PresShell::DidDoReflow
PresShell::ProcessReflowCommands
Unfortunately when nsCSSFrameConstructor::ProcessPendingRestyles returns, PresShell::FlushPendingNotifications then goes on to call ProcessReflowCommands(), and hence we end up back in nsHTMLScrollFrame::ReflowContents, and down into ChildrenOnlyTransformChanged with the first stack given above, but one loop deeper. And so it goes on.
Assignee | ||
Comment 13•12 years ago
|
||
Attachment #656382 -
Flags: review?(roc)
Attachment #656382 -
Flags: review?(roc) → review+
Comment 14•12 years ago
|
||
bug 778492 is likely the same issue as this one.
Comment 15•12 years ago
|
||
bug 786693 might be this too.
Assignee | ||
Comment 17•12 years ago
|
||
Flags: in-testsuite+
Target Milestone: --- → mozilla18
Comment 18•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 19•12 years ago
|
||
We're getting fairly frequent crash hang reports on Linux that are fixed by this.
Comment 20•12 years ago
|
||
Please nominate for aurora/beta approval once comfortable with the testing this has received on trunk.
Assignee | ||
Comment 21•12 years ago
|
||
Comment on attachment 656382 [details] [diff] [review]
patch
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 734082
User impact if declined: crashes
Testing completed (on m-c, etc.): baked on m-c for several days
Risk to taking this patch (and alternatives if risky): no alternatives
String or UUID changes made by this patch: none
Attachment #656382 -
Flags: approval-mozilla-beta?
Attachment #656382 -
Flags: approval-mozilla-aurora?
Updated•12 years ago
|
Attachment #656382 -
Flags: approval-mozilla-beta?
Attachment #656382 -
Flags: approval-mozilla-beta+
Attachment #656382 -
Flags: approval-mozilla-aurora?
Attachment #656382 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 22•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/383e1f3f6636
https://hg.mozilla.org/releases/mozilla-beta/rev/0469860cebfb
Target Milestone: mozilla18 → ---
Comment 25•12 years ago
|
||
Mozilla/5.0 (X11; Linux i686; rv:16.0) Gecko/20100101 Firefox/16.0
Mozilla/5.0 (Windows NT 5.1; rv:16.0) Gecko/20100101 Firefox/16.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:16.0) Gecko/20100101 Firefox/16.0
Verified the fix on above builds (16beta3): no crashes occurred when zooming or resizing SVG images.
Updated•12 years ago
|
Target Milestone: --- → mozilla18
Comment 29•12 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0
Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17.0 Firefox/17.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/17.0 Firefox/17.0
Fix is verified on above builds (17.0beta1), as well: no crashes occur when zooming/resizing SVGs.
QA Contact: mihaela.velimiroviciu
You need to log in
before you can comment on or make changes to this bug.
Description
•