This is new regression orange and different from bug 1321173. Perhaps, caused by the fix of bug 1375491 and some other fix in https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=6e0dd1705c75d8aeedc26e7f5c9ea466f92aec96 because this orange appeared high frequently after the bug fix met some of changes in the latter. The bug 1375491 changes the timing of notifying focus. I guess that it could cause some race between setting IME focus and destroying the window. Anyway, bug 1349255 will make it async. So, this would appear anyway. I'll investigate this in next Monday.

The cause of this bug is, IMEStateManager in the main process doesn't handle delayed notifications and requests to IME properly. For example, IMEStateManager doesn't discard some delayed notifications which came from previously focused remote process. Additionally, IMEStateManager doesn't set the origin of InputContext properly in some cases. So, we need to update IMEStateManager. Especially for bug 1349255. I'd like smaug to review around process checking and non-IMEStateManager part and kato-san to review IMEStateManager's changes. # Patches coming soon.

Comment on attachment 8883584 [details] Bug 1377672 - part3: IMEStateManager::NotifyIME() should ignore notifications and requests which comes from unexpected process https://reviewboard.mozilla.org/r/154466/#review159594 ::: commit-message-71b7f:1 (Diff revision 1) > +Bug 1377672 - part3: IMEStateManager::NotifyIME() should ignore notifications nor requests which comes from unexpected process r?smaug, m_kato I don't understand this sentence "nor"? Should this be IMEStateManager::NotifyIME() should not ignore notifications nor requests which comes from unexpected process. Or should it be IMEStateManager::NotifyIME() should ignore notifications and requests which comes from unexpected process or IMEStateManager::NotifyIME() should ignore notifications, not requests, which comes from unexpected process Based on the explanation, the second one with 'and' ::: dom/events/IMEStateManager.h:284 (Diff revision 1) > // sWidget is cache for the root widget of sPresContext. Even afer > // sPresContext has gone, we need to clean up some IME state on the widget > // if the widget is available. > static nsIWidget* sWidget; > - // sFocusedIMEWidget is, the widget which was sent to "focus" notification > - // from IMEContentObserver and not yet sent "blur" notification. > + // sFocusedIMEWidget and sFocusedIMETabParent are, the tab parent sent > + // "focus" notification to the widget (and not yet send "blur" notification). Rather weird sentence. Perhaps sFocusedIMETabParent is the tab parent, which sent "focus" notification to sFocusedIMEWidget (and didn't yet sent "blur" notification). ::: dom/events/IMEStateManager.cpp:145 (Diff revision 1) > +IsSameProcess(const TabParent* aTabParent1, const TabParent* aTabParent2) > +{ > + if (aTabParent1 == aTabParent2) { > + return true; > + } > + if (!aTabParent1 ^ !aTabParent2) { Why ^ Wouldn't !aTabParent1 != !aTabParent2 work.

Comment on attachment 8883584 [details] Bug 1377672 - part3: IMEStateManager::NotifyIME() should ignore notifications and requests which comes from unexpected process https://reviewboard.mozilla.org/r/154466/#review159594 > I don't understand this sentence "nor"? > > Should this be > IMEStateManager::NotifyIME() should not ignore notifications nor requests which comes from unexpected process. > > Or should it be > IMEStateManager::NotifyIME() should ignore notifications and requests which comes from unexpected process > > or > IMEStateManager::NotifyIME() should ignore notifications, not requests, which comes from unexpected process > > > Based on the explanation, the second one with 'and' Ah, the second is I tried to say. > Rather weird sentence. > > Perhaps > > sFocusedIMETabParent is the tab parent, which sent "focus" notification to sFocusedIMEWidget (and didn't yet sent "blur" notification). Thanks! > Why ^ > Wouldn't !aTabParent1 != !aTabParent2 work. Wow, you're right!

Comment on attachment 8883582 [details] Bug 1377672 - part1: IMEStateManager in the main process should destroy active IME content observer when a tab parent takes focus https://reviewboard.mozilla.org/r/154462/#review159822

Comment on attachment 8883583 [details] Bug 1377672 - part2: IMEStateManager::SetIMEState() should set input context with proper origin information https://reviewboard.mozilla.org/r/154464/#review159836

Comment on attachment 8883585 [details] Bug 1377672 - part4: ContentCacheInParent::RequestIMEToCommitComposition() should ignore too late requests https://reviewboard.mozilla.org/r/154468/#review159846

Comment on attachment 8883586 [details] Bug 1377672 - part5: IMEStateManager::OnChangeFocusInternal() should notify IME of blur when focus is moving from a remote process to another process https://reviewboard.mozilla.org/r/154470/#review160114

Comment on attachment 8883584 [details] Bug 1377672 - part3: IMEStateManager::NotifyIME() should ignore notifications and requests which comes from unexpected process https://reviewboard.mozilla.org/r/154466/#review160116

Wow, I realized that I requested the review with older patches. But the difference is only minor fixes. So, I don't request review again. If you have some disagreement about the different, please file a followup bug since this bug causes a lot of oranges which waste the time of a lot of developers.

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 02b18f9cf6f4 -d 52d810352774: rebasing 405647:02b18f9cf6f4 "Bug 1377672 - part1: IMEStateManager in the main process should destroy active IME content observer when a tab parent takes focus r=m_kato" merging dom/events/IMEStateManager.cpp rebasing 405648:2f507d297f8e "Bug 1377672 - part2: IMEStateManager::SetIMEState() should set input context with proper origin information r=m_kato" merging dom/events/IMEStateManager.cpp merging widget/nsBaseWidget.cpp warning: conflicts while merging dom/events/IMEStateManager.cpp! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

Wow... The patches were rebased with the latest m-c...

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/dde8300a4831 part1: IMEStateManager in the main process should destroy active IME content observer when a tab parent takes focus r=m_kato https://hg.mozilla.org/integration/autoland/rev/cd648a42778f part2: IMEStateManager::SetIMEState() should set input context with proper origin information r=m_kato https://hg.mozilla.org/integration/autoland/rev/14a41922c96a part3: IMEStateManager::NotifyIME() should ignore notifications and requests which comes from unexpected process r=m_kato,smaug https://hg.mozilla.org/integration/autoland/rev/feb3d54b7cca part4: ContentCacheInParent::RequestIMEToCommitComposition() should ignore too late requests r=m_kato https://hg.mozilla.org/integration/autoland/rev/3bdc87d4e538 part5: IMEStateManager::OnChangeFocusInternal() should notify IME of blur when focus is moving from a remote process to another process r=m_kato

Removing trailing whitespace of unrelated line was landed before this and that caused the rebasing error.

https://hg.mozilla.org/mozilla-central/rev/dde8300a4831 https://hg.mozilla.org/mozilla-central/rev/cd648a42778f https://hg.mozilla.org/mozilla-central/rev/14a41922c96a https://hg.mozilla.org/mozilla-central/rev/feb3d54b7cca https://hg.mozilla.org/mozilla-central/rev/3bdc87d4e538