+++ This bug was initially created as a clone of Bug #994518 +++ While investigating bug 989713 I noticed that on master the keyboard application is always given PROCESS_PRIORITY_FOREGROUND priority when visible instead of PROCESS_PRIORITY_FOREGROUND_KEYBOARD. This was caused by either bug 928270 or bug 915570 which changed the application type and thus prevented the process priority manager from recognizing the keyboard app. The fix is fortunately straightforward and requires changing this check to match with "inputmethod" rather than "keyboard": http://hg.mozilla.org/mozilla-central/file/b5b35ec88db8/dom/ipc/ProcessPriorityManager.cpp#l985

Whoops, wrong title.

Can we add a test for this?

WIP patch with additional tests covering this issue. Somehow the priority right after the app has been created is still wrong, I have to do some further testing to figure out why.

(In reply to Gabriele Svelto [:gsvelto] from comment #3) > WIP patch with additional tests covering this issue. Somehow the priority > right after the app has been created is still wrong, I have to do some > further testing to figure out why. IIRC there's some sort of delay involved when switching priorities.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #4) > IIRC there's some sort of delay involved when switching priorities. I just found out that this was caused by a code-path where we blindly set the new process priority to FOREGROUND instead of computing it when constructing a ContentParent: http://hg.mozilla.org/mozilla-central/file/41a54c8add09/dom/ipc/ContentParent.cpp#l702 After fixing this too my new test passes correctly. I still have to do some testing on the device but we're almost there.

Here's a patch that attempts to fix the issue: the changes involve both updating the mozapptype the process priority manager needs to look for to identify the keyboard app as well as changes in the ContentParent class to give the proper priority to the keyboard app when it's starting up. I've added a new mochitest to check for the keyboard priority. Alternatively this test can be folded into one of the existing tests; I know we're paying attention to not making the whole testing run too long but I'm not sure how much overhead mochitests have and if it's worth avoiding to have too many of them.

Comment on attachment 8424794 [details] [diff] [review] [PATCH] Assign the correct priority to the keyboard app Review of attachment 8424794 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/ipc/ProcessPriorityManager.cpp @@ +981,5 @@ > } > } > > if (isVisible) { > + return HasAppType("inputmethod") ? :P

Thanks for the review Kyle! The patch had bit-rotted so here's a refreshed version, I'm carrying the r+ over from comment 7. The try run is here: https://tbpl.mozilla.org/?tree=Try&rev=015ce33c7e1d

Hi,gsvelto. Does that mean keyboard app has the priority PROCESS_PRIORITY_FOREGROUND? and can not get killed? tarako v1.3t has the same problem.

(In reply to ying.xu from comment #10) > Hi,gsvelto. > > Does that mean keyboard app has the priority PROCESS_PRIORITY_FOREGROUND? > > and can not get killed? > > tarako v1.3t has the same problem. Tarako runs the keyboard in-process, so none of that is revelant for 1.3t.

(In reply to < away until June 17 > from comment #11) > (In reply to ying.xu from comment #10) > > Hi,gsvelto. > > > > Does that mean keyboard app has the priority PROCESS_PRIORITY_FOREGROUND? > > > > and can not get killed? > > > > tarako v1.3t has the same problem. > > Tarako runs the keyboard in-process, so none of that is revelant for 1.3t. thanks