861610 - ASSERTION: Map should be empty already: 'clearedEntries == 0' running content/base/crashtests

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Simon Montagu :smontagu]

+++ This bug was initially created as a clone of Bug #861607 +++

With the patches from bug 861606 and bug 861607, running crashtests in content/base/crashtests still asserts:

REFTEST TEST-UNEXPECTED-FAIL | file:///home/smontagu/mozwork/hgtree/mozilla/content/base/crashtests/831287.html | assertion count 1 is more than expected 0 assertions
REFTEST TEST-UNEXPECTED-FAIL | file:///home/smontagu/mozwork/hgtree/mozilla/content/base/crashtests/849727.html | assertion count 1 is more than expected 0 assertions

Both assertions are:
 ASSERTION: Map should be empty already: 'clearedEntries == 0', file /home/smontagu/mozwork/hgtree/mozilla/content/base/src/DirectionalityUtils.cpp, line 551

Comment 2

[Simon Montagu :smontagu]

Attachment: Patch

When we are setting dir to something other than auto, and the element has dirAutoSet, it must have had dir=auto before so we need to clear it from TextNodeDirectionalityMap.

Comment 3

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5af257307e2c

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5af257307e2c

Comment 5

[Simon Montagu :smontagu]

Comment on attachment 737219 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 548206 or one of its followups
User impact if declined: without bug 861606 this could lead to sec-crit vulnerabilities. No such vulnerabilities are known at the time of writing, but they could probably be constructed as variations on earlier heap use after free bugs. With bug 861606 its impact is mitigated, but since risk is very low I'd like to get this in for the first release of dir=auto
Testing completed (on m-c, etc.): baked on m-c since 2013-04-15
Risk to taking this patch (and alternatives if risky): minimal
String or IDL/UUID changes made by this patch: none

Comment 6

[Simon Montagu :smontagu]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0d6b513a06a1
https://hg.mozilla.org/releases/mozilla-beta/rev/cc63217713dc

Comment 7

[u279076]

Are there any user facing regression risks QA should be testing not covered by your tests?

Comment 8

[Simon Montagu :smontagu]

I think the existing tests cover it.

Comment 9

[u279076]

Thanks Simon, marking [qa-]