Closed
Bug 1376424
Opened 7 years ago
Closed 7 years ago
[e10s] ContentCacheInParent may set different composition's start offset to active TextComposition instance
Categories
(Core :: Widget, defect)
Core
Widget
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
thunderbird_esr52 | --- | unaffected |
firefox-esr52 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | --- | fixed |
firefox56 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
(Keywords: inputmethod, regression)
Attachments
(2 files)
59 bytes,
text/x-review-board-request
|
m_kato
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
59 bytes,
text/x-review-board-request
|
m_kato
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
This is regression of bug 1368554. When a remote process is too slow, it may have older composition than the user is actually typing. However, TextComposition instance in the parent process is destroyed when eCompositionCommit(AsIs) is being sent to the remote process (i.e., while the remote process still has the composition).
Assignee | ||
Updated•7 years ago
|
status-firefox54:
--- → unaffected
status-firefox55:
--- → affected
status-firefox56:
--- → affected
status-firefox-esr52:
--- → unaffected
Assignee | ||
Updated•7 years ago
|
Keywords: regression
Assignee | ||
Comment 1•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=850bb30a4e465d1618b34d3e64f229e0a8c67727
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8881655 [details] Bug 1376424 - part0: Backout the patch for bug 1368554 https://reviewboard.mozilla.org/r/152824/#review157962
Attachment #8881655 -
Flags: review?(m_kato) → review+
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8881656 [details] Bug 1376424 - part1: TabChild should notify TabParent of "request to commit composition" handled https://reviewboard.mozilla.org/r/152826/#review157968
Attachment #8881656 -
Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/fa41e128cb5c part0: Backout the patch for bug 1368554 r=m_kato https://hg.mozilla.org/integration/autoland/rev/111ba549dd0a part1: TabChild should notify TabParent of "request to commit composition" handled r=m_kato
Comment 7•7 years ago
|
||
backout bugherder |
landed on m-c https://hg.mozilla.org/mozilla-central/rev/fa41e128cb5c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/111ba549dd0a
Assignee | ||
Comment 9•7 years ago
|
||
Comment on attachment 8881655 [details] Bug 1376424 - part0: Backout the patch for bug 1368554 Approval Request Comment [Feature/Bug causing the regression]: Backing out the patch of bug 1368554. [User impact if declined]: Not sure. User may see unreported bugs because the approach to fix bug 1368554 was really wrong. The fix made TabParent not handle pending composition events correctly. [Is this code covered by automated tests?]: No. [Has the fix been verified in Nightly?]: 56. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: No. [Is the change risky?]: No. [Why is the change risky/not risky?]: This is just back out the patch for bug 1368554. So, the reported crash bug will be reproduced if we won't uplift the following patch. [String changes made/needed]: No.
Attachment #8881655 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 10•7 years ago
|
||
Comment on attachment 8881656 [details] Bug 1376424 - part1: TabChild should notify TabParent of "request to commit composition" handled Approval Request Comment [Feature/Bug causing the regression]: Fixing bug 1368554 with different approach (probably this is the right way). [User impact if declined]: User may meet the crash reported in bug1368554 if we'll take only the previous patch. If we won't take both patches for this bug, user may see odd behavior when remote process is too busy since the patch for bug 1368554 stopped counting pending composition in the remote process. [Is this code covered by automated tests?]: No. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: No. [Is the change risky?]: Not so risky. [Why is the change risky/not risky?]: This patch adds internal notification which is sent from a remote process having IME composition to the main process when the remote process finishes handling request to IME of committing or canceling composition. (No such notification caused not decreasing the pending composition count in the main process. And when it reaches 255 and starts new composition, the main process crashed.) [String changes made/needed]: No.
Attachment #8881656 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 11•7 years ago
|
||
So, at least only "part 0" should be uplifted. And if it's acceptable, "part 1" should be uplifted together for fixing the original crash report. Note that the crash depends on the user's behavior whether users see the crash. If a user meets the crash, they may see the crash every several hours or almost everyday. Otherwise, users won't meet this bug. So, the number of crash reports does NOT represent the users' actual inconvenience.
Comment 12•7 years ago
|
||
Comment on attachment 8881655 [details] Bug 1376424 - part0: Backout the patch for bug 1368554 seems worth taking both of those... should be in 55.0b8
Attachment #8881655 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•7 years ago
|
Attachment #8881656 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 13•7 years ago
|
||
backout bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/3d31624e0a5b
Comment 14•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/0119ca3a5757
Comment 15•7 years ago
|
||
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #10) > [Is this code covered by automated tests?]: > No. > > [Has the fix been verified in Nightly?]: > Yes. > > [Needs manual test from QE? If yes, steps to reproduce]: > No. Setting qe-verify- based on Masayuki Nakano's assessment on manual testing needs.
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•