Closed Bug 1217859 Opened 9 years ago Closed 9 years ago

[Messages] It's not possible to navigate to any conversation or create new message until Inbox is fully loaded

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.5+, b2g-v2.2 unaffected, b2g-master affected)

RESOLVED FIXED
blocking-b2g 2.5+
Tracking Status
b2g-v2.2 --- unaffected
b2g-master --- affected

People

(Reporter: azasypkin, Assigned: azasypkin)

References

Details

(Keywords: regression)

Attachments

(1 file)

STR: * Have a device with lots of conversations (e.g. "make reference-workload-heavy"); * Open Messages app and try to tap on any conversation or "new message" button. Expected result: user should be able to navigate to conversation or create new message as soon as _first page_ of conversations is loaded; Actual result: nothing happens until Inbox is _fully_ loaded.
I failed to see a difference when I compared Flame 2.5 and Flame 2.2 devices. When I cold launch Messages app (with 2000 msgs using xheavy) and immediately tap on a conversation or new message icon, both 2.5 and 2.2 wait until some sort of loading is done before it goes to the designated page. Without seeing a clear difference we are unable to do a regression window or even call it a regression. Device: Flame 2.5 BuildID: 20151023030241 Gaia: 410e91ddabc7ba82a9b43b3711a3fdf2cb8de309 Gecko: 0625c68c0abcfe4d10880d15d8fe7d06df3369c9 Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a Version: 44.0a1 (2.5) Firmware Version: v18Dv4 User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0 Device: Flame 2.2 BuildID: 20151023032506 Gaia: 885647d92208fb67574ced44004ab2f29d23cb45 Gecko: 9e0aa88fea7a Gonk: bd9cb3af2a0354577a6903917bc826489050b40d Version: 37.0 (2.2) Firmware Version: v18Dv4 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Flags: needinfo?(azasypkin)
Well, I still see the same using latest images from PVT (I'm flashing full image) + "make reference-workload-heavy": Here is video: https://www.youtube.com/watch?v=b2NVxrU35e4&feature=youtu.be (left one is master). Device Name flame v2.2 Build ID 20151023032506 Gaia Revision 885647d92208fb67574ced44004ab2f29d23cb45 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/9e0aa88fea7a Gecko Version 37.0 Build Number eng.cltbld.20151023.065326 Device Name flame master Build ID 20151023150204 Gaia Revision 410e91ddabc7ba82a9b43b3711a3fdf2cb8de309 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/0625c68c0abcfe4d10880d15d8fe7d06df3369c9 Gecko Version 44.0a1 Build Number eng.cltbld.20151023.183221 Pi Wei, do you do shallow or full flash? If shallow, could you please try with the full PVT image instead? Thanks!
Flags: needinfo?(azasypkin) → needinfo?(pcheng)
I did full flash on both 2.2 and 2.5. I was on 512MB memory on both of them. I'll try it again today.
I'm still not seeing the issue on Flame 2.5 master. Cold launching Messages app and immediately tapping on a thread or on create new message takes ~1 second to execute the action, which is about the same time as on Flame 2.2. Bug repro rate is 0 out of 7 on central. Video showing there's no difference: https://www.youtube.com/watch?v=bfRgFKT0bNc Tested on: Device: Flame 2.5 Master (512MB memory, full flashed) BuildID: 20151028030421 Gaia: 2e89362de40a6c9c36525d36317fa1ae8e67e143 Gecko: fc706d376f0658e560a59c3dd520437b18e8c4a4 Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a Version: 44.0a1 (2.5) Firmware Version: v18Dv4 User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0 Device: Flame 2.2 (512MB memory, full flashed) BuildID: 20151028032502 Gaia: 885647d92208fb67574ced44004ab2f29d23cb45 Gecko: 05144e035522 Gonk: bd9cb3af2a0354577a6903917bc826489050b40d Version: 37.0 (2.2) Firmware Version: v18Dv4 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0 What base image are you using, Oleg?
Flags: needinfo?(pcheng) → needinfo?(azasypkin)
Grrr, something is definitely wrong on my side, I'll double check with the latest image once again and report here. Thanks for double-checking Pi Wei, and sorry for the noise!
After investigation I see that it's because of the fact that we wait for the "load" event before initial navigation (Inbox view) is considered as ended and we can proceed with the following navigation request (open composer). But "load" is delayed by a huge number of blob image resources we have for contacts in heavy workload. Still trying to figure out why nobody else sees this. Pi Wei, the last question (I swear :)), do you wait enough time after "make reference-workload-heavy" before testing so that contact images starting to appear like in video from comment 2?
Flags: needinfo?(pcheng)
Hey Julien, I can't remember exactly why do we want to wait until "load" event [1] instead of something like "waitForReady()" [2] to mark initial navigation request as finished. Do you remember? Thanks! [1] https://github.com/mozilla-b2g/gaia/blob/20a6205f418ef65b97f7ee39dc1cc13096c3eedb/apps/sms/views/shared/js/navigation.js#L508 [2] https://github.com/mozilla-b2g/gaia/blob/20a6205f418ef65b97f7ee39dc1cc13096c3eedb/apps/sms/views/shared/js/navigation.js#L267
Flags: needinfo?(azasypkin) → needinfo?(felash)
I don't think I had waited for contact pictures to display. How long before it would display? I only waited long enough so the threads would show up, which is about 15 minutes(?) after make reference workload was done. And I thought that waiting was pretty long.
Flags: needinfo?(pcheng)
Also, I only make reference workload on messages. Does this require make reference workload on contacts as well? If yes then I think that's the part I didn't do.
Adding qawanted to try make reference workload heavy on everything (not just messages).
Keywords: qawanted
Messages and Contacts should be enough (use "APP=communications/contacts" for contacts).
(In reply to Pi Wei Cheng [:piwei] from comment #9) > Also, I only make reference workload on messages. Does this require make > reference workload on contacts as well? If yes then I think that's the part > I didn't do. Yes, as it turned out that the main thing here, sorry for not emphasizing this earlier.
With makereferenceworkload on both messages and contacts I can finally reproduce! :D And I clearly see a difference between 2.2 and 2.5. Thanks for figuring this out with me. I'll work on getting the regression window.
Flags: needinfo?(jmercado)
Keywords: qawanted
QA Contact: pcheng
Flags: needinfo?(jmercado)
b2g-inbound regression window: Last Working Device: Flame BuildID: 20150807071843 Gaia: 6af83bf581235920c7f769c40e3c1cfcbe329b7b Gecko: 3326bade9e59 Version: 42.0a1 Firmware Version: v18Dv4 User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0 First Broken Device: Flame BuildID: 20150807105308 Gaia: 68d369c2a2b0cd16db028f0c2e660107c52ce113 Gecko: 00f0d8a18cad Version: 42.0a1 Firmware Version: v18Dv4 User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0 Last Working Gaia First Broken Gecko - no repro Gaia: 6af83bf581235920c7f769c40e3c1cfcbe329b7b Gecko: 00f0d8a18cad Last Working Gecko First Broken Gaia - repro Gaia: 68d369c2a2b0cd16db028f0c2e660107c52ce113 Gecko: 3326bade9e59 Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/6af83bf581235920c7f769c40e3c1cfcbe329b7b...68d369c2a2b0cd16db028f0c2e660107c52ce113 Caused by changes made in Bug 1162030.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
Julien the changes for bug 1162030 seem to have caused this issue. Can you please take a look?
Blocks: 1162030
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado)
Yeah this was expected from a discussion I had with Oleg on IRC :) As Oleg said this happens because we wait for the 'load' event before doing any navigation and this is delayed by loading all blob images. The fix is to _not_ wait for the 'load event' and use instead a more app-specific trigger. There is still something not clear to me: if we didn't get the 'load' event yet, we should not have access to the app because the splash screen would still be displayed by the System app. Or the 'load' event is sent but delayed because the app's thread is busy. So it could still be another reason, and this might need more investigation. Hey triagers, this will happen especially when people have a lot of threads (and especially contact pictures, if I understood properly). Your call for blocking :)
blocking-b2g: --- → 2.5?
Flags: needinfo?(felash)
Comms triage: Functional regression that happened in 2.5.
blocking-b2g: 2.5? → 2.5+
(In reply to Julien Wajsberg [:julienw] from comment #16) > There is still something not clear to me: if we didn't get the 'load' event > yet, we should not have access to the app because the splash screen would > still be displayed by the System app. Or the 'load' event is sent but > delayed because the app's thread is busy. It's seems that splash screen can be dismissed earlier, by system timeout [1] - after 2500ms in the opening state splash screen is dismissed; However even if I increase this timeout (e.g. 25000000), I still see that splash screen is dismissed earlier - once app enters "opening" state. Initially mozbrowser is hidden with [2], but once it enters any other state, [3] makes it visible. Etienne, please correct me if my investigations are wrong :) Thanks! [1] https://github.com/mozilla-b2g/gaia/blob/7954ff0cbd794a35499a1082bed273598f82ee6f/apps/system/js/app_transition_controller.js#L172 [2] https://github.com/mozilla-b2g/gaia/blob/7954ff0cbd794a35499a1082bed273598f82ee6f/apps/system/style/window.css#L260 [3] https://github.com/mozilla-b2g/gaia/blob/7954ff0cbd794a35499a1082bed273598f82ee6f/apps/system/style/window.css#L256
Flags: needinfo?(etienne)
(In reply to Oleg Zasypkin [:azasypkin][⏰UTC+1] from comment #18) > (In reply to Julien Wajsberg [:julienw] from comment #16) > > There is still something not clear to me: if we didn't get the 'load' event > > yet, we should not have access to the app because the splash screen would > > still be displayed by the System app. Or the 'load' event is sent but > > delayed because the app's thread is busy. > > It's seems that splash screen can be dismissed earlier, by system timeout > [1] - after 2500ms in the opening state splash screen is dismissed; > > However even if I increase this timeout (e.g. 25000000), I still see that > splash screen is dismissed earlier - once app enters "opening" state. > Initially mozbrowser is hidden with [2], but once it enters any other state, > [3] makes it visible. > > Etienne, please correct me if my investigations are wrong :) Thanks! There's tons of stuff that happens on |appopened| so we switch to this state - at the end of the opening transition for warm launches - once the app is loaded on cold launch - on cold launch, if the app isn't loaded after 2500ms, we switch The touch blocker (legacy code from when pointer-events was causing a reflow) is hooked on this state change [1]. But the black background + icon splashscreen isn't. I think it's tied to the .render class [2] which is toggled only with the loadend event. [1] https://github.com/mozilla-b2g/gaia/blob/7954ff0cbd794a35499a1082bed273598f82ee6f/apps/system/style/window.css#L290-L295 [2] https://github.com/mozilla-b2g/gaia/blob/7954ff0cbd794a35499a1082bed273598f82ee6f/apps/system/style/window.css#L22-L25
Flags: needinfo?(etienne)
Comment on attachment 8682158 [details] [gaia] azasypkin:bug-1217859-navigate-to-another-panel > mozilla-b2g:master Hey Julien, What do you think about this approach? I've left some thoughts at GitHub. Thanks!
Attachment #8682158 - Flags: feedback?(felash)
Comment on attachment 8682158 [details] [gaia] azasypkin:bug-1217859-navigate-to-another-panel > mozilla-b2g:master I'm not so sure of the approach. In my mind, the "navigation-transition-end" is used only with split views. And the fact we used the "load" event was only a fallback for this event, in cases where it was not fired (which means always currently). That means both are only used for the split views. In the case we use the old architecture, we should wait for none of these events, IMO. And not even for the ready promise. What do you think ?
Attachment #8682158 - Flags: feedback?(felash)
(In reply to Julien Wajsberg [:julienw] from comment #22) > Comment on attachment 8682158 [details] > [gaia] azasypkin:bug-1217859-navigate-to-another-panel > mozilla-b2g:master > > I'm not so sure of the approach. > > In my mind, the "navigation-transition-end" is used only with split views. > And the fact we used the "load" event was only a fallback for this event, in > cases where it was not fired (which means always currently). That means both > are only used for the split views. In the case we use the old architecture, > we should wait for none of these events, IMO. And not even for the ready > promise. > > What do you think ? Mmm, iirc we should wait for the ready promise as it indicates that all lazy loaded files are loaded - e.g. we shouldn't navigate to conversation view if conversation.js isn't loaded yet, or I'm missing something? For the navigation transition event - I'd rather remove it entirely as I mentioned on Github as it is something we are not sure about at all. What do you think?
Assignee: nobody → azasypkin
Depends on: 1200583
Status: NEW → ASSIGNED
Comment on attachment 8682158 [details] [gaia] azasypkin:bug-1217859-navigate-to-another-panel > mozilla-b2g:master Hey Julien, Does it look it better? Thanks!
Attachment #8682158 - Flags: review?(felash)
Comment on attachment 8682158 [details] [gaia] azasypkin:bug-1217859-navigate-to-another-panel > mozilla-b2g:master I have no problem with the code but I'd like a better unit test :)
Attachment #8682158 - Flags: review?(felash)
Comment on attachment 8682158 [details] [gaia] azasypkin:bug-1217859-navigate-to-another-panel > mozilla-b2g:master Added simple unit test that fails on master and doesn't with the PR - somewhat synthetic but still :)
Attachment #8682158 - Flags: review?(felash)
Comment on attachment 8682158 [details] [gaia] azasypkin:bug-1217859-navigate-to-another-panel > mozilla-b2g:master lgtm :) r=me !
Attachment #8682158 - Flags: review?(felash) → review+
(In reply to Julien Wajsberg [:julienw] from comment #27) > Comment on attachment 8682158 [details] > [gaia] azasypkin:bug-1217859-navigate-to-another-panel > mozilla-b2g:master > > lgtm :) > > r=me ! Thanks for review! Treeherder is green (based on the latest master). Master: https://github.com/mozilla-b2g/gaia/commit/26bae16182f975e6c2dc0165846cf559d154495f
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 8682158 [details] [gaia] azasypkin:bug-1217859-navigate-to-another-panel > mozilla-b2g:master [Approval Request Comment] [Bug caused by] (feature/regressing bug #): bug 1162030 [User impact] if declined: user will not be able to enter conversation or new message editor until inbox is fully loaded (noticeable in case user has a lot of conversations with contacts _with_ contact images) [Testing completed]: yes, manual and unit [Risk to taking this patch] (and alternatives if risky): relatively low [String changes made]:n/a
Attachment #8682158 - Flags: approval-gaia-v2.5?
Depends on: 1226555
Comment on attachment 8682158 [details] [gaia] azasypkin:bug-1217859-navigate-to-another-panel > mozilla-b2g:master Limiting 2.5 only to TV patches. Foxfooders getting updates from master/b2g-ota branch. Removing the uplift nomination.
Attachment #8682158 - Flags: approval-gaia-v2.5?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: