Closed Bug 890309 Opened 8 years ago Closed 8 years ago

crash in mozilla::dom::Element::FontSizeInflation @ nsIFrame::PresContext

Categories

(Core :: DOM: Core & HTML, defect)

24 Branch
x86
Windows 7
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla25
Tracking Status
firefox23 --- unaffected
firefox24 --- affected
firefox25 --- verified

People

(Reporter: Honza, Assigned: jwir3)

References

Details

(Keywords: crash, regression, reproducible, Whiteboard: [firebug-p1])

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is 
report bp-0796a4d5-ddb3-4c2f-b256-8c67a2130704 .
 ============================================================= 

I don't have any solid STR with a clean profile, but I can provide my existing profile (zipped) with Firebug installed where I can easily reproduce it.

In any case, I am experiencing the crash a lot.

Honza
It first showed up in 25.0a1/2013070211 so most likely a regression from bug 878931.
Blocks: 878931
Component: XPConnect → DOM
Keywords: regression
OS: Windows NT → Windows 7
Summary: crash in nsIFrame::PresContext() → crash in mozilla::dom::Element::FontSizeInflation @ nsIFrame::PresContext
Version: 20 Branch → 24 Branch
I also saw this crash right now[1] when I wanted to display an object logged to FBTrace.
I can reproduce this constantly, so if you don't have a test case, I could provide one. Right now I just don't have the time to do so.

Bad side effect of this crash:
All my tabs are closed and there's no way to restore the previous session.
I guess that's related to bug 791195 and bug 806114.

Sebastian

[1] https://crash-stats.mozilla.com/report/index/fe0e8d08-8699-4a2a-8435-912582130705
Hm, so this is quite strange. It seems that the presContext is null in the situation where we're hitting it. I'll add in another check for a null prescontext and see if that alleviates the problem.
Assignee: nobody → sjohnson
Even more strange is the fact that this is happening on windows. It should only be included in the browser.js code for android, and even then, it shouldn't be enabled via prefs on windows.
Attached patch b890309 (obsolete) — Splinter Review
I think we're getting into a situation where we're initializing (or tearing down) a frame, causing one of mStyleContext, mRuleNode, or StyleContext()->RuleNode()->PresContext() to return nullptr.

After looking through this code, I actually think that nsIFrame::StyleContext() should be nsIFrame::GetStyleContext(), since I believe it can return nullptr if the frame hasn't yet called DidSetStyleContext() from within Init().
Attachment #772081 - Flags: review?(mounir)
Comment on attachment 772081 [details] [diff] [review]
b890309

Review of attachment 772081 [details] [diff] [review]:
-----------------------------------------------------------------

I think a layout person would be more appropriate than me to review this change.
Attachment #772081 - Flags: review?(mounir) → review?(dbaron)
Comment on attachment 772081 [details] [diff] [review]
b890309

Not quite ready for review yet. I want to double-check this actually fixes the problem.
Attachment #772081 - Flags: review?(dbaron)
(In reply to Sebastian Zartner from comment #2)
> I also saw this crash right now[1] when I wanted to display an object logged
> to FBTrace.
> I can reproduce this constantly, so if you don't have a test case, I could
> provide one. Right now I just don't have the time to do so.

Sebastian:

It would be helpful if you can provide a test case. I see that there are some crashes on Windows 7, and I think I have a reasonable guess as to what the problem is, but it would be good if I was able to consistently reproduce the problem (and thus verify that it's fixed).
Here are my steps:

1. Install Firebug from https://getfirebug.com/releases/firebug/1.12/firebug-1.12.0b2.xpi
2. Install FBTrace from https://getfirebug.com/releases/fbtrace/1.12/fbTrace-1.12b3.xpi
3. Restart the browser
4. Go to https://getfirebug.com
5. Open Firebug via F12
6. Open the FBTrace console via Shift+R
7. Inside the FBTrace console switch to the "Options" tab
8. Click the "WATCH" button
9. Switch back to the "Logs" tab
10. Inside Firebug enable and switch to the Script panel
11. Reload the page
12. Switch to the Watch side panel
13. Click the "New watch expression..." field
14. Type "test" and hit Enter
    => A new watch expression will be created.
15. Remove the watch expression again via the X at the right side of it
    => Inside the FBTrace console there should be an entry "Firebug.WatchPanel.deleteWatch"
16. Click the "Firebug.WatchPanel.deleteWatch" entry to expand it
17. Click the "Object" tab for the entry

=> The browser crashes with that error signature.

Sorry that they are quite complicated, but this should hopefully at least allow you to reproduce it. Tested on FF 25.0a1. If anything is unclear, please let me know.
I'll try to find simpler steps in the meantime.

Sebastian
Keywords: reproducible
Thanks for the steps, Sebastian. I'm not getting the same crash as you, though. What version of Windows are you using? I'm on Windows 7, which is equivalent to windows NT 6.2, I think - there are crash signatures with this OS, so I would expect it to crash...
Aha... I spoke too soon. I just updated nightly and am now seeing the crash as described. Let me see if my patch fixes it.
> I just updated nightly and am now seeing the crash as described.
Good.

> What version of Windows are you using?
Currently Win8[1] but in comment 2 it was on Win7.

> I'm on Windows 7, which is equivalent to windows NT 6.2, I think
6.2 is Win8. Win7 is 6.1.[2]

Sebastian

[1] https://crash-stats.mozilla.com/report/index/6d65d68a-f4b6-4a3e-9a27-ebb532130709
[2] https://en.wikipedia.org/wiki/Windows_NT#Releases
(In reply to Scott Johnson (:jwir3) from comment #3)
> Hm, so this is quite strange. It seems that the presContext is null in the
> situation where we're hitting it. I'll add in another check for a null
> prescontext and see if that alleviates the problem.

In general layout code assumes that frame->PresContext() is non-null is frame is non-null, so we should probably understand the root cause of this issue.
(In reply to Timothy Nikkel (:tn) from comment #13)
> (In reply to Scott Johnson (:jwir3) from comment #3)
> > Hm, so this is quite strange. It seems that the presContext is null in the
> > situation where we're hitting it. I'll add in another check for a null
> > prescontext and see if that alleviates the problem.
> 
> In general layout code assumes that frame->PresContext() is non-null is
> frame is non-null, so we should probably understand the root cause of this
> issue.

Yeah, so the pres context is non-null (I can tell from the frame's parent). The problem is that mStyleContext->mRootNode is null, which doesn't seem to be a situation that should be possible.
I get the following situation when I look at the style context of the frame in question:

(gdb) p *frame->StyleContext()
$11 = {
  mParent = 0x7ffff6238530 <vtable for nsNodeInfo+16>, 
  mChild = 0x7fffcd8a0800, 
  mEmptyChild = 0x7fffe3d31d00, 
  mPrevSibling = 0x0warning: (Internal error: pc 0x0 in read in psymtab, but not in symtab.)

, 
  mNextSibling = 0xa5a5000100000003, 
  mStyleIfVisited = {
    mRawPtr = 0x0warning: (Internal error: pc 0x0 in read in psymtab, but not in symtab.)


  }, 
  mPseudoTag = {
    mRawPtr = 0x0warning: (Internal error: pc 0x0 in read in psymtab, but not in symtab.)


  }, 
  mRuleNode = 0x0warning: (Internal error: pc 0x0 in read in psymtab, but not in symtab.)

, 
  mAllocations = 0x7fffcd9d4e60, 
  mCachedResetData = 0x7ffff64ca3a8 <_ZL12table_buffer+8>, 
  mCachedInheritedData = {
    mStyleStructs = {
      mArray = {0x500000005, 0x7fffccf50628, 0x500000005, 0x7ffff64ca3a8 <_ZL12table_buffer+8>, 0x500000005, 0xdwarning: (Internal error: pc 0xd in read in psymtab, but not in symtab.)

, 0x7ffff6a3f370, 
        0xa5a5a5a5a5a5a5a5, 0x7ffff6238530 <vtable for nsNodeInfo+16>}
    }
  }, 
  mBits = 3448375296, 
  mRefCnt = 32767
}
Oh, interesting... I guess the primary frame is an HTMLTableElement:

(gdb) p this->mPrimaryFrame
$14 = (mozilla::dom::HTMLTableElement *) 0x7fffc89a82a0
(In reply to Scott Johnson (:jwir3) from comment #16)
> Oh, interesting... I guess the primary frame is an HTMLTableElement:
> 
> (gdb) p this->mPrimaryFrame
> $14 = (mozilla::dom::HTMLTableElement *) 0x7fffc89a82a0

Ok, I think I see the problem. I should be using GetPrimaryFrame() instead of mPrimaryFrame to refer to the primary frame. This does a check to determine if the element is in a document or not, and if not, returns null (which I already had a check for).
Attached patch b890309 (v2)Splinter Review
Retrieves the correct frame for the element in question when not in a document.
Attachment #772081 - Attachment is obsolete: true
Attachment #773399 - Flags: review?(tnikkel)
Comment on attachment 773399 [details] [diff] [review]
b890309 (v2)

Good catch.

No one should really be using mPrimaryFrame directly except for a few special users, it would be nice if we could somehow enforce that but making it private won't work because some of those special uses are in a derived class.
Attachment #773399 - Flags: review?(tnikkel) → review+
https://hg.mozilla.org/mozilla-central/rev/ca07a06eab8f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Works for me. Yay! Thanks guys!

Sebastian
Verified fixed based on comment 22, thanks Sebastian.
Status: RESOLVED → VERIFIED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.