This is a regression of bug 1275906. The fix of bug 1275906 made NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED sent to widget asynchronously. However, IMEStateManager checks if there is a valid TextComposition at sending it <https://dxr.mozilla.org/mozilla-central/rev/88bebcaca249aeaca9197382e89d35b02be8292e/dom/events/IMEStateManager.cpp#1452-1453,1457> but TextComposition has gone immediately after composition event is sent to the DOM tree or remote process <https://dxr.mozilla.org/mozilla-central/rev/88bebcaca249aeaca9197382e89d35b02be8292e/dom/events/IMEStateManager.cpp#1170-1172,1188,1196-1197>. So, IMEStateManager should stop checking if there is a valid TextComposition at sending IME notifications like the others. And this bug causes a serious regression of bug 1224994. The cached content is never synced with actual content due to this bug.

[Tracking Requested - why for this release]: This is serious regression. TSFTextStore releases a lot of cache when it receives NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED but it's not notified after compositionend. That means TSFTextStore loses a chance of releasing unnecessary cache.

Hmm, this is non-e10s only bug. So, this shouldn't cause of bug 1286464...

Review commit: https://reviewboard.mozilla.org/r/64236/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/64236/

Comment on attachment 8770934 [details] Bug 1286730 IMEStateManager::NotifyIME() should treat NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED same as the other notifications because all of them are notified by IMEContentObserver asynchronously https://reviewboard.mozilla.org/r/64236/#review61290

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/d5cf90154e67 IMEStateManager::NotifyIME() should treat NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED same as the other notifications because all of them are notified by IMEContentObserver asynchronously r=smaug

Tracking 49/50+ for this cache regression.

Oh, this is important for e10s too. Even in e10s mode, this is reproduce in remote process, then, never sent the notification to the chrome process, of course.

Please request uplift to 49.

Approval Request Comment [Feature/regressing bug #]: Regression of bug 1275906. [User impact if declined]: The notification is used for a timing to clear cache in TSFTextStore. Especially, after bug 1224994, this is the only chance to clear cache. The cache may be big and doesn't include content changes without IME. So, for example, reconvert of selected text may not work as expected by this bug. [Describe test coverage new/current, TreeHerder]: Already landed on m-c. And I dogfooded with patched Aurora build without e10s for a couple of days too. [Risks and why]: Low because bug 1275906 made NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED treat same as other notifications at sending (made it sent asynchronously). However, actual sending path isn't changed to async-aware. This adds the notification to *existing* async path. [String/UUID change made/needed]: Nothing. # Thank you for your ping, I completely forgot to do that.

Ah, the reason why we need another patch for Aurora is, bug 1286489. The bug changed the log messages which are removed by the patch.

Comment on attachment 8772707 [details] [diff] [review] Patch for aurora Review of attachment 8772707 [details] [diff] [review]: ----------------------------------------------------------------- This patch fixes a regression. Take it in aurora.