Gaia/SMS/Scrum/2.1S2

From MozillaWiki
< Gaia‎ | SMS‎ | Scrum
Jump to: navigation, search

List of bugs

SMS issues handled by the SMS subteam (blocks the sprint bug 1048360)

Bugzilla link

Full Query
ID Assigned to Summary Blocking b2g Feature-b2g Whiteboard Resolution
976091 Julien Wajsberg [:julienw] [Messages][DSDS] Show the SIM information in the message bubble --- No cf_feature-b2g [p=1] FIXED
1041124 Oleg Zasypkin [:azasypkin] [Messages][Refactoring] Make use of EventDispatcher in MessageManager --- No cf_feature-b2g [sms-sprint-2.1S1][p=1] FIXED
1048204 Oleg Zasypkin [:azasypkin] The back button seems like pressed even without pressed 2.0+ No cf_feature-b2g [g+][LibGLA, Dev, B] [p=1] FIXED
1048362 Oleg Zasypkin [:azasypkin] [Messages] Compose Panel refresh (highlight actionable part) --- No cf_feature-b2g [tako][p=1] FIXED
1048365 Steve Chung [:steveck] [Messages] Remove the "tap" action on the contact when in the report panel --- No cf_feature-b2g [tako][p=1] FIXED
1048839 Oleg Zasypkin [:azasypkin] [messages] Investigate performance hit from style changes --- No cf_feature-b2g [p=2][c=graphics u= p= s=] WONTFIX
1048841 Steve Chung [:steveck] [Messages] investigate performance hit from IndexedDB at startup (eg: Contact lookup) --- No cf_feature-b2g [p(2.1S3)=1][p(2.1S2)=2][c=startup p= s= u=] WONTFIX
1048843 Steve Chung [:steveck] [Messages] Investigate about concatenating/minifying CSS --- No cf_feature-b2g [p=1] WONTFIX

8 Total; 8 Open (100%); 0 Resolved (0%); 0 Verified (0%);


Remaining points and burndown chart

google chart api url for Sprint 2.1S2

Burndown chart
Remaining points
Start 11
Day 2 10
Day 3 10
Day 4 10
Day 5 8
Day 6 8
Day 7 7
Day 8 7
Day 9 6
Day 10 3
End 0


SMS issues handled by the SMS subteam outside of the sprint (contains whiteboard "sms-sprint-2.1S2")

Full Query
ID Assigned to Summary Blocking b2g Feature-b2g Whiteboard Resolution
1051788 Julien Wajsberg [:julienw] [System][Notifications] Add a support for "priority" notifications and "silent" notifications --- No cf_feature-b2g [sms-sprint-2.1S2] FIXED
1051852 Julien Wajsberg [:julienw] [Messages] use 'DOMContentLoaded' instead of 'load' to init the various resources 2.0+ No cf_feature-b2g [sms-sprint-2.1S2] FIXED
1052442 Oleg Zasypkin [:azasypkin] [sms] We should never close the activity when viewing the message report 2.0+ No cf_feature-b2g [g+][LibGLA, Dev, B][sms-sprint-2.1S2] FIXED
1053276 Julien Wajsberg [:julienw] [Messages] ActivityHandler tests are failing if the test agent is hidden --- No cf_feature-b2g [sms-sprint-2.1S2] FIXED

4 Total; 4 Open (100%); 0 Resolved (0%); 0 Verified (0%);


All SMS issues tracked for this sprint (target milestone)

Bugzilla link

Full Query
ID Assigned to Summary Blocking b2g Feature b2g Resolution
976091 Julien Wajsberg [:julienw] [Messages][DSDS] Show the SIM information in the message bubble --- 2.1 FIXED
1041124 Oleg Zasypkin [:azasypkin] [Messages][Refactoring] Make use of EventDispatcher in MessageManager --- --- FIXED
1048204 Oleg Zasypkin [:azasypkin] The back button seems like pressed even without pressed 2.0+ --- FIXED
1048360 [meta] SMS subteam sprint 2.1S2 --- --- FIXED
1048362 Oleg Zasypkin [:azasypkin] [Messages] Compose Panel refresh (highlight actionable part) --- 2.1 FIXED
1048365 Steve Chung [:steveck] [Messages] Remove the "tap" action on the contact when in the report panel --- 2.1 FIXED
1048839 Oleg Zasypkin [:azasypkin] [messages] Investigate performance hit from style changes --- --- WONTFIX
1048843 Steve Chung [:steveck] [Messages] Investigate about concatenating/minifying CSS --- --- WONTFIX
1049741 Julien Wajsberg [:julienw] [Messages] navigation.js sets the data-position after a first reflow and triggers a 50ms reflow at launch time 2.0+ --- FIXED
1051852 Julien Wajsberg [:julienw] [Messages] use 'DOMContentLoaded' instead of 'load' to init the various resources 2.0+ --- FIXED
1052442 Oleg Zasypkin [:azasypkin] [sms] We should never close the activity when viewing the message report 2.0+ --- FIXED

11 Total; 11 Open (100%); 0 Resolved (0%); 0 Verified (0%);


Sprint planning

Minutes are on a separate page.

Daily meetings

Day 2: 6th August

Steve

  • bug 1048365 - [Messages] Remove the "tap" action on the contact when in the report panel
    • Patch given and some feedbacks return, need to fix it for another round.
  • bug 983172 - Parsing jpeg header information for downsampling the image for thumbnail
    • Create 2 WIP for more feedback, pending to next sprint for now
  • bug 1048841 - [Messages] investigate performance hit from IndexedDB at startup (eg: Contact lookup)
    • Some profiling result: When I tried the latest build on flame, it seems reducing the fb IDB lookup didn't help much for reducing the rendering time(10~ 20 ms less for first panel ready), and it dodn't help much either even I reduce the contact look up (50ms less), but rendering contact with photo did increase the time(around 180ms, comparing with all first panel with no photo/ all with photo). So need to dive in deeper if it's related to styling or image loading
      (Julien) it's strange, I really saw the difference when running the buri side by side, but maybe it was only my eyes ;) (and also I think I have one faster Buri)
      (Julien) a guess: flame has 2 CPU while Buri has 1 CPU <- Maybe, I could try it on buri
      (Julien) is it the time for full rendering, or only first panel? How many threads do you have?
      (Steve) Only first view, so it's 10 threads rendering time
      (Julien) ok, 180ms for 10 threads is really a lot ! :)

Today:

  • Fix bug 1048365
  • For QC blocker: Trying to find another way to reduce rendering time or cache the threads, and do some experiment on css minify manually.
    (Julien) I can take the css minify experimentation if you want (unless you're ready yet :) ) ?
    (Steve) For the experimentation, it should be fine for only manual testing, but I might need more time for inject this in build time
    (Julien) ok, I can take this task if we find an improvement. I've seen other apps are doing it already (the ones that use requireJS) BTW.
    (Steve) cool, that's would be helpful :)
    (Steve) But just wondering if we need to cache the threads instead? Since caching the contact seems not much improvement(~750ms -> 700ms, for the first panel)
    (Julien) if caching contacts does not bring much improvement, then yes, let's focus on the big improvements unless it's something easy (like data-position: 50ms but easy :) ).
    (Steve) data-position? Oh I saw it :)
    (Julien) see Oleg's update ;)

Julien

  • performance stuff:
    • bug 1043293: make new homescren use "touchend": landed, asked the Gaia Python team to fix the issues with the Python tests.
  • sprint planning preparation and then reporting to bugs
  • helped Oleg to test a change in the SMS app affecting startup (set data-position="right" in index.html)
    • took a lot of time because on of my buri was not in good shape
  • reviewed bug 1048365: remove "tap" action in the report panel. Looks good, only some nits.

Others:

  • worked on a Notification issue affecting SMS: when a notification is still displayed at reboot, the app is launched because of the "show" event: bug 1046828
    • will try to do a marionette test about this today but won't spend too much time on it

Today:

  • will look more to the facebook lib from a perf point of view
  • lots of reviews are piling in, I want to do some of them (esp the one from Gabriele's GSoC).

Oleg

  • bug 1015841 - [Messages][Refresh] Recipients panel visible area handling
    • Fixed unit tests in accordance with the latest review comments (landed).
  • bug 1048204 - The back button seems like pressed even without pressed
    • Small update of existing "hover" fix (landed).
  • bug 1043445 - [Messages][MMS] It's impossible to send or manually download MMS with "smartmobil.de" SIM
    • Helped a bit to Albert to collect necessary data about my SIM since he doesn't have smartmobil.de SIM card. His temporal patch works for me.
      (julien) his patch reverts the previous patch, so it's normal it works ;)
      (Oleg) Yep :) But not sure what will be the final patch though.
  • bug 1048839 - [messages] Investigate performance hit from style changes
    • Still investigating performance impact, not much results for now, should have today.
    • While learning profiling tools/Cleopatra noticed that we have style flush/reflow (from 50 to 95 ms) at startup due to initial mainWrapper[data-position] change. Asked Julien to help to test the change that should get rid of this flush/reflow (by setting default data-position in index.html). Looks like it improves situation a bit. Tested it today on two flames with x-heavy load, I can spot the difference too.
      (Julien) I can do a PR for this while you continue investigating if you want?
      (Oleg) Yep :)
      (Julien) ok, will file a separate bug and ask you review :)

Today:

  • Will handle review comments for the patches that currently in review;
  • Will continue to work on bug 1048839.

Day 3: 7th August

Steve

  • bug 1048365 - [Messages] Remove the "tap" action on the contact when in the report panel
    • Fix patch for 2nd round review.
  • bug 983172 - Parsing jpeg header information for downsampling the image for thumbnail
    • Create 2 WIP for more feedback, pending to next sprint for now
  • bug 1048841 - [Messages] investigate performance hit from IndexedDB at startup (eg: Contact lookup)
    • Trying to cache the threads data into localStorage or asyncStorage, but the progress didn't go smoothly... :-/ thread property seems missed while using localStorage, and asyncStorage seems got another problem about race condition with getThread request. Anyway I will keep trying to make one of them able to display at least 1st panel correctly.
      (Julien) about thread properties: probably the Thread object from gecko is not serializable; I don't remember if we have a utiliy function in threads.js to make a clone of this (I know we have one from a message to a thread).
      (Steve) great! Thanks for reminder

bug 1048843 - [Messages] Investigate about concatenating/minifying CSS

    • Some brief result after manual test: The minified/concat result only got 40~50 ms less then original one... Not really an obvious improvement
      (Julien) it's not bad for startup ! especially that it's quite easy to do automatically :) I can try to do a build script?
      (Steve) Sure, we will need to do our build script for it
      (Julien) maybe I can do it in the "webapp optimize" as in the general build ? Or maybe ask Yuren about it ? Would be nice if all apps can benefit from this.
      (Steve) I've told him about the css minified and he is open if someone could help.
      (Julien) oki

Today:

  • Fix bug 1048365
  • For QC blocker: Trying to make the thread cache work.

Julien

  • performance stuff:
    • bug 1043293: make new homescren use "touchend": there is a regression (bug 1049014), there is a patch I'll review :/
  • bug 1049741: prepared a patch for the "data-position=right" issue, but I want to test on my Buri first (they don't have the same Gecko right now :/)
  • spent some time on the CMAS POC
    • did the necessary changes in System regarding the notification handling
    • keep the notification instead of closing it
    • make the minimized attention screen transparent

Others:

  • sent some mails to clean up the peers for the SMS team, and have Oleg as new peer :)
    (Oleg) aaah, can't believe! :)
    (Steve) \0/ Congrats!

Today (same than yesterday basically...):

  • ask review on bug 1049741
  • will look more to the facebook lib from a perf point of view
  • lots of reviews are piling in, I want to do some of them (esp the one from Gabriele's GSoC).

Oleg

  • bug 1043445 - [Messages][MMS] It's impossible to send or manually download MMS with "smartmobil.de" SIM
    • Tested the latest Albert's patch that uses MMS settings from smartmobil.de web page, I can download MMS, but can't send :) Sent question to smartmobil.de support to confirm the settings.
  • bug 1048839 - [messages] Investigate performance hit from style changes
    • Analyzing profiles to determine what can affect startup path. I can't see that contact images in the thread element affect time to show first thread, as images are displayed later (after actual thread element is rendered). Also don't see any difference when comparing two flames side by side (one without images at all).
      (Julien) I think that now we need to look at time to display each threads; I think that to start displaying the first thread we're in quite good shape
      (Oleg) Yep, comparing to v1.3(flames) I don't think that we're too bad :) It looks similar now (using index.html patch locally too). On Friday we expect feedback from QC, right?
    • Rendering of the first thread container takes some time ~90-100ms. Noticed that we insert thread container to the DOM first and then appends actual thread element. Will try to insert thread container with the first thread at once.
    • Trying to make thread element as lightweight as possible to see if it affects painting time.
    • Some reflows are caused by "sticky" headers and l10n - will look into it too.
      (Julien) need to know if they're big or small reflows. small reflows are fine and expected
      (Oleg) Agree, maybe it's not a big deal, some profiles contain reflow\repaint caused by sticky header code, when I see it - it's about 40ms (style flush + reflow + repaint) and it happens not in the beginning.
      (Julien) 40ms is actually a lot. Small reflows take some ms.
      (Oleg) Ok, will dig to understand why I don't see it in some profiles
      (Julien) our profiler polls the device; as a result, we don't see what happens between polls. The poll delay is what you specify when you start the profiler (IIRC), by default 10ms, can be 1ms.
      (Oleg) Oh, btw, to profile SMS app from the startup I call "./profile.sh start" without any arguments, is it possible to specify profiling only one app that isn't run yet? If I add some any arguments (ex. -i 1) it requires me to have app run.
      (Julien) yep, there is a "prestarted" process that the next launched app will use; you can profile this process and then start the app. In the profile, you'll see a flat graph first, which is this process doing nothing :) Just run b2g-ps to know its PID.
      (Oleg) Ah, cool, thanks! So we have "warmed up" process for the next app to run?
      (Julien) yep; it starts about 5sec after the previous app is launched. That's why when you test the perf you need to wait about 5 sec to not get false results (I mean, not have the full gecko launch in addition to just the app launch). We have also something called "Nuwa" but I don't exactly know what it is.
      (Oleg) Good to know, thanks!

Other:

  • Extracted thread batch deletion code to a separate branch

Today:

  • Will handle review comments for the patches that currently in review;
  • Will continue to work on bug 1048839.

Day 4: 8th August

Steve

  • bug 1048365 - [Messages] Remove the "tap" action on the contact when in the report panel
    • r+ but ome final fix this some test changes. Waiting for gaia-try green.
  • bug 983172 - Parsing jpeg header information for downsampling the image for thumbnail
    • Create 2 WIP for more feedback, pending to next sprint for now
      (Julien) I'll give your feedacks today, so that you can move forward next sprint or when you have time ;)
      (Steve) Thanks! It's not part of sprint, so not in hurry ;)
  • bug 1048841 - [Messages] investigate performance hit from IndexedDB at startup (eg: Contact lookup)
    • Using a localStorage to cache the fist panel of thread for rendering. Will update some result later. It will make first panel visible quickly, but need to update the thread correctly(this part i not ready yet).
      (Julien) In short, my radical approach is to render only first panel and push data in Threads.set, and see if I can scroll up/down correctly by rendering only what's needed. I'll see if I can get something to work.

bug 1048843 - [Messages] Investigate about concatenating/minifying CSS

    • It looks like Fred already tried before :p But the patch in bug 855989 looks did a lot more things with minify part. Not sure how much time will be improved if we only concat the css.
      (Julien) Vivien told me that some of the penalties we used to have was because of the CSP checks; but nowadays CSP is a lot faster.

Today:

  • Fix bug 1048365
  • For QC blocker: Trying to make the thread cache work.

Julien

  • performance stuff:
    • don't really know where to head now, we spent a lot of time on this already and I'm not sure of the _real_ priority of this. I'll try to figure out today and possibly try other approaches too.
    • worked on a patch to concatenate CSS at build time for whole gaia, but I don't see much improvements ... I'll try to measure better but in the end we'll probably abandon this lead. (bug 1048843)
      (Steve) Considering the result they want, css minify seems not a satisfying answer for them. But It' still confusing that how QC measuring the loading time... will push Wesley to figure them out.
      (Julien) Thanks
  • bug 1049741: use "data-position=right" in index.html: landed
  • CMAS POC
    • not any work on this yesterday except answering some comments
  • bug 1046828: the notification "show" event should not start the app: this is in Gecko but affects our app, I have a working patch but a red Gij build that I don't reproduce locally :( Since it's less priority (won't likely get 2.0+) I'll keep it aside for now.
  • reviewed patches from Steve (tap handling in report view) and Oleg (use events in Messagemanager; and generate images in tests instead of loading real and big images)

Today:

  • reviews
  • try more radical approaches to the perf issue
  • will update my debug patch for bug 1047303 so that Patrick can fix it correctly

Oleg

  • bug 1044224 - [Messages][Tests] Generate assets on the fly instead of loading it with XHR
    • Fixed the latest review comments and landed (landed).
  • bug 1041124 - [Messages][Refactoring] Make use of EventDispatcher in MessageManager
    • Started to work on review comments (in progress).
      (Julien) don't forget it's not part of the sprint ;)
      (Oleg) no? It looks like it's included in sprint :) I'm puzzled :) Yes, seems target milestone is for this sprint and it blocks current sprint too :)
      (Julien) ok, sorry, then _I_ was confused :p
  • bug 1048839 - [messages] Investigate performance hit from style changes
    • Made a bunch of experiments: gecko master + gaia master + sms v1.3, gecko master +gaia v1.3 + sms v1.3, gecko master + gaia v1.3 + sms master. Looks like "gecko master + gaia v1.3 + sms master" is the fastest option :) Painting time for "insertThreadContainer" increased on ~20ms in master, but it's just sum of a bunch of new things, so don't see how we can improve that to make a real difference :(
    • I'll post my experiment profiles to the bug, just for the record, but I'd rather stop this small tuning\optimizations for now.
    • I clearly see that time from tap on app icon to first frame of our app regressed, then difference between master and v1.3 not really noticeable. So "gecko master + gaia master + sms v1.3" is much slower than "gecko v1.3 + gaia v1.3 + sms v1.3". But maybe it's not correct to do such crazy experiments :)
      (Julien) I did do such crazy experiments (it's a good thing it works ok ;) ), and I basically found like you. Please post your device too, I really think 1 or 2 CPU makes a difference here as well.
      (Oleg) Yeah, will mention that I use flames, I can record videos for every such experiment if it's valuable.
      (Julien) also what's important to mention is your start and end points.
      (Oleg) mm, what do you mean? I mainly look into two things, length of reflow\repain of thread in profile and side by side compassion,
      (Julien) I mean, when you say it's faster, what is faster? :) displaying the first panel? displaying all threads ? Do your threads have contacts ? Do they have pictures ? QC itself is not really clear about what they're measuring.
      (Oleg) Ah, ok. I'm mainly concerned about displaying first thread
      (Julien) ok; I think that now we need to focus on how to display the first panel, but that's not easy...

Today:

  • Will handle review comments for the patches that currently in review;
  • Will continue to work on bug 1041124 and post my profiles to the bug 1048839.
  • Will pick up next sprint bug.

Day 5: 11th August

Steve

  • bug 1048365 - [Messages] Remove the "tap" action on the contact when in the report panel
    • Landed in master
  • bug 983172 - Parsing jpeg header information for downsampling the image for thumbnail
    • Got feedback from the patch without additional resizing for thumb. Will ask visual/UX about removing the thumbnail size limitation for less logic in thumbnail creation.
  • bug 1048841 - [Messages] investigate performance hit from IndexedDB at startup (eg: Contact lookup)
    • I've checked the performance about caching with asyncStorage, but the result is not as good as I thought. Maybe I misuse the asyncStorage or using the multiple asyncStorage at the same time hurt the performance?
      (Julien) in my early tests (last year) I've seen that indexeddb (and so asyncStorage) is slower than localStorage at the start. But localStorage is not very good when we constantly write to it...
      (Steve) ya, I agree it's not very ideal if we need to update it frequently...
      (Julien) Something that we can try is read from asyncStorage at the very start in startup.js (without waiting for "load") just to make it load in memory? Even if we throw away the data at that moment.
      (Julien) if we store only the first threads, maybe we can store in a cookie like email does... Not sure this is better in terms of elegance :p
      (Steve) Any specific reason that cookie might do better that localStorage?
      (Oleg) cookies-as-local-storage only can't be elegant :)
      (Julien) things may have changed from when I worked on this but at the time: cookies where loaded before the app was loaded, so it was quick for read access. For write access maybe we can write once at the end. (but maybe we can do the same for localStorage too) (and I _think_ localStorage is also loaded before the app is loaded now... so maybe it's nearly the same than cookies nowadays...). Don't know who to ask to check all these questions?
      (Steve) Even we could know cookie(or localStorage) could be loaded before dom loaded, could we confirm that it won't delay the dom load time? But it would be interesting to know it.
      (Julien) yep, I don't know :) First think I'd like to try loading asyncStorage very early and see if this improves. Then maybe localStorage is the best we can do but I can ask Vivien today. Or we can ask on dev-gaia too.
      (Steve) Thanks, I will do more experiment about this as well(and ask somebody else)

Today:

  • Fix bug 1048365
  • For QC blocker: Trying to make the thread cache work.

Julien

  • performance stuff: bug 1038176
    • reviewed the good approach from Steve in bug 1048841
    • concatenate CSS at build time for whole gaia abandon this lead. (bug 1048843, closed WONTIX)
    • discussed over the week end with QC to know what they measure. Good news: they measure what we optimize for
    • started a WIP to have a proper in-javascript model for the threadList. Hoping we can display only part of the threads... https://github.com/julienw/gaia/compare/experimentation-rendering-threads not much done yet !
    • I'd like to try something simple: replace the "load" event with "DOMContentLoaded" in startup.js...
  • CMAS POC
    • no more work on friday
  • reviewed the patches from Steve to use David Flanagan's image_utils.js
    • this goes in the right direction because this removes dataurl in favor of blob urls; also we don't decode completely the image to merely know its size => faster, less memory
  • bug 1047303: a "alert" stops events in the wrong window
    • improved the debug patch

Today:

  • reviews
  • try to replace "load" by "DOMContentLoaded" in startup.js
  • will handle comments for CMAS and possibly ask for a first review from Steve :)
  • possibly move forward the WIP for thread rendering

Oleg

  • bug 1041124 - [Messages][Refactoring] Make use of EventDispatcher in MessageManager
    • Fixed the latest review comments, will send for review today (in progress, almost ready).
  • bug 1048362 - [Messages] Compose Panel refresh (highlight actionable part)
    • Prepared patch, testing (in progress);
    • Noticed two issues while working on this one:
      • "Overlay" messages (like sms converting to mms message and similar) aren't positioned correctly after "subject handling" patch, it should respect single/mulitline To: field. Going to include this small fix to this patch.
      • Eg. on Flame if we enter manually two recipients "123456789" then caret will move to second line and To: field will transition to multiline mode. But once we blur To: field, its height decreases to single line, but we don't handle that at all. For now not sure about fix.
    • It seems that we need to remove "tap-on-composer-container-to-focus-input" behaviour in the scope of this bug too I just don't remember whether we discussed it earlier.
      (Julien) yeah, i discussed about it with Jenny, see https://bugzilla.mozilla.org/show_bug.cgi?id=1026690#c12
      (Oleg) Ok, cool, thanks
  • bug 1048839 - [messages] Investigate performance hit from style changes
    • Posted profiling results and video with my comments; Don't think that we can do anything significant from styling PoV;
      (Oleg) Julien, do you want me to check with first page of threads where all threads has contact images?
      (Julien) yes please ! :)
      (Oleg) Ok, but it won't help to the QC test case anyway :)

Today:

  • Will handle review comments for the patches that currently in review;

Luke

  • bug 1047473 - [Messages] Small bottom padding difference when the recipient panel is closed/open
    • landed

Today:

  • bug 1050682 - [Messages] The recipient container will remain multiline style when removing the last recipient from opened panel
    • study codebase (thread_ui.js and recipients.js)

Day 6: 12th August

Steve

  • bug 983172 - Parsing jpeg header information for downsampling the image for thumbnail
    • Discussion with UX and visual, they might prefer to keep the original behaviour more, maybe square thumbnail would be another option like Omega said before.
      (Julien) that's because they don't use the phone! current behavior is a pain once you start having MMS in the thread :(
      (Steve) I could understand that having a fixed height for both portrait/landscape seems not a ideal layout for them (because portrait image could be much smaller than landscape)
      (Julien) in my proposal, portrait images would be the same than now, and landscape images would be bigger. Is it what they don't like?
      (Steve) They might prefer square instead, but I'm not sure :p
      (Julien) okay; square works for me :p but it's more difficult to adjust in CSS only? We'll see!
  • bug 1048841 - [Messages] investigate performance hit from IndexedDB at startup (eg: Contact lookup)
    • More investigation in localStorage and asyncStorage, it actually have not much difference in the end because penalty for using localStorage is in load time... Things I don't understand is the cache update stuff, it seems the update result was updated only when next time we fetch again? I'll trying to fetch the cache again after saving and see if the load time could be reduced.
      (Julien) I haven't read the code yet but at render time, do you wait that asyncStorage returns before requesting messages from Gecko?
      (Steve) I didn't wait for asyncStorage return, so it's possible to render the threads from gecko first, but rendering at the same time seems not works quite well because you could see the render will stuck for the while when getThreads start first than cache return later
      (Julien) I mean that you first request asyncStorage, and only once you get the result and append threads you can call getThreads (like you do with the localStorage solution after all). The goal is that asyncStorage call does not compete with the getThreads call (both use IndexedDB). I don't know if it will be better but worth to try !
      (Steve) ok, I could try it, but it seems not possible to have the cache return faster than getThreads cursor return :-/
      (Julien) I don't understand: if you wait that you get the cache return before calling getThreads... ?
      (Steve) I know, just get thread 'after' the cache rendering complete. I means fetching the cache by localStorage or asyncStorage seems no faster than indexedDB cursor return
      (Julien) with localStorage it's already the case because it's synchronous :)
      (Julien) so what is slow? calling "continue" and waiting for the next thread? would it be better to have a getThreads method returning data by chunks? Can you discuss with Bevis about this? (Not sure this can be in v2.0 if we need to change gecko though, so we might still want to do code in Gaia) In the past we had a getThreads that was returning everything, and we changed to a cursor. But maybe we can have a middle ground.
      (Steve) Returning a chunk might be an idea, but we can wait for someday that we implement our own threads indexedDB(with datastore changes)
      (Julien) right, threads-in-gecko will eventually disappear

bug 1051852 - [Messages] use 'DOMContentLoaded' instead of 'load' to init the various resources

    • It's amazing that we should utilize event for earlier initialization :p And I was miss-leaded by time-to-load value at first...
      (Julien) such a simple change ;)
      (Julien) I even tried just removing this event (because we're in a defer script so the DOM should be loaded) but there are more issues with l10n that I reported. Maybe we'll be able to do this later.

Today:

  • Clean review queue.
  • For QC blocker: Trying to make the thread cache work.

Julien

  • performance stuff: bug 1038176
    • tried to replace the "load" event with "DOMContentLoaded" in startup.js, and... it worked well !
  • CMAS POC
    • handled the feedback comments
    • cleaned up the JS and added unit tests.
    • created several bugs to separate the existing patch in several patches and show what's missing in next sprint
    • asked review for the System app part
  • Reviewed bug 1040271: some change in how we use mozL10n by the l10n team
  • Reviewed bug 1020306: change how the "select mode" look like
  • Wasted some time to make a working Flame...

Today:

  • reviews
    • especially need to look at the "Predefined message" patch; I've seen that the developer asked for a feedback from Steve because I was too slow to answer. Problem is I discussed about this a lot with Anthony already so some knowledge is not in the bug unfortunately :/
  • will finish cleaning up the CMAS POC and ask for a first review from Steve :)
  • possibly move forward the WIP for thread rendering

Oleg

  • bug 1041124 - [Messages][Refactoring] Make use of EventDispatcher in MessageManager
    • Sent for review (in review).
  • bug 1048362 - [Messages] Compose Panel refresh (highlight actionable part)
    • Prepared patch and sent for review (in review);
    • Subject looks ugly IMO without next bug :)
  • bug 1048845 - [Messages] Compose Panel refresh (subject behavior change)
    • Provided WIP patch for this. I had this for a while already, just rebased and attached to the bug, it seems patch works well, but will polish it if we have time during this sprint, otherwise - for v2.2.
  • bug 1048839 - [messages] Investigate performance hit from style changes
    • Added profiles/videos for the threads with contact images and without border-radius/shadow. Can't spot big difference on Flame. I see that we started to use image-rendering: -moz-crisp-edges for contact thumbnails, but in theory it should only improve performance.
    • Hopefully Gabriele will reveal more on that.
  • bug 1041765 - [Messages] Thread view redesign
    • Started to look into that, looks like we have some conflicting changes going on in a separate bug "bug 1020306 - Make the thread/message edit mode to match the style used by other applications"
      (Julien) bug 1020306 will likely land earlier so you should base your work on it
      (Oleg) Are there any approximate ETA for this patch?
      (Julien) I think it should be ready this week. Worst case is we land it as it is and we handle any things that are not "Design-Ready" separately
      (Oleg) Ok, that's fine

Today:

  • Will handle review comments for the patches that currently in review;
  • Will start to work on Thread view redesign, at least will look closely what may need from UX\VD

Luke

Today:

  • bug 1050682 - [Messages] The recipient container will remain multiline style when removing the last recipient from opened panel

Day 7: 13th August

Steve

  • bug 983172 - Parsing jpeg header information for downsampling the image for thumbnail
    • Mike would like to try with square, so I might try with square thumb first. It should not be difficult to use css for square thumb.
      (Julien) how do we decide what to display? Do we take the center?
      (Steve) Yes, we do center first, and UX will review the overall result.
  • bug 1048841 - [Messages] investigate performance hit from IndexedDB at startup (eg: Contact lookup)
    • Update with my testing video, it's sad that buri didn't improve much(or maybe even regress)... Maybe we should withdraw the cache idea :(
      (Julien) How is the Flame, in your testing? If the Flame is better with your patch, maybe we can ask feedback to QC too, and have a build-time flag or something? Maybe too much work for a small improvement :/
      (Steve) You can see the video here: https://www.youtube.com/watch?v=823dS2_TnF4&list=UUkM1XSMB9c0p9684qCa6ZAA
      (Julien) both sides are using DOMContentLoaded?
      (Steve) yap.
      (Julien) okay so on Flame it's definitely better, I think. Especially that we can't really access the threads until the first panel is rendered ;) We can ask feedback to QC but it will take 2 or more days...
    • Will try to move getThreads iteration as early as possible and render data from Threads
      (Oleg) I see that in comments there are some reference to experimental patches with batch rendering, I tried this approach some time ago but it didn't look really good (for the first page), but it's really nice to have to render threads that aren't visible. Just for the record :)
      (Steve) If we only batch the threads for rendering, the effect is limited since bottleneck should be in cursor iteration. So maybe moving the getThreads action forward with batch rendering could bring better result.
      (Oleg) yeah, totally. Saving some reflows only didn't give much benefits.
      (Julien) note that we should have a feedback from QC soon with the current master code already (so with DOMContentLoaded change).
      (Steve) Ya, but they need ~500ms improvement :p , so I might take additional time for some more experiment
      (Julien) okay. I run out of ideas personally ;)
      (Steve) That's my last thought in my mind :-/
      (Julien) crossing fingers ;)

bug 1051792 - [CMAS] First UI implementation

    • Take some time to playing the app and discussion with Jenny. The UX spec seems not confirm yet because of attention screen behavior. If we press home button while screen on, the new behavior is the bar will dismiss after timeout and insert a irremovable notification in system tray... And this conflict with our behavior.
      (Julien) should be easy to do, except the "irremovable" part. But Wilfred told me: "anything that is easiest to do" for CMAS, so maybe additional behaviors should be kept for v2.2. We can discuss on the bug or by mail, let's not hold this meeting for this !
      (Steve) ok!

Today:

  • Clean review queue.
  • For QC blocker: Trying another idea to getThread earlier.

Julien

  • performance stuff: bug 1038176
    • replace the "load" event with "DOMContentLoaded" in startup.js: landed. However, I saw that it does not play well with Steve's patch, at least on Buri. So now I don't really know..
    • reviewed Steve's patches (both using asyncStorage and localStorage). The first patch from few days ago gave a better impression, but was still using "load" event.
  • CMAS POC
    • System app part: landed
    • asked review for the application architecture and minimal UI
  • Reviewed bug 1040271: some change in how we use mozL10n by the l10n team
    • moved the review to Oleg because I don't have a dev Flame with 2 slots anymore :/
  • Reviewed bug 1041124: use EventDispatcher in MessageManager: r+ \o/

Others:

  • My v2.0 flame behaves strangely: does not show the SIM number in the send button. Will investigate today, Doug does not reproduce.
  • I broke the 2G slot of my other flame :(

Today:

  • reviews
    • especially need to look at the "Predefined message" patch; still haven't :(
  • possibly move forward the WIP for thread rendering
  • or take another bug

Oleg

  • bug 1041124 - [Messages][Refactoring] Make use of EventDispatcher in MessageManager
    • Fixed last review comments and landed (landed).
  • bug 1048362 - [Messages] Compose Panel refresh (highlight actionable part)
    • Answered Steve's questions (in review).
  • bug 1041765 - [Messages] Thread view redesign
    • Have 80% WIP patch, like that message bubble markup became a bit easier (in progress);
    • Still have several questions regarding to the latest spec especially about "not-downloaded" message styling. NI'ed Fang on that, will continue work on it once get answers.
    • Also per latest assets looks like we're changing placeholder thumbnail size once again :(

Today:

  • Will handle review comments for the patches that currently in review;
  • Will look into new v2.0 nominations we have, will pick up something.
  • Will make functional review for report panel for "bug 1040271 - [L10n][SMS] Clean up mozL10n.get uses in SMS app"
  • Will prepare batch deletion PR, since my MessageManager patch is landed now.

Luke

  • bug 1050682 - [Messages] The recipient container will remain multiline style when removing the last recipient from opened panel
    • added an unit test

Today:

  • bug 1047475 - [Messages] The suggestions panel scroll position should be reset after it's been hidden
    • investigate and study thread_ui.js

Day 8: 14th August

Steve

  • bug 983172 - Parsing jpeg header information for downsampling the image for thumbnail
    • Creating a testing patch for UX about the square thumb.
  • bug 1048841 - [Messages] investigate performance hit from IndexedDB at startup (eg: Contact lookup)
    • Moving getThreads before loading complete seems not improve much because it also increases cursor traversal time, so merely 1 or 2 thread item faster than original(about ~100 or less). Considering the size of changing, this might not a good changes for 2.0 as well...
    • Other small scale trying:
      • Moving setContact/render draft to timeout
      • Avoid any styling changes ( clean up container innerHTML, setEmpty)
      • Delay l10n settings in startup
      • Moving the render thread at the beginning of threadlist init
      • Delay timeheader update
    • We might be able to save additional ~200ms totally... But it looks really ugly because everything flashing after first panel complete :p
      (Julien) yeah, it's not really appropriate :(
      (Oleg) I'm really eager to see side by side video from QC
      (Steve) Actually I have no idea why we need to compete with android kk message... they are not using result on 1.3 as stander
      (Julien) in their tests, v1.3 is even faster then Android, but I doubt it... I'll check it out today, keeping only 80 threads like they do.

bug 1051792 - [CMAS] First UI implementation

    • Sorry that I haven't leave any comment about it yet...Still spend some time with Alive that if he accept these 'tricks' for how we dealing with notification
      (Julien) I don't really follow what is changing now. Maybe we can follow up on IRC about this.
      (Steve) Sure

bug 1045493 [Messages] It's impossible to activate recipients input field once all recipients were deleted in expanded mode

    • This one is related to bug 1050682, but we will need to created another 2.0 specific patch for fixing. Will checkout if we need to solve this issue in 2.0(I think we should anyway)
      (Julien) The issue happens since v1.3 so I don't think it will be a blocker. We'll see.
      (Steve) ya , let release mnt decides

Today:

  • Clean review queue.
  • For QC blocker: Trying another idea to getThread earlier.

Julien

  • performance stuff: bug 1038176
    • nothing new yesterday
    • today: will compare again v1.3 vs master (and also v1.3 SMS on gaia master vs master) to see if we can gain something elsewhere than in the SMS app.
  • CMAS POC
    • asked review for the application architecture and minimal UI
  • bug 976091: show the sim information in the message bubble
    • produced a first patch, waited for some Jenny's answers
    • good news: Jenny's answers match the patch ;) will ask review today then

Others:

  • My v2.0 flame behaves strangely: does not show the SIM number in the send button. Found the issue: this is bug 1043662. Should be fixed shortly. Issue is that the "hacky" properties we added in v2.0 are lost in multilocale builds.
  • bug 1046828: the "show" event for a notification should never launch the app (affecting the reboot process when some remaining notifications need to be displayed). :mhenretty helped me with the tests so should be able to land soon.
  • Tomorrow is bank holiday in France !

Today:

  • reviews
    • especially need to look at the "Predefined message" patch; still haven't :(
    • will start with this today
  • ask review on bug 976091
  • investigate the performance issue from some other angle
  • possibly move forward the WIP for thread rendering

Oleg

  • bug 1048362 - [Messages] Compose Panel refresh (highlight actionable part)
    • Got r+, waiting for try Gij (almost landed).
  • bug 1041765 - [Messages] Thread view redesign
    • No updates from Fang yet.
  • bug 1052442 - [sms] We should never close the activity when viewing the message report
    • Prepared small css-only patch to fix this for both participants and report panels (awaiting for feedback);
    •  :dietrich asked for test that covers this behaviour, it's actually a good idea to make marionette test for the app in activity mode, but I'm not sure whether we have done something like this already.
      (Julien) maybe in the python tests, but it would be nice to have some JS tests too (the python tests are really for the "main" things).
      (Oleg) mmm, not sure how to make JS test for this css only fix... Do you have something in mind?
      (Julien) I meant, marionette JS test :)
      (Oleg) Ah, my bad, never touched it, will look what I can do :)
      (Julien) we have basically everything to do; in most apps there is a small lib to have useful functions for common operations. Eg "tap_new_message_button" etc. You can have a look to the python tests for SMS for inspiration too, their lib is quite good and make the tests easy to follow.
      (Oleg) Yeah, I remember python libs when I was dealing with python test :) Ok, so just one more question, is marionette JS is something that we're going to use in the long term?
      (Julien) yep definitely.
      (Oleg) nice, thanks!
  • bug 1040271 - [L10n][SMS] Clean up mozL10n.get uses in SMS app
    • Small functional review, looked fine to me, r+'ed.

Other:

  • Finished batch deletion patch, updating unit tests, trying to get rid off MockDialog in favour of plain sinon stubs from ThreadListUI.delete tests, will see what happens :)

Today:

  • Will handle review comments for the patches that currently in review;
  • Will write a marionette JS test for bug 1052442!
  • Will ask feedback for my subject handling patch (VR 2.1).

Luke

  • bug 1050682 - [Messages] The recipient container will remain multiline style when removing the last recipient from opened panel
    • landed on master
      (Julien) \o/ first patch in Messages? :)
      (Luke) second. :p (I tried to fix a simple bug last weekend.)
  • bug 1047475 - [Messages] The suggestions panel scroll position should be reset after it's been hidden
    • review requested

Today:

  • bug 1045493 - [Messages] It's impossible to activate recipients input field once all recipients were deleted in expanded mode
    • investigate

Day 9: 15th August

Steve

  • bug 983172 - Parsing jpeg header information for downsampling the image for thumbnail
    • Pending for next sprint :-/.
  • bug 1048841 - [Messages] investigate performance hit from IndexedDB at startup (eg: Contact lookup)
    • Although delay the contact/draft fetching will improve the performance a little bit, and the UX is really bad and Jenny reject this idea :p
    • Discussion with vicamo about the indexedDB issue, except for the IPC performance regression issue, maybe returning a batch of threads is a idea because of reduces the iteration time, but this involves API changes. So it might not happens in near future.
    • However, if we could put the each iteration action into timeout and continue immediately, maybe we can finish the cursor iteration ASAP and let system to schedule rendering instead.

bug 1051792 - [CMAS] First UI implementation

    • Feedback given and r+, will rebase the commits and merge to master

Today:

  • Clean review queue.
  • For QC blocker: Trying to do the 'last' experiment to improve the getThreads and rendering result.

Oleg

  • bug 1048362 - [Messages] Compose Panel refresh (highlight actionable part)
    • Landed (landed).
  • bug 1053952 - [Messages][Refactoring] Delete all messages at once when deleting threads
    • Prepared patch with updated unit tests, attached to the bug, waiting for travis, will send for review as soon as Travis is green.
    • Posted my measurements to the bug.
  • bug 1052442 - [sms] We should never close the activity when viewing the message report
    • Landed fix itself, will handle marionette test in the separate "bug 1053964 - [Messages][Tests] Add marionette JS test case to verify close\back button behaviour when app is in activity mode"
  • bug 1048845 - [Messages] Compose Panel refresh (subject behavior change)
    • Rebased on the latest master and asked Steve for early feedback, it's v2.2, so it's not time critical.

Today:

  • Will handle review comments for the patches that currently in review;
  • Will write a marionette JS test for bug 1053964!
  • Review DSDS VR patch from Julien;
  • Will work on "bug 1051852 - [Messages] use 'DOMContentLoaded' instead of 'load' to init the various resources"

Luke

  • bug 1047475 - [Messages] The suggestions panel scroll position should be reset after it's been hidden
    • writing unit test

Today:

  • bug 1052424 - [sms]cursor behaviour is not proper
    • investigate

Julien

  • Absent/no report

Day 10: 18th August

Steve

  • bug 983172 - Parsing jpeg header information for downsampling the image for thumbnail
    • Make a quick patch about square thumbnail for visual team. If they prefer the square image more, they will also provide a square attachment place holder.
  • bug 1048841 - [Messages] investigate performance hit from IndexedDB at startup (eg: Contact lookup)
  • bug 1051792 - [CMAS] First UI implementation
    • Patch landed on master

Today:

  • Clean review queue.
  • For QC blocker: Trying another idea to getThread earlier.

Julien

  • performance stuff: bug 1038176
    • compared again various cases (v1.3/master SMS vs v1.3/master homescreen vs v1.3/master System+Gecko). Looks like the app's performance on master is as good as the app's performance in v1.3.
    • Some work maybe needed in Gecko?
      (Oleg) just quick question, did we use app_usage in v1.3? I see some logs from it on startup just wondering whether it can affect it somehow.
      (Julien) it's in System, right? I didn't see much difference in the System part... but I may be wrong here.
      (Oleg) Yeah, system/app_usage_metrics, probably it doesn't affect just noticed :)
  • CMAS POC
    • fixed review comments and landed !
  • bug 976091: show the sim information in the message bubble
    • asked for review, will fix review comments today

Others:

  • v2.0 production builds do not show the SIM number in the send button (bug 1043662). Looks like the fix didnd't fix anything. Will need someone from you to look after this bug while I am away.
  • Holidays starting Wednesday.

Today:

  • fix comments on bug 976091 and ask for a new review
  • reviews
    • still the "Predefined message" patch; still haven't :(
  • possibly take a blocker, but I need to be able to finish it by tomorrow
  • possibly move forward the WIP for thread rendering

Oleg

  • bug 1053952 - [Messages][Refactoring] Delete all messages at once when deleting threads
    • Have patch ready, not sure whether we need this right now. If you guys think yes, then I ask for review :) (ready)
      (Julien) depends on "riskyness" :) we can still review now and land after branching if it's risky.
      (Oleg) Ok, will probably ask Steve, as I sent several feedback requests to you :)
  • bug 1054004 - [Messages] There is a possible race condition between navigator.mozL10n.ready and "DOMContentLoaded"
    • Prepared two WIPs for feedback. Once with solution suggested by Julien in the original bug and the second one is the one that tries to respect drafts and mozL10n readiness while firing performance events (awaiting for feedback).
      (Steve) Except for the no threads case, is there any case that we need l10n ready first?
      (Julien) we need l10n for the time format :( (ah yeah, wrong bug probably :) )
      (Oleg) Basically it's really a global problem, I just put some timeout inside l10n (when loading localisation files) and everything went wrong (time formats, labels in dialog), i didn't test much, but once l10n is ready, the majority localizations issues weren't fixed automatically :)
      (Steve) It sounds like DOMContentLoaded didn't wait for localization files
      (Oleg) I would say that load doesn't wait too, it just less obvious, as far as I understand load just waits for file
      (Julien) agreed; we have this race since a long time ago (I remember we used to wait for the "ready" event before doing any init, but it was unnecessary).
      (Steve) Ah, yeah you are right, I remember this
      (Oleg) So my main question here, does l10n lib has some "performance" metrics that we should expect?
      (Julien) the l10n lib was completely changed between v1.4 and v2.0; performance should be approximately the same but maybe it regressed.
      (Oleg) I mean if for some reason it takes longer to load locale file via XHR, should it be problem of l10n lib or we should take care of such delays? Like we do in this patch
      (Julien) in production mode, locale should be already part of index.html; but still it looks like that in some cases if we're really fast we can have issues. (I tried master SMS on faster gecko v1.3, and got the issue)
      (Oleg) I just see that l10n lib loads some stuff via XHR that is async, that potentially can delay. Ok, so we expect that it should be fast enough
      (Julien) yes. And as I said on production there should be no XHR, at least for l10n-id used in the first panel, but maybe this changed... this was the case in v1.4.
  • bug 1053964 - [Messages][Tests] Add marionette JS test case to verify close\back button behaviour when app is in activity mode
    • Got acquainted with Marionette JS tests, so it took some time :) I've prepared patch that covers discussed test case and asked for feedback (awaiting for feedback).
  • bug 976091 - [Messages][DSDS] Show the SIM information in the message bubble
    • Reviewed, code looks good, but I noticed that SIM info is shown on Flame with single SIM inserted, asked to look into it.

Today:

  • Will handle review comments for the patches that currently in review;
  • Will ping Fang on questions regarding Thread View redesign, so it should be ready for the next sprint planning.

Luke

  • bug 1047475 - [Messages] The suggestions panel scroll position should be reset after it's been hidden
    • landed
    • it is asked a blocker for v2.0
  • bug 1045493 - [Messages] It's impossible to activate recipients input field once all recipients were deleted in expanded mode
    • review requested

Today:

Demos

bug 976091: [Messages][DSDS] Show the SIM information in the message bubble

  • Before

  • After

bug 1051792: [CMAS] First UI implementation

  • Attention Screen

  • Minimized Attention Screen

  • Notification

Notifications are silent (means: no vibration, no sound, no banner) and are always shown on top: implemented in bug 1051788.

bug 1048204: The back button seems like pressed even without pressed

  • Before

  • After

bug 1048362: Compose Panel refresh (highlight actionable part)

  • Before

  • After

Fixed notification position

  • Before

  • After

bug 1052442: We should never close the activity when viewing the message report

  • Before

  • After

Retrospective

Previous sprints' actions

  • subdivise bigger bugs in subtasks for planning only
  • divide the refresh in smaller prioritizable tasks
    • I think we still took most of refresh tasks in 2.1 :p
    • yes, this is good :) dividing was in case we couldn't :D
    • still I don't think you'll be able to do the thread view redesign when I'll be in holidays.
    • I hope we'll do :) If spec is finished in time of course
    • Oleg, does the visuals answer satisfy you? :D
    • Yeah, only not-downloaded spec is left
    • Just remember other 2.1 tasks have higher priority :) If you have time in the end, then fine. (also remember any big patch can lead to regresion :) )
    • Yeah, anyway thread redesign is committed 2.1 feature
    • really? I think that's something we removed from 2.1, but you're right it still has the "2.1" flag. Ah yeah, you're right, I just checked. It was removed only in my head. :)
    • I remember that we removed from 2.1 subject handling and few not really important bugs
    • We could discuss this with Wifred, but surely we have to finish the CMAS/24h first
    • in my first proposal to Wilfred I removed it, but we added it back in the end, that's why I was confused.

What was good in the last sprint

  • we finished it. We're better and better with estimates :D +1

What was bad in the last sprint

  • 2.0 performance blocker !!!
    • Seem like we could never resist the request from QC ;)
  • some design spec was not completely precise _but_ Fang and Jenny answer quickly so it's fine.
    • Yeah, now it's much faster :)
Retrieved from "https://wiki.mozilla.org/index.php?title=Gaia/SMS/Scrum/2.1S2&oldid=1006967"