Closed Bug 1040215 Opened 6 years ago Closed 6 years ago

[Messages] Move initSentAudio out of enableSend

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: julienw, Assigned: julienw)

References

Details

Attachments

(1 file)

46 bytes, text/x-github-pull-request
azasypkin
: review+
Details | Review
Currently, we run "initSentAudio" in "ThreadUI.enableSend". Since we want to remove enableSend in bug 944249, it's probably better to move this call to init. Obviously, we don't want to regress load time, which could be possible if we set "preload" to "none".

We also need to look whether there is a latency when sending the first message.

The alternative is to run it when the user presses the "send" button.
Attached file github PR
Attachment #8458193 - Flags: review?(azasypkin)
Assignee: nobody → felash
Status: NEW → ASSIGNED
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Comment on attachment 8458193 [details] [review]
github PR

Code looks good, but I think this won't work as expected for the "Resend message" case since "simSelectedCallback" is not called. Could you please check it?
Attachment #8458193 - Flags: review?(azasypkin)
Comment on attachment 8458193 [details] [review]
github PR

Hey Oleg,

I moved the initSendAudio call to beforeEnter as you suggested, it makes a lot more sense and it works in all cases I tried.

I didn't want to call in init because I feared about a launch time regression and I didn't have much time to do measurements.

I still added back the "preload='none'" change that I did when I had the call in "init" because this could help to not compete with other things we do when we change panels (eg: sliding).

Tell me what you think !
Attachment #8458193 - Flags: review?(azasypkin)
Comment on attachment 8458193 [details] [review]
github PR

(In reply to Julien Wajsberg [:julienw] from comment #3)
> Comment on attachment 8458193 [details] [review]
> github PR
> 
> Hey Oleg,
> 
> I moved the initSendAudio call to beforeEnter as you suggested, it makes a
> lot more sense and it works in all cases I tried.
> 
> I didn't want to call in init because I feared about a launch time
> regression and I didn't have much time to do measurements.
> 
> I still added back the "preload='none'" change that I did when I had the
> call in "init" because this could help to not compete with other things we
> do when we change panels (eg: sliding).
> 
> Tell me what you think !

It looks good now! Also I agree that "preload='none'" is the right thing here.

Thanks!
Attachment #8458193 - Flags: review?(azasypkin) → review+
master: 77e73c20c180fdf7f37247fa60a3d1886cdc726e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.