1184012 - [Messages] Throttle event handlers for the "input" event handlers

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Julien Wajsberg [:julienw]]

There are several places where we use "input" handlers. Problem is that the "input" event is sent synchronously for each change. We should throttle this event so that we don't block on this event each time something changes.

We also need to check the "key*" events; while some should still be synchronous, some could be made asynchronous (eg: the "keypress" event in conversation.js).

We should add a "throttle" function in utils.js and use it in the appropriate places.

Comment 1

[Julien Wajsberg [:julienw]]

(It's more "debounce" than "throttle" here: we don't want the function to run "right now").

Comment 2

[Julien Wajsberg [:julienw]]

OK, it's really throttling but without the first execution ;)

Comment 3

[Stéphane Roucheray]

I am working on this one.

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] sroucheray:add-throttle > mozilla-b2g:master

Comment 5

[Stéphane Roucheray]

I implemented Utils.throttle and its unit tests.
I used throttling with the "to" field keyboard handlers in conversation.js. The metrics might require adjustments. For now it's 300ms delay.
I was not able to test it on a real device.

Comment 6

[Steve Chung [:steveck]]

Hi Stéphane,

If you don't have the device, maybe you can try out the mulet or B2G desktop[1] here. I assign the bug to you since you already created the patch. Please remember to ask the review on the patch(enter the detail and set the review flag with one of the peer) I saw your commit message that you might want julien to review your patch, so I simply left some comments without testing. Just a reminder that Julien is in PTO now so you can ask for review from :azasypkin(also in PTO but will be online next Monday) or me :)

BTW I saw some failed unit tests about the recipient behavior in conversation-test.js. Please also adjust the test in conversation-test.js if necessary, thanks!

[1]https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Developing_Gaia/Different_ways_to_run_Gaia

Comment 7

[Stéphane Roucheray]

Hi Steve,

I have answered some of your comments and modified the code. I fixed the conversion_test.js test failures.
This is my first patch so any new comments (about the code as well as about the process) is very welcome :)

Comment 8

[Stéphane Roucheray]

Comment on attachment 8656544 [details] [review]
[gaia] sroucheray:add-throttle > mozilla-b2g:master

>https://github.com/mozilla-b2g/gaia/pull/31677

Comment 9

[Steve Chung [:steveck]]

Hi Stéphane,

I left some comment on the github. Mostly are about unit test and convention, and one question about the preventLastcall. Please ping me when you're ready, thanks!

Comment 10

[Stéphane Roucheray]

Hi Steve,

I answered most of your comments on github and changed the throttle signature and associated tests.
Please have a look and give me your feedbacks ! Thanks.

Comment 11

[Steve Chung [:steveck]]

(In reply to Stéphane Roucheray from comment #10)
> Hi Steve,
> 
> I answered most of your comments on github and changed the throttle
> signature and associated tests.
> Please have a look and give me your feedbacks ! Thanks.

Ok, it really close but just last few nits! The question I asked about https://github.com/mozilla-b2g/gaia/pull/31677/files#diff-547b790d842789bb9f2a73a778bc81cfR760 is because I'm a little bit confused about the why we didn't reset previous to 0 for non-preventFisrtCall case, but now I realize that you still want to check the duration for next call, not just treat it as the first call. Sorry about the noise :/

Comment 12

[Stéphane Roucheray]

Steve, I think I fixed all the last things you commented. Tell me if that's ok for you!

Comment 13

[Steve Chung [:steveck]]

Comment on attachment 8656544 [details] [review]
[gaia] sroucheray:add-throttle > mozilla-b2g:master

Looks great and thanks for your contribution! Please squash your commits into one commit and request "checkin-needed" in the Keywords fields.

Comment 14

[Stéphane Roucheray]

This has been a great first experience and thanks for your comments and feedbacks !

Comment 15

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/e0bd0ec64986fe7b99699434da502404fb6ae128

Comment 16

[Julien Wajsberg [:julienw]]

Thanks Stéphane !