1282669 - Get rid of nsIMEUpdatePreference::DontNotifyChangesCausedByComposition()

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

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

Due to supporting e10s mode, DontNotifyChangesCausedByComposition() is worked well only with non-e10s cases (e.g., searchbar in chrome) because selection or text changes occurs asynchronously. I think that we can remove it and widget should ignore the notifications if it's actually not necessary.

Comment 1

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2eace8cb4c5

Comment 2

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=774c91ff6349

Comment 3

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

Attachment: Bug 1282669 Get rid of nsIMEUpdatePreference::DontNotifyChangesCausedByComposition()

I think that we can drop nsIMEUpdatePreference::DontNotifyChangesCausedByComposition(), i.e., nsIMEUpdatePreference::NOTIFY_CHANGES_CAUSED_BY_COMPOSITION because it's now used only by TSFTextStore but TSFTextStore ignores if SelectionChangeDataBase::mCausedByComposition or TextChangeDataBase::mCausedOnlyByComposition is true (for supporting async changes in e10s mode).  So, only issue is, dropping the flag might cause increasing computing TextChangeData cost during composition in TSF mode.  However, now, it's already enough fast and even if it'd cause performance regression, we could add a hack with TextComposition's offset information.  Therefore, we don't need to worry about the performance regression so seriously.

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

Comment 4

[Makoto Kato [:m_kato]]

Comment on attachment 8765814 [details]
Bug 1282669 Get rid of nsIMEUpdatePreference::DontNotifyChangesCausedByComposition()

https://reviewboard.mozilla.org/r/61000/#review58032

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/032a49223156
Get rid of nsIMEUpdatePreference::DontNotifyChangesCausedByComposition() r=m_kato

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/032a49223156