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

Status: VERIFIED FIXED

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

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

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

Comment 1

[Scoobidiver (away)]

It first showed up in 25.0a1/2013070211 so most likely a regression from bug 878931.

Comment 2

[Sebastian Zartner [:sebo]]

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

Comment 3

[Scott Johnson (:jwir3)]

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.

Comment 4

[Scott Johnson (:jwir3)]

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.

Comment 5

[Scott Johnson (:jwir3)]

Attachment: b890309

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().

Comment 6

[Mounir Lamouri (: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.

Comment 7

[Scott Johnson (:jwir3)]

Comment on attachment 772081 [details] [diff] [review]
b890309

Not quite ready for review yet. I want to double-check this actually fixes the problem.

Comment 8

[Scott Johnson (:jwir3)]

(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).

Comment 9

[Sebastian Zartner [:sebo]]

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

Comment 10

[Scott Johnson (:jwir3)]

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...

Comment 11

[Scott Johnson (:jwir3)]

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.

Comment 12

[Sebastian Zartner [:sebo]]

> 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

Comment 13

[Timothy Nikkel (:tnikkel)]

(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.

Comment 14

[Scott Johnson (:jwir3)]

(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.

Comment 15

[Scott Johnson (:jwir3)]

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
}

Comment 16

[Scott Johnson (:jwir3)]

Oh, interesting... I guess the primary frame is an HTMLTableElement:

(gdb) p this->mPrimaryFrame
$14 = (mozilla::dom::HTMLTableElement *) 0x7fffc89a82a0

Comment 17

[Scott Johnson (:jwir3)]

(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).

Comment 18

[Scott Johnson (:jwir3)]

Attachment: b890309 (v2)

Retrieves the correct frame for the element in question when not in a document.

Comment 19

[Timothy Nikkel (: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.

Comment 20

[Scott Johnson (:jwir3)]

Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca07a06eab8f

Comment 21

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ca07a06eab8f

Comment 22

[Sebastian Zartner [:sebo]]

Works for me. Yay! Thanks guys!

Sebastian

Comment 23

[u279076]

Verified fixed based on comment 22, thanks Sebastian.