1082052 - [Messages][Refactoring] Return a Promise from SMIL.parse

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw]]

SMIL.parse uses a callback to return its result. We should convert it to use a Promise instead.

The code is located in smil.js and is unit-tested, therefore it should be a good first bug for someone wanting to work on Gaia.

Comment 1

[kumar rishav (:rishav_)]

Hi Julienw
Here we have to use Window.promise instead of window.smil and other related to this in smil.js. Right?
Thanks

Comment 2

[Julien Wajsberg [:julienw]]

We have to return a promise instead of calling the callback, and change where it's called.

Not sure I understand your question though!

Comment 3

[Julien Wajsberg [:julienw]]

Note that another contributor asked me by mail to work on this. Rishav, since you start to be used to work on the SMS app, I think it's better if you let someone else take this "good first bug" :)

Comment 4

[kumar rishav (:rishav_)]

Yes, sure :)

Comment 5

[Anirudh GP(:anirudhgp)]

Hey I'd like to work on this bug. First Gaia bug for me. How do i begin?

Comment 6

[kumar rishav (:rishav_)]

Hi anirudh
Hope you build the Gaia in your system, if not then follow this to build it first
https://developer.mozilla.org/en-US/Firefox_OS
Thanks

Comment 7

[Julien Wajsberg [:julienw]]

Hi Anirudh!

You can find some information about how to run the SMS app in Firefox in [1]. You'll also find information about unit tests in [2].

Please come on IRC (irc.mozilla.org, #gaia and #gaia-messaging) if you want to discuss directly!

[1] https://github.com/julienw/gaia/blob/1082618-sms-readme/apps/sms/README.md
[2] https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Gaia_unit_tests

Comment 8

[Anirudh GP(:anirudhgp)]

(In reply to Julien Wajsberg [:julienw] from comment #7)
> Hi Anirudh!
> 
> You can find some information about how to run the SMS app in Firefox in
Hey I'm building gaia on my Linux PC. I'll get on with it as soon as its done building. Thanks!

Comment 9

[Julien Wajsberg [:julienw]]

Ok, adding you as assignee for now :)
Please report if you decide to not work on this again. Thanks!

Comment 10

[Anirudh GP(:anirudhgp)]

After generating the debug profile, when i run firefox -profile /home/gp/mozilla-source/firefox/mozilla-central/gaia/profile-sms, I get this:
(process:7318): GLib-CRITICAL **: g_slice_set_config: assertion 'sys_page_size == 0' failed
Firefox opens up with "chrome/branding/locale/browserconfig.properties" at the web address box
and the contents of the page are : "browser.startup.homepage=about:home"
Is this how its supposed to be have i done something wrong?

Comment 11

[Julien Wajsberg [:julienw]]

(In reply to anirudh.gp from comment #10)
> After generating the debug profile, when i run firefox -profile
> /home/gp/mozilla-source/firefox/mozilla-central/gaia/profile-sms, I get this:

You really need to launch firefox using:

firefox --no-remote -profile /home/gp/mozilla-source/firefox/mozilla-central/gaia/profile-sms app://sms.gaiamobile.org

> (process:7318): GLib-CRITICAL **: g_slice_set_config: assertion
> 'sys_page_size == 0' failed

The error appears for me too, I don't know what it means.

> Firefox opens up with "chrome/branding/locale/browserconfig.properties" at
> the web address box
> and the contents of the page are : "browser.startup.homepage=about:home"
> Is this how its supposed to be have i done something wrong?

This is strange, but please follow the directions :)

Comment 12

[Anirudh GP(:anirudhgp)]

(In reply to Julien Wajsberg [:julienw] from comment #11)
> (In reply to anirudh.gp from comment #10)
> > After generating the debug profile, when i run firefox -profile
> > /home/gp/mozilla-source/firefox/mozilla-central/gaia/profile-sms, I get this:
> 
> You really need to launch firefox using:
> 
> firefox --no-remote -profile
> /home/gp/mozilla-source/firefox/mozilla-central/gaia/profile-sms
> app://sms.gaiamobile.org
> 
> > (process:7318): GLib-CRITICAL **: g_slice_set_config: assertion
> > 'sys_page_size == 0' failed
> 
> The error appears for me too, I don't know what it means.
> 
> > Firefox opens up with "chrome/branding/locale/browserconfig.properties" at
> > the web address box
> > and the contents ohpposed to be have i done something wrong?
> 
> This is strange, but please follow the directions :)
Thanks :) That worked!
T

Comment 13

[Julien Wajsberg [:julienw]]

Hey Anirudh, could you move forward here? :)

Comment 14

[Anirudh GP(:anirudhgp)]

I'm not well versed with javascript yet so i dont know how to convert it to return a promise. Can you help me?

Comment 15

[Julien Wajsberg [:julienw]]

About Promises, I suggest to first read [1]. Please read this resource very thoroughly, and if you have questions after reading it, I'd suggest you come on IRC (irc.mozilla.org, channel #gaia or #gaia-messaging) and we can discuss.

[1] http://www.html5rocks.com/en/tutorials/es6/promises/

Comment 16

[Alexey Novak]

I could give a shot at this bug.
But I have a question. I see that parse might accept an array of attachments. You can especially see this in |attachments.forEach(SMIL_parseWithoutSMIL);| How would the promise workout here? Because for every blob processing it seems there is |exitPoint()| is called. So that I understand correctly exitPoint IS a returning spot from the function?

Comment 17

[Steve Chung [:steveck]]

(In reply to Alexey Novak from comment #16)
> I could give a shot at this bug.
> But I have a question. I see that parse might accept an array of
> attachments. You can especially see this in
> |attachments.forEach(SMIL_parseWithoutSMIL);| How would the promise workout
> here? Because for every blob processing it seems there is |exitPoint()| is
> called. So that I understand correctly exitPoint IS a returning spot from
> the function?

There is an idea that replacing all the activeReaders related function into promise(readTextBlob/convertWbmpToPng), and replace exitPoint with Promise.all to wait for all the promises completed and return slides.

Comment 18

[Julien Wajsberg [:julienw]]

Reseting assignee.

Comment 19

[Julien Wajsberg [:julienw]]

Actually taking it, I need it to make my life easier in bug 1162030.

Comment 20

[GitHub Autolander Bot]

Attachment: [gaia] julienw:1082052-smil-promises > mozilla-b2g:master

Comment 21

[Julien Wajsberg [:julienw]]

Comment on attachment 8635336 [details] [review]
[gaia] julienw:1082052-smil-promises > mozilla-b2g:master

Hey Oleg, how does it look ?

Comment 22

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8635336 [details] [review]
[gaia] julienw:1082052-smil-promises > mozilla-b2g:master

Looks good, left some questions and nits at github.

Thanks!

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/2f059a213fe84f114fc693b8e90b28c452dec5cb