Closed Bug 1237582 Opened 4 years ago Closed 4 years ago

[TSF][e10s] Browser hangs after bug1234422 with (hidden) Microsoft New Changjie or New Quick

Categories

(Core :: Widget: Win32, defect)

46 Branch
All
Windows
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
e10s + ---
firefox42 --- unaffected
firefox43 --- unaffected
firefox44 - wontfix
firefox45 + fixed
firefox46 + fixed
firefox-esr38 --- unaffected

People

(Reporter: Fanolian+BMO, Assigned: masayuki)

References

(Blocks 1 open bug)

Details

(Keywords: inputmethod, regression)

Attachments

(4 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160106030225

Steps to reproduce:

Prerequisite: Expose and enable the hidden IMEs Microsoft New Changjie / New Quick
1. Add the following registry in Win8.1/10

Windows Registry Editor Version 5.00

[HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\CTF\TIP\{B115690A-EA02-48D5-A231-E3578D2FDF80}\LanguageProfile\0x00000404\{F3BA907A-6C7E-11D4-97FA-0080C882687E}]
"Description"="Microsoft New ChangJie"
"Display Description"=hex(2):40,00,25,00,53,00,79,00,73,00,74,00,65,00,6d,00,\
  52,00,6f,00,6f,00,74,00,25,00,5c,00,53,00,59,00,53,00,54,00,45,00,4d,00,33,\
  00,32,00,5c,00,69,00,6e,00,70,00,75,00,74,00,2e,00,64,00,6c,00,6c,00,2c,00,\
  2d,00,35,00,30,00,39,00,33,00,00,00
"IconFile"=hex(2):25,00,53,00,79,00,73,00,74,00,65,00,6d,00,52,00,6f,00,6f,00,\
  74,00,25,00,5c,00,73,00,79,00,73,00,74,00,65,00,6d,00,33,00,32,00,5c,00,49,\
  00,4d,00,45,00,5c,00,49,00,4d,00,45,00,54,00,43,00,5c,00,49,00,6d,00,54,00,\
  43,00,54,00,69,00,70,00,2e,00,44,00,4c,00,4c,00,00,00
"IconIndex"=dword:00000002
"Enable"=dword:00000000
"ProfileFlags"=dword:00000004

[HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\CTF\TIP\{B115690A-EA02-48D5-A231-E3578D2FDF80}\LanguageProfile\0x00000404\{0B883BA0-C1C7-11D4-87F9-0080C882687E}]
"Description"="Microsoft New Quick"
"Display Description"=hex(2):40,00,25,00,53,00,79,00,73,00,74,00,65,00,6d,00,\
  52,00,6f,00,6f,00,74,00,25,00,5c,00,53,00,59,00,53,00,54,00,45,00,4d,00,33,\
  00,32,00,5c,00,69,00,6e,00,70,00,75,00,74,00,2e,00,64,00,6c,00,6c,00,2c,00,\
  2d,00,35,00,31,00,34,00,39,00,00,00
"IconFile"=hex(2):25,00,53,00,79,00,73,00,74,00,65,00,6d,00,52,00,6f,00,6f,00,\
  74,00,25,00,5c,00,73,00,79,00,73,00,74,00,65,00,6d,00,33,00,32,00,5c,00,49,\
  00,4d,00,45,00,5c,00,49,00,4d,00,45,00,54,00,43,00,5c,00,49,00,6d,00,54,00,\
  43,00,54,00,69,00,70,00,2e,00,44,00,4c,00,4c,00,00,00
"IconIndex"=dword:00000004
"Enable"=dword:00000000
"ProfileFlags"=dword:00000004

2. In Windows Control Panel, add Language Traditional Chinese. Then add IME New Changjie or New Quick in Language Option.


To reproduce the hang:
1. In a text box in the Content Window, spam/input any Chinese with the 2 mentioned IMEs in a moderately fast pace.


Actual results:

Browser hangs with high CPU usage after a few seconds of typing. To stop the hang, I have to focus outside the Content Window, e.g. open the menu, or switch to another tab. (See attachment)


There is no hangs if I type in the following scenarios:
1. Type in URL bar or search bar.
2. Type in a slow pace, much slower than common typing.
3. Input English with the IMEs, either in Chinese mode (shift + type) or Alphanumeric mode.

The issue does not affect the default Microsoft Changjie or Quick, or Japanese Microsoft IME.

Workaround:
Browser does not hang if I set intl.tsf.enable to false. Toggling others intl.tsf.* settings make no difference.
Registry file mentioned in comment 0.

Tutorials about the hidden IMEs:
http://www.hkepc.com/forum/viewthread.php?tid=1999173
https://www.enterpr1se.info/2015/09/use-changjie-5-in-windows-10/

Unfortunately I cannot find any related tutorials in English. Both of the above are in Traditional Chinese.
Okay, I'll post a patch for this tomorrow.

[Tracking Requested - why for this release]:
This must be a regression of bug 1234422 which was uplift to both Aurora and Beta.

This regression must be able to be fixed easy. If not so, I'll request to back out the patch from Beta (and Aurora if it's really serious).
Assignee: nobody → masayuki
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Unspecified → Windows
Hardware: Unspecified → All
Tracked bcuz this is a hang and a new regression on 44.
Fanolian:

I have some questions:

1. Do you reproduce this bug with disabling e10s (multi-process) mode? If this is a bug only in e10s mode, we don't need to fix this on 44 (beta).
2. When I try to reproduce this bug on Win10, I cannot type the composition string with Microsoft New ChangJie. When I type 'a' key multiple times, '日' is displayed only on the reading window (popup window) (i.e., not displayed in Gecko as inline composition string) and I can type only 5 characters. Do I misunderstand your report??
Flags: needinfo?(Fanolian+bugzilla)
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (offline until 1/6) from comment #5)
> Fanolian:
> 
> I have some questions:
> 
> 1. Do you reproduce this bug with disabling e10s (multi-process) mode? If
> this is a bug only in e10s mode, we don't need to fix this on 44 (beta).

I cannot reproduce it by disabling e10s. CPU usage is about 1 to 2% which I think it is normal.

> 2. When I try to reproduce this bug on Win10, I cannot type the composition
> string with Microsoft New ChangJie. When I type 'a' key multiple times, '日'
> is displayed only on the reading window (popup window) (i.e., not displayed
> in Gecko as inline composition string) and I can type only 5 characters. Do
> I misunderstand your report??

To type 日, press 'a' then <space> to confirm the Changjie code.
(I just spam 'a' and <space> repeatedly in the video. As 日,昌,晶,and 
Flags: needinfo?(Fanolian+bugzilla)
Um... bug in bugzilla maybe? Characters after U+232AB is rejected... Anyway, to continue my comment,

(I just spam 'a' and <space> repeatedly in the video. As 日,昌,晶,and U+232AB bear Changjie code A, AA, AAA, and AAAA respectively, but no Chineses characters for AAAAA. You always have to, however, press <space> to confirm the code even if Changjie code max length is 5.)
(If you test it with New Quick, you can skip <space> and hold 'a' to input Chinese repeatedly. (New) Quick's code length is always 2 and the first word candidate is automatically selected.)
(In reply to Fanolian from comment #6)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (offline until 1/6)
> from comment #5)
> > Fanolian:
> > 
> > I have some questions:
> > 
> > 1. Do you reproduce this bug with disabling e10s (multi-process) mode? If
> > this is a bug only in e10s mode, we don't need to fix this on 44 (beta).
> 
> I cannot reproduce it by disabling e10s. CPU usage is about 1 to 2% which I
> think it is normal.

Oh, thanks. So, this is a bug only in e10s mode. I guess that we don't need to fix this bug in 44.

Requesting Tracking 44? again, IIRC, e10s won't be enabled by release of 44.
Blocks: e10s-ime-tsf
tracking-e10s: --- → ?
Summary: Browser hangs after bug1234422 with (hidden) Microsoft New Changjie or New Quick → [TSF][e10s] Browser hangs after bug1234422 with (hidden) Microsoft New Changjie or New Quick
Thank you for the detail of STR. However, I still cannot reproduce this bug in my environment.

Could you check if this bug is reproducible with new profile?

And if it's reproducible, could you take a log of the behavior (see following comment)?

If this is really a regression of bug 1234422, you must get an evidence of infinite loop into the log. So, could you get log with following steps?

0. Start Nightly with new profile and create a bookmark to "data:text/html,<textarea></textarea>" and close Nightly.
1. Set NSPR_LOG_FILE environment variable as path to log file (full path)
2. Set NSPR_LOG_MODULES environment variable as nsTextStoreWidgets:5,IMEContentObserver:5,ContentCacheWidgets:5
3. Start Nightly with the new profile again
4. Click the bookmark only with mouse
5. Set focus to the <textarea>
6. Type characters and wait until CPU becomes busy
7. Close Nightly with clicking "X" button of the window

Then, could you attach the log file to this bug? (If the log file is too large, you need to zip it.)
Flags: needinfo?(Fanolian+bugzilla)
I can still reproduce it in a new profile.

It is the first time I do the logging thing. I hope I do this right.
The .7z file contains 2 files, cj.log (307MB) and cj.log.child-1 (177KB). I follow the instruction at https://wiki.mozilla.org/MailNews:Logging#Windows and edit the variables to the ones you advised.
Flags: needinfo?(Fanolian+bugzilla)
Thank you for your logging. Indeed, this bug must be a bug only in e10s mode when you use the TIPs.

Could you try to test with this build?
http://archive.mozilla.org/pub/firefox/try-builds/masayuki@d-toybox.com-68c3db72913ca1a36232484e313f79a37648feb7/try-win32/firefox-46.0a1.en-US.win32.zip
Flags: needinfo?(Fanolian+bugzilla)
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #13)
> Thank you for your logging. Indeed, this bug must be a bug only in e10s mode
> when you use the TIPs.
> 
> Could you try to test with this build?
> http://archive.mozilla.org/pub/firefox/try-builds/masayuki@d-toybox.com-
> 68c3db72913ca1a36232484e313f79a37648feb7/try-win32/firefox-46.0a1.en-US.
> win32.zip

There is no hangs after spam-typing for 3 minutes in a new profile.
I think the issue is fixed in this build.
Flags: needinfo?(Fanolian+bugzilla)
(In reply to Fanolian from comment #14)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #13)
> > Thank you for your logging. Indeed, this bug must be a bug only in e10s mode
> > when you use the TIPs.
> > 
> > Could you try to test with this build?
> > http://archive.mozilla.org/pub/firefox/try-builds/masayuki@d-toybox.com-
> > 68c3db72913ca1a36232484e313f79a37648feb7/try-win32/firefox-46.0a1.en-US.
> > win32.zip
> 
> There is no hangs after spam-typing for 3 minutes in a new profile.
> I think the issue is fixed in this build.

Thank you very much!
Current Gecko believes that if TIP calls neither GetTextExt() nor GetACPFromPoint() at calling OnLayoutChange() after returning TS_E_NOLAYOUT, there is a problem and TIP must want Gecko to call OnLayoutChange() again.

This is true when OnLayoutChange() is called during document lock or something similar complicated timing. However, not true when we call it with MOZ_WM_NOTIY_TSF_OF_LAYOUT_CHANGE since there shouldn't be any problems in such safe time.

So, TIP may give up retrieving layout information in some cases. In such case, we shouldn't post the message again.

Unfortunately, this patch may post MOZ_WM_NOTIY_TSF_OF_LAYOUT_CHANGE message again even if NotifyTSFOfLayoutChange() is called by NotifyTSFOfLayoutChangeAgain(). However, it'll be ignored because mWaitingQueryLayout is reset if Gecko doesn't return TS_E_NOLAYOUT newly during a call of NotifyTSFOfLayoutChange(). (Note that this shouldn't occur under current design because TS_E_NOLAYOUT is used only during handling composition but not yet dispatching them into the DOM tree.)
Attachment #8708254 - Flags: review?(m_kato)
Attachment #8708254 - Flags: review?(m_kato) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2b4d101d00691f835c4f109524a3506d5f51626
Bug 1237582 Don't retry to call TSFTextStore::NotifyTSFOfLayoutChangeAgain() when TSFTextStore::NotifyTSFOfLayoutChange() is called from it and newly TSFTextStore returns TS_E_NOLAYOUT error during the call r=m_kato
https://hg.mozilla.org/mozilla-central/rev/d2b4d101d006
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Comment on attachment 8708254 [details] [diff] [review]
Don't retry to call TSFTextStore::NotifyTSFOfLayoutChangeAgain() when TSFTextStore::NotifyTSFOfLayoutChange() is called from it and newly TSFTextStore returns TS_E_NOLAYOUT error during the call

Approval Request Comment
[Feature/regressing bug #]: bug 1234422
[User impact if declined]: Some IME users may meet the infinite loop.
[Describe test coverage new/current, TreeHerder]: Landed on to m-c. And the fix was confirmed by the reporter.
[Risks and why]: No. I tested if the fix of bug 1234422 is broken.
[String/UUID change made/needed]: Nothing.

# Although, this is now confirmed with some Chinese IMEs and e10s mode. But this may occur even without e10s mode with some other IMEs. So, it might be better to fix this bug even on beta...
Attachment #8708254 - Flags: approval-mozilla-aurora?
Comment on attachment 8708254 [details] [diff] [review]
Don't retry to call TSFTextStore::NotifyTSFOfLayoutChangeAgain() when TSFTextStore::NotifyTSFOfLayoutChange() is called from it and newly TSFTextStore returns TS_E_NOLAYOUT error during the call

Fix an important issue with e10s, taking it.
Attachment #8708254 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.