938226 - crash in nsHtml5TreeBuilder::flushCharacters()

Status: RESOLVED WORKSFORME

(Core :: DOM: HTML Parser, defect)

Description

[Tracy Walker [:tracy]]

This bug was filed from the Socorro interface and is 
report bp-3d4ea2e4-6a14-4414-b575-f9e1a2131113.
=============================================================

Highly correlated with Infoaxe add-on:
98% (244/249) vs.   0% (437/105672) {3EB3C1FE-4FED-4ef7-A78C-6616E2521FB5} (Infoaxe: Full Text Web History Search synchronized between Firefox, IE on multiple computers., https://addons.mozilla.org/addon/9412)

Reports on Windows and Mac are climbing up the charts.

Frame 	Module 	Signature 	Source
0 	xul.dll 	nsHtml5TreeBuilder::flushCharacters() 	parser/html/nsHtml5TreeBuilder.cpp
1 	xul.dll 	nsHtml5TreeBuilder::Flush(bool) 	parser/html/nsHtml5TreeBuilderCppSupplement.h
2 	xul.dll 	nsHtml5StreamParser::MarkAsBroken() 	parser/html/nsHtml5StreamParser.cpp
3 	xul.dll 	nsHtml5StreamParser::WriteStreamBytes(unsigned char const *,unsigned int,unsigned int *) 	parser/html/nsHtml5StreamParser.cpp

Comment 1

[Tracy Walker [:tracy]]

many reports on Fx25, Fx26 and 2 on Fx28  (I'm assuming 27 is affected, just no one using the suspected addon with aurora yet).

Comment 2

[u279076]

Definitely appears to be rising (at least in Release and Beta).

7-day Rank:
 * Release: #61  (0.14%)
 * Beta:    #25  (0.23%)
 * Aurora:  #234 (0.04%)
 * Nightly: N/A

3-day Rank:
 * Release: #28  (0.24%)
 * Beta:    #13  (0.45%)
 * Aurora:  #131 (0.09%)
 * Nightly: #232 (0.04%)

1-day Rank:
 * Release: #24  (0.28%)
 * Beta:    #9   (0.52%)
 * Aurora:  N/A
 * Nightly: N/A

I think this is on the virge of becoming a topcrash so I'm adding the keyword.

Comment 3

[u279076]

More reports:
https://crash-stats.mozilla.com/report/list?signature=nsHtml5TreeBuilder%3A%3AflushCharacters%28%29

Comment 4

[u279076]

I'm seeing a lot of correlations to http://static.flipora.com/websearch.html in the URLs.

Comment 5

[u279076]

I'm not sure if this is relevant but many comments mention Flipora hijacking their search preferences. Also the top hits for "infoaxe" on Google talk about how to remove this software. I did manage to get the add-on installed from their website but I've yet to find a way to get it to hijack any preferences as of yet. I've also not reproduced any crashes. Do we know what sorts of user/website behaviours can trigger nsHtml5TreeBuilder::flushCharacters()?

Comment 6

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

https://crash-stats.mozilla.com/report/index/3b7c42d7-32f7-4d62-8ac9-960de2131114 implicates bug 920725.

(In reply to Anthony Hughes, QA Mentor (:ashughes) [On vacation, Nov 15-Dec 3] from comment #5)
> Do we know what
> sorts of user/website behaviours can trigger
> nsHtml5TreeBuilder::flushCharacters()?

All kinds all the time.

However, the stack I linked to above suggests that the crash happens as part of the failure condition that was papered over in bug 920725.

Comment 7

[Steve Workman [:sworkman] (INACTIVE)]

The stack trace does seem to be closely related. Bug 920725 also had mentions of Flipora.

I'll take a look at a minidump to see if I can find out more info.

Comment 8

[Steve Workman [:sworkman] (INACTIVE)]

(In reply to Steve Workman [:sworkman] from comment #7)
> I'll take a look at a minidump to see if I can find out more info.

Minidump not super helpful - no local variables available in flushCharacters. And I'm afraid that nsHtml5TreeBuilder is outside my expertise, so I can't make a guess re a defensive/speculative fix, sorry.

Comment 9

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

We really need to track down the cause of bug 920725 and fix the root cause. It's too late if we get to flushCharacters with corrupt memory.

Comment 10

[u279076]

This signature has fallen out of the 7-day topcrash range:
 * Firefox 29: N/A in the top-300
 * Firefox 28: #90 @ 0.16% with 17 crashes 
 * Firefox 27: #43 @ 0.24% with 557 crashes
 * Firefox 26: #22 @ 0.43% with 2654 crashes

Given this information I'm not sure this qualifies as a topcrash anymore. Henri, is there anything QA can do to assist with the investigation you're suggesting in comment 9? We should probably have another bug to track that investigation.

Comment 11

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #10)
> Henri, is there anything QA can do to assist with the investigation you're
> suggesting in comment 9?

Probably not, since I expect it to be a thread interaction issue, so I don't expect stacks to tell an interesting story.

I expect the most effective method of investigation to be looking at the patches from bug 497003 very, very carefully.

Comment 12

[u279076]

(In reply to Henri Sivonen (:hsivonen) from comment #11)
> I expect the most effective method of investigation to be looking at the
> patches from bug 497003 very, very carefully.

Should we take care of that in a separate bug which blocks this one?

Comment 13

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #12)
> (In reply to Henri Sivonen (:hsivonen) from comment #11)
> > I expect the most effective method of investigation to be looking at the
> > patches from bug 497003 very, very carefully.
> 
> Should we take care of that in a separate bug which blocks this one?

Yeah, that's probably the best way to deal with this.

Comment 14

[u279076]

Would you mind filing that bug for me? I'm not sure how to articulate what's needed.

Comment 15

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #14)
> Would you mind filing that bug for me? I'm not sure how to articulate what's
> needed.

Filed bug 960519.

Comment 16

[u279076]

Thanks Henri. I'm removing topcrash from this bug given we now have a blocker for investigation and based on the stats from comment 10.

Comment 17

[Tracy Walker [:tracy]]

This has made its way back into topcrash-win on 30beta.  The fix for the depends bug 960519 landed in 31 and appears to have fixed this there. Should that fix be uplifted to 30beta?  I'm asking here because this is the bug containing the crash signature in question.

Comment 18

[Robert Kaiser]

Henri, can you answer comment #17?

Comment 19

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

(In reply to [:tracy] Tracy Walker - QA Mentor from comment #17)
> This has made its way back into topcrash-win on 30beta.  The fix for the
> depends bug 960519 landed in 31 and appears to have fixed this there. Should
> that fix be uplifted to 30beta?

I guess that would make sense. I'll have to check if the patch applies.

Comment 20

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking and also looking for the uplift in bug 960519 before Thursday's Beta.

Comment 21

[Lukas Blakk [:lsblakk] use ?needinfo]

Currently at #13 in the topcrash list - unfortunately there was no time to land the uplift mentioned in comment 20 so we have to wait one more cycle to see this fixed.  Will hope that this remains relatively low (< 10) in release population for 6 weeks.

Comment 22

[Robert Kaiser]

The signature still exists in current releases, but at low level. The ni? itself isn't really relevant any more, though.

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

Since the crash volume is low (less than 5 per week), the severity is downgraded to S3. Feel free to change it back if you think the bug is still critical.

For more information, please visit auto_nag documentation.

Comment 24

[BugBot [:suhaib / :marco/ :calixte]]

Closing because no crashes reported for 12 weeks.