1229850 - [e10s] crash in nsComboboxControlFrame::GetAvailableDropdownSpace

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This bug was filed from the Socorro interface and is 
report bp-d0a8f47f-7665-4a2b-b124-70a5e2151202.
=============================================================

These crashes have a crash address of 0xfffffffff0de801b or 0xffffffffffffffff.

These crashes are occurring only on nightly and aurora -- but they started happening at any decent frequency on nightly and aurora on the same date, 2015-10-15, which makes me think the cause might be some Web content change in the wild.  Though there were 3 crashes on nightly in the few months prior to that.

It seems possible they'll start showing up on beta at the next merge, although the behavior of starting on aurora and nightly on the same date is pretty odd, so I'm not entirely sure.

It seems worth looking into, especially if the crash is obvious from one of the crash reports.

Comment 1

[Jonathan Kew [:jfkthame]]

I notice that there's at least one report from a build as far back as 41.0a1 (although it occurred within the past week -- someone was running an old build: 20150526030202). So it's apparently not a particularly recent regression in Gecko, though as you suggest, it may be triggered by something that's recently appeared on the web.

FWIW, I also notice that all the crashes appear to be with e10s enabled. Don't know if that's at all significant.

> These crashes have a crash address of 0xfffffffff0de801b or 0xffffffffffffffff.

It looks like the former occurs in 32-bit Windows builds, and the latter in 64-bit Windows builds.

There are also a few (32-bit) Linux crashes, with an address 0xf0dea81b that looks eerily similar (but not identical) to the 32-bit Windows one. But I don't know what it signifies, if anything.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think f0de801b is a small offset from the default frame poison address:
http://hg.mozilla.org/mozilla-central/file/f6ac392322b3/mfbt/Poison.cpp#l164

The e10s correlation makes the nightly/aurora thing make more sense.

Comment 4

[Jim Mathies [:jimm]]

2015110300  1
2015111906  3
2015112000  2
2015112100  2
2015112300  42
2015112400  1
2015112500  20
2015112503  2
2015112600  24
2015112603  2
2015112700  5
2015112800  4
2015112900  5
2015112903  3
2015113000  3
2015113003  16
2015120100  49
2015120103  78
2015120200  135
2015120203  74
2015120211  60
2015120300  14
2015120303  16
2015120305  19

11-22: abbd213422a560f1180c4ec6e3bf4792c2ea81ba

11-23: 8b1fc0961a076e35646d0472a81feefc0074558c

22 -> 23: 
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=abbd213422a560f1180c4ec6e3bf4792c2ea81ba&tochange=8b1fc0961a076e35646d0472a81feefc0074558c

11-24: 45273bbed8efaface6f5ec56d984cb9faf4fbb6a

11-25: 099f695d31326c39595264c34988a0f4b7cbc698

11-26: c321d84038519dcf1670d59fd2c5c00ad8a85a55

11-27: 47b49b0d32360fab04b11ff9120970979c426911

11-28: 47b49b0d32360fab04b11ff9120970979c426911

11-29: 47b49b0d32360fab04b11ff9120970979c426911

11-30: 47b49b0d32360fab04b11ff9120970979c426911

26 -> 30:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c321d84038519dcf1670d59fd2c5c00ad8a85a55&tochange=47b49b0d32360fab04b11ff9120970979c426911

12-01: 66a6d7ec9534b9d7847b665142fef0dd87623768

30 -> 01:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=47b49b0d32360fab04b11ff9120970979c426911&tochange=66a6d7ec9534b9d7847b665142fef0dd87623768

Comment 5

[Jim Mathies [:jimm]]

Not much stands out here.

Comment 6

[Jim Mathies [:jimm]]

tracing that PresContext() call - 

http://hg.mozilla.org/mozilla-central/annotate/59c648a3f955/layout/forms/nsComboboxControlFrame.cpp#l599

599   nsPresContext* pc = PresContext()->GetToplevelContentDocumentPresContext();

http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsIFrame.h#433

433   nsPresContext* PresContext() const {
434     return StyleContext()->PresContext();
435   }

http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsIFrame.h#542

542   nsStyleContext* StyleContext() const { return mStyleContext; }

http://mxr.mozilla.org/mozilla-central/source/layout/style/nsStyleContext.h#135

135   nsPresContext* PresContext() const { return mRuleNode->PresContext(); }

582   // The rule node is the node in the lexicographic tree of rule nodes
583   // (the "rule tree") that indicates which style rules are used to
584   // compute the style data, and in what cascading order.  The least
585   // specific rule matched is the one whose rule node is a child of the
586   // root of the rule tree, and the most specific rule matched is the
587   // |mRule| member of |mRuleNode|.
588   nsRuleNode* const       mRuleNode;

http://mxr.mozilla.org/mozilla-central/source/layout/style/nsRuleNode.h#885

884   // NOTE: Does not |AddRef|.  Never null.
885   nsPresContext* PresContext() const { return mPresContext; }

Comment 7

[Jim Mathies [:jimm]]

bz, curious if you could take a look at these push ranges to see if anything stands out to you layout related? This crash doesn't appear to be e10s specific, although it only shows up in the content process. A nsRuleNode or the nsPresContext in a nsRuleNode is invalid when it shouldn't be.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Nothing jumping out at me offhand.  There are various style system and layout changes in http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=47b49b0d32360fab04b11ff9120970979c426911&tochange=66a6d7ec9534b9d7847b665142fef0dd87623768 but not obviously related to comboboxes.

Of course there are lots of changesets in all those that could have some bug that corrupts memory or whatnot...

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Not sure why that push range is interesting.  See regression range info in comment 0.

(Note that crash-stats defaults to showing only crashes in the past week, but you can change that by adding date > 2015-07-01 or similar.)

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

To see the data:

Crashes on nightly:
https://crash-stats.mozilla.com/signature/?product=Firefox&signature=nsComboboxControlFrame%3A%3AGetAvailableDropdownSpace&date=%3E2015-07-01&release_channel=nightly#aggregations

and crashes on aurora:
https://crash-stats.mozilla.com/signature/?product=Firefox&signature=nsComboboxControlFrame%3A%3AGetAvailableDropdownSpace&date=%3E2015-07-01&release_channel=aurora#aggregations

have both spiked around the beginning of December.  So I think the variation in crash rates is related to Web content changes.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Note that I consider the e10s theory particularly credible because comboboxes work differently in e10s (from bug 897060).

Comment 12

[Jim Mathies [:jimm]]

A list of top sites affected by this might help.

Comment 13

[Tracy Walker [:tracy]]

facebook.com and google search queries account for the bulk of reports.  dbaron hid my previous post, for what he felt were privacy concerns (comment 3) that listed more specifically the urls.  I can send you a detailed list, but I don't think it will be any more useful.

Comment 14

[Jim Mathies [:jimm]]

Attachment: patch

I think the most straight forward fix here is to avoid doing these unnecessary drop down calculations in the content process. This will avoid the crash because the code involved never gets called.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=54b8ed7a562b

Comment 15

[Jim Mathies [:jimm]]

Comment on attachment 8700122 [details] [diff] [review]
patch

Looks good on try and should avoid this crash by avoiding executing drop down calculations code in the content process. We don't need to do this here, the drop down is managed in chrome.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8700122 [details] [diff] [review]
patch

I wonder if nsListControlFrame::ReflowAsDropdown could do even less work, but this looks fine for now.

r=dbaron

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/90d5c8f1fa4b

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/90d5c8f1fa4b

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Crash is gone from crash-stats in today's nightly, based on links in http://dbaron.org/mozilla/crashes-by-build .  Thanks.

Comment 20

[Jim Mathies [:jimm]]

Comment on attachment 8700122 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]:
e10s combo box drop down work
[User impact if declined]:
crashed tabs
[Describe test coverage new/current, TreeHerder]:
on mc since Dec. 23rd.
[Risks and why]:
pretty low risk, e10s specific, patch shunts some code that shouldn't run. no fallout detected thus far on mc.
[String/UUID change made/needed]:
none

Comment 21

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8700122 [details] [diff] [review]
patch

Fix a crash, taking it.

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ea159f50b4c7