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)
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.
Comment 1•9 years ago
|
||
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)
Assignee | ||
Comment 2•9 years ago
|
||
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)
Comment 3•9 years ago
|
||
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.
Comment 4•9 years ago
|
||
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)
Assignee | ||
Comment 5•9 years ago
|
||
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!
Assignee | ||
Comment 6•9 years ago
|
||
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)
Assignee | ||
Comment 7•9 years ago
|
||
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)
Comment 8•9 years ago
|
||
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)
Comment 9•9 years ago
|
||
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.
Comment 10•9 years ago
|
||
Adding qawanted to try make reference workload heavy on everything (not just messages).
Keywords: qawanted
Comment 11•9 years ago
|
||
Messages and Contacts should be enough (use "APP=communications/contacts" for contacts).
Assignee | ||
Comment 12•9 years ago
|
||
(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.
Comment 13•9 years ago
|
||
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.
status-b2g-v2.2:
--- → unaffected
status-b2g-master:
--- → affected
Flags: needinfo?(jmercado)
Keywords: qawanted
QA Contact: pcheng
Updated•9 years ago
|
Flags: needinfo?(jmercado)
Comment 14•9 years ago
|
||
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.
Comment 15•9 years ago
|
||
Julien the changes for bug 1162030 seem to have caused this issue. Can you please take a look?
Comment 16•9 years ago
|
||
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+
Assignee | ||
Comment 18•9 years ago
|
||
(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)
Comment 19•9 years ago
|
||
Comment 20•9 years ago
|
||
(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)
Assignee | ||
Comment 21•9 years ago
|
||
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 22•9 years ago
|
||
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)
Assignee | ||
Comment 23•9 years ago
|
||
(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?
Updated•9 years ago
|
Assignee: nobody → azasypkin
Assignee | ||
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 24•9 years ago
|
||
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 25•9 years ago
|
||
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)
Assignee | ||
Comment 26•9 years ago
|
||
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 27•9 years ago
|
||
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+
Assignee | ||
Comment 28•9 years ago
|
||
(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
Assignee | ||
Comment 29•9 years ago
|
||
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?
Comment 30•9 years ago
|
||
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.
Description
•