Note: There are a few cases of duplicates in user autocompletion which are being worked on.

[FIX] Crash with float, columns, and a feed [@ nsFontMetricsMac::Init] [@ nsUnicodeFontMappingMac::GetFontID] [@ nsIFrame::AddStateBits] [@ nsTextStyle::nsTextStyle]

RESOLVED FIXED

Status

()

Core
Layout
--
critical
RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: Mats Palmgren (vacation - back in August))

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
crash, testcase, verified1.8.0.7, verified1.8.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 +
blocking1.8.0.7 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical], crash signature)

Attachments

(5 attachments)

(Reporter)

Description

11 years ago
This testcase causes a crash with a varying signature and sometimes with a random address on top.
(Reporter)

Comment 1

11 years ago
Created attachment 221578 [details]
testcase
(Reporter)

Updated

11 years ago
Flags: blocking1.9a2?
Whiteboard: [sg:critical]
(Reporter)

Comment 2

11 years ago
I'm curious why it matters that the page has a feed.
(Reporter)

Comment 3

11 years ago
Still crashes a trunk nightly on Mac, Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060602 Minefield/3.0a1.

Does not crash on Windows.
Summary: Crash with float, columns, and a feed [@ nsFontMetricsMac::Init] [@ nsUnicodeFontMappingMac::GetFontID] [@ nsIFrame::AddStateBits] → Crash with float, columns, and a feed [@ nsFontMetricsMac::Init] [@ nsUnicodeFontMappingMac::GetFontID] [@ nsIFrame::AddStateBits] [@ nsTextStyle::nsTextStyle]
(Reporter)

Comment 4

11 years ago
Created attachment 224297 [details]
A stack trace with nsTextStyle::nsTextStyle on top
(Reporter)

Comment 5

11 years ago
Also crashes on Linux, at least using a build modified for use with Valgrind.  I'll attach a Valgrind log shortly.
(Reporter)

Comment 6

11 years ago
Created attachment 224298 [details]
Valgrind log
This looks like a bit of a mess -- CreateContinuingFrame often doesn't set its out parameter when it returns failure, but callers check the otherwise uninitialized out parameter rather than the nsresult.

It's also worth figuring out why it's failing, though.
(Reporter)

Comment 8

11 years ago
Still crashes on trunk, despite recent fixes for column-related crashes.
OS: Mac OS X 10.4 → All
Hardware: Macintosh → All
(Reporter)

Comment 9

11 years ago
Now it crashes even without the feed, and fires this assertion shortly before crashing:

###!!! ASSERTION: unexpected frame type: 'PR_FALSE', file /Users/admin/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 11152
(Reporter)

Comment 10

11 years ago
CCing Mats, who fixed bug 348688, in the hope that he'll be interested in fixing this float-related crash as well.
Assignee: nobody → mats.palmgren
Flags: blocking1.8.1?
Flags: blocking1.8.0.7?
Summary: Crash with float, columns, and a feed [@ nsFontMetricsMac::Init] [@ nsUnicodeFontMappingMac::GetFontID] [@ nsIFrame::AddStateBits] [@ nsTextStyle::nsTextStyle] → [FIX] Crash with float, columns, and a feed [@ nsFontMetricsMac::Init] [@ nsUnicodeFontMappingMac::GetFontID] [@ nsIFrame::AddStateBits] [@ nsTextStyle::nsTextStyle]
Created attachment 234872 [details] [diff] [review]
Patch A, rev. 1

1. Make all CreateContinuingFrame() callers check the returned error code.
This patch fixes the crash but the float is not rendered.
I think it's a safe fix for branches.

The reason we fail to render the float is that the out-fo-flow is a
nsHTMLScrollFrame which we can't create a continuation for.
So with this patch we still get these two assertions:
###!!! ASSERTION: unexpected frame type: 'Not Reached', file nsCSSFrameConstructor.cpp, line 11185
###!!! ASSERTION: reflow dirty lines failed: 'NS_SUCCEEDED(rv)', file nsBlockFrame.cpp, line 916

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockReflowState.cpp&rev=3.514&root=/cvsroot&mark=654-661#623
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockFrame.cpp&rev=3.787&root=/cvsroot&mark=4231,4240,4242-4243#4230
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockFrame.cpp&rev=3.787&root=/cvsroot&mark=4318,4322#4317
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsHTMLContainerFrame.cpp&rev=3.217&root=/cvsroot&mark=338,351-352#337
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1251&root=/cvsroot&mark=11119-11132,11152-11153#11118
Attachment #234872 - Flags: superreview?(bzbarsky)
Attachment #234872 - Flags: review?(bzbarsky)
Created attachment 234873 [details] [diff] [review]
Patch B, rev. 1

1. Make scroll frames non-splittable.
2. Make a placeholder splittable only if the out-of-flow is.
3. If a float placeholder is not splittable then place it immediately
   instead of failing to reflow it because it can't be split.

This patch makes us render the testcase without any assertions
(together with Patch A), but is probably more risky.


BTW, maybe we should change:
  NS_IMETHOD IsSplittable(nsSplittableType& aIsSplittable) const;
to
  nsSplittableType GetSplittableType() const;

It's only used within layout/ and always return NS_OK as far as I can see.
Attachment #234873 - Flags: superreview?(roc)
Attachment #234873 - Flags: review?(bzbarsky)

Comment 13

11 years ago
roc should probably just review all of these...
Comment on attachment 234872 [details] [diff] [review]
Patch A, rev. 1

+  nsresult rv = aPresContext.PresShell()->FrameConstructor()->
     CreateContinuingFrame(&aPresContext, &aRowFrame, this, aContRowFrame);
-  if (!*aContRowFrame) return;
+  if (NS_FAILED(rv)) {
+    return;
+  }

Set *aContRowFrame to nsnull before this return, and check callers to make sure they check that.
Attachment #234872 - Flags: superreview?(bzbarsky)
Attachment #234872 - Flags: superreview+
Attachment #234872 - Flags: review?(bzbarsky)
Attachment #234872 - Flags: review+
Comment on attachment 234873 [details] [diff] [review]
Patch B, rev. 1

looks good
Attachment #234873 - Flags: superreview?(roc)
Attachment #234873 - Flags: superreview+
Attachment #234873 - Flags: review?(bzbarsky)
Attachment #234873 - Flags: review+
(In reply to comment #14)
> Set *aContRowFrame to nsnull before this return, and check callers
> to make sure they check that.

Good point.  Patch A with the additional null assignment checked in
to trunk at 2006-08-21 18:33 PDT. (I'll hold Patch B for 24h to get nightlies)

Updated

11 years ago
Flags: blocking1.8.1? → blocking1.8.1+
Flags: blocking1.8.0.7? → blocking1.8.0.7+
Attachment #234872 - Flags: approval1.8.1?
Attachment #234872 - Flags: approval1.8.0.7?
"Patch B" checked in to trunk at 2006-08-23 22:22 PDT.

Filed bug 349973 on changing IsSplittable() signature.

-> FIXED
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Depends on: 350046

Updated

11 years ago
Whiteboard: [sg:critical] → [sg:critical] [baking until 8/25]

Comment 18

11 years ago
Comment on attachment 234872 [details] [diff] [review]
Patch A, rev. 1

 a=schrep for drivers
Attachment #234872 - Flags: approval1.8.1? → approval1.8.1+
Comment on attachment 234872 [details] [diff] [review]
Patch A, rev. 1

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #234872 - Flags: approval1.8.0.7? → approval1.8.0.7+
Patch A: checked into to MOZILLA_1_8_BRANCH at 2006-08-25 13:21 PDT.
Keywords: fixed1.8.1
Patch A: checked in to MOZILLA_1_8_0_BRANCH at 2006-08-25 15:36 PDT.
Keywords: fixed1.8.0.7

Comment 22

11 years ago
v.fixed on both 1.8.0 and 1.8.1 branches with:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7) Gecko/20060906 Firefox/1.5.0.7
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1b2) Gecko/20060907 BonEcho/2.0b2
Keywords: fixed1.8.0.7, fixed1.8.1 → verified1.8.0.7, verified1.8.1
Group: security
Flags: in-testsuite?
(Reporter)

Comment 23

10 years ago
Crashtest checked in.
Flags: in-testsuite?
Flags: in-testsuite+
Flags: blocking1.9a2?
Whiteboard: [sg:critical] [baking until 8/25] → [sg:critical]
Crash Signature: [@ nsFontMetricsMac::Init] [@ nsUnicodeFontMappingMac::GetFontID] [@ nsIFrame::AddStateBits] [@ nsTextStyle::nsTextStyle]
You need to log in before you can comment on or make changes to this bug.