This bug was filed from the Socorro interface and is report bp-88a99c64-da13-41fd-97bc-8a2dd0170528. ============================================================= There is 1 crash in nightly 55 with buildid 20170527030204. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1366140. [1] https://hg.mozilla.org/mozilla-central/rev?node=f32c73555c98ab565cb7ae10a761d6817aa5c89a

Ah, this must be a simple bug of the patch. Taking.

Comment on attachment 8872272 [details] Bug 1368318 Call TSFTextStore::ThinksHavingFocus() before clearing sEnabledTextStore since the method refers it https://reviewboard.mozilla.org/r/143762/#review147484

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/024c52408261 Call TSFTextStore::ThinksHavingFocus() before clearing sEnabledTextStore since the method refers it r=m_kato

AFAICT, we'll want this on Beta/ESR52 as well. Please nominate accordingly :)

Yep. But let's watch if it's actually fixed by the patch before that. (Note that it could be just a bug of TSF or MS-IME.)

Wait, looks like that this bug is not a recent regression. I see same crash reports in 53 and older: https://crash-stats.mozilla.com/signature/?product=Firefox&signature=IsWindowColorDark&date=%3E%3D2016-11-30T04%3A31%3A21.000Z&date=%3C2017-05-31T04%3A31%3A21.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-date&page=1 So, I guess that my patch for this bug may has never fixed the crash yet. However, my patch actually fixes a regression of bug 1366140. So, anyway, we should take it to the branches, though.

Comment on attachment 8872272 [details] Bug 1368318 Call TSFTextStore::ThinksHavingFocus() before clearing sEnabledTextStore since the method refers it [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: The crash might not have been fixed by this patch, though (see previous comment), but this fixes an actual regression of the patch for sec-high bug. User impact if declined: Some TSF objects are not released when every editor loses focus. Fix Landed on Version: 55 Risk to taking this patch (and alternatives if risky): No, really simple. String or UUID changes made by this patch: No. Approval Request Comment [Feature/Bug causing the regression]: Although, this might have not fixed the crash yet (see my previous comment). But actually fixes a regression of bug 1366140. [User impact if declined]: Some TSF objects are not released when every editor loses focus. [Is this code covered by automated tests?]: No. [Has the fix been verified in Nightly?]: Any visible symptom is there. So, cannot verify this unless if this actually fixes the crash and crash reports won't come. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: Depends on the patch for bug 1366140 due to fixing its regression. [Is the change risky?]: No. [Why is the change risky/not risky?]: See the patch. Really simple mistake. [String changes made/needed]: No.

> Flags: approval-mozilla-aurora? Oops, I meant Flags: approval-mozilla-beta?... But I assume that drivers will handle it as expected ;-)

Comment on attachment 8872272 [details] Bug 1368318 Call TSFTextStore::ThinksHavingFocus() before clearing sEnabledTextStore since the method refers it The crash volume for this signature is extremely low, the other aspect of not cleaning up references in TSFTextStore while valid is not severe enough either. If it was a significant memleak, we would have heard. Given this data, I don't feel the need to uplift this fix so late in 54 or ESR52.2.

(In reply to Ritu Kothari (:ritu) from comment #11) > Comment on attachment 8872272 [details] > Bug 1368318 Call TSFTextStore::ThinksHavingFocus() before clearing > sEnabledTextStore since the method refers it > > The crash volume for this signature is extremely low, the other aspect of > not cleaning up references in TSFTextStore while valid is not severe enough > either. If it was a significant memleak, we would have heard. Given this > data, I don't feel the need to uplift this fix so late in 54 or ESR52.2. Hmm, but it fixes a recent regression of bug 1366140 obviously. (I'm not sure if it actually fixes the crash bug, though, because same signature reports exist before bug 1366140 fix.) We'll destroy TSF objects without notifying TSF/TIP of losing focus. That must be risky... (The patch should've been included in the patch for bug 1366140.)

Setting ESR52 to wontfix based on comment 12. No signs of ESR52 crashes in the last 3 months anyway. OTOH, 55 Beta builds are still hitting crashes with this signature :(