1081992 - Crash in mozilla::TextComposition::OnCompositionEventDiscarded(mozilla::WidgetCompositionEvent const*)

Status: VERIFIED FIXED

(Core :: DOM: Events, defect)

Description

[Alice0775 White]

This bug was filed from the Socorro interface and is 
report bp-37a8c712-7890-4d05-afae-c71312141013 .
       bp-40e05000-e439-4923-beb2-60dcb2141013 
=============================================================

This crash happens on Wndows7 and Windows8.
Using :
Microsoft Office IME 2010 on Windows7SP1 x64
Microsoft IME on Windows8.1 x64

Steps To Reproduce:
1. Set intl.tsf.enable = false and Restart browser
2. Click textbox (E.g. SearchBar), Turn on IME
3. Type moji and conversion to open candidate window
4. Click [x] button of window contorols button at the top-right corner, while candidate window still is opened

Actual Results:
Browser crashes:

Comment 1

[Alice0775 White]

bp-507af762-7371-4088-90c3-e0d552141013

Atok2014(trial version) on Windows8.1 x64 crashes with intl.tsf.enable = true.
the crash happens regardless of setting of intl.tsf.enable.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Looks odd. I don't understand the reason why the TextComposition instance was already destroyed.

We don't do null check here,
http://hg.mozilla.org/mozilla-central/annotate/44168a7af20d/dom/events/IMEStateManager.cpp#l982
but if it's null, it should be crash in IMEStateManager. I'll check the actual behavior at crash tomorrow.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Alice0775 White from comment #1)
> the crash happens regardless of setting of intl.tsf.enable.

Yeah, it should be.

Comment 4

[Alice0775 White]

[Tracking Requested - why for this release]: Crashed by regression of Bug 1065835

Regression window(m-i)
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/11fc11a90d6b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20140925165440
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/73b8079308bd
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20140925170540
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=11fc11a90d6b&tochange=73b8079308bd

Regressed by: Bug 1065835

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

Oh, the method which is addressed from nullptr is performed! I think that it should be checked even in release build if it's really a bug check, e.g.,if arguments have nullptrs.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'm not sure why this bug can be reproduced only with the last window at closing it. I.e., I cannot reproduce this bug when two or more windows are open.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a19c2e8143cb

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Alice-san, could you verify this? After that, I'll request approval for Aurora.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oops, see the previous comment.

Comment 11

[Alice0775 White]

I cannot reproduce the crash any more on windows7 and windows8.1.

https://hg.mozilla.org/mozilla-central/rev/62f0b771583c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141015030202

https://hg.mozilla.org/mozilla-central/rev/62f0b771583c
Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141015030202

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thank you, Alice-san.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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

Approval Request Comment
[Feature/regressing bug #]: bug 1065835
[User impact if declined]: Use may meet this crash, but the STR is a rare case, I think.
[Describe test coverage new/current, TBPL]: Tested manually because we have no API to test this automatically.
[Risks and why]: This just adds null check before using a pointer. So, there is no risk.
[String/UUID change made/needed]: Nothing.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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