Closed
Bug 1169576
Opened 9 years ago
Closed 8 years ago
[Messages][NG] Implement Conversation service: method for streaming joined threads and drafts list
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(b2g-master fixed)
RESOLVED
FIXED
FxOS-S6 (04Sep)
Tracking | Status | |
---|---|---|
b2g-master | --- | fixed |
People
(Reporter: azasypkin, Assigned: azasypkin)
References
Details
(Whiteboard: [p(FxOS-S5)=3][p(FxOS-S4)=3][p(FxOS-S3)=3])
Attachments
(2 files)
In this bug we'd like to implement "getThreads" (may have better name) defined in bug 1169573.
Updated•9 years ago
|
Blocks: messages-nga
Assignee | ||
Updated•9 years ago
|
No longer blocks: messages-nga
Assignee | ||
Updated•9 years ago
|
Blocks: ng-conversation-service
Assignee | ||
Updated•9 years ago
|
No longer blocks: ng-messaging-service
No longer depends on: 1169573
Summary: [Messages][NG] Implement Messaging service: method for retrieving of all threads → [Messages][NG] Implement Conversation service: method for retrieving of all threads
Whiteboard: [p=1]
Target Milestone: 2.2 S14 (12june) → ---
Assignee | ||
Updated•9 years ago
|
Summary: [Messages][NG] Implement Conversation service: method for retrieving of all threads → [Messages][NG] Implement Conversation service: method for streaming joined threads and drafts list
Updated•9 years ago
|
Blocks: sms-sprint-FxOS-S3
Whiteboard: [p=3]
Updated•9 years ago
|
Assignee: nobody → felash
Assignee | ||
Comment 2•9 years ago
|
||
Stealing while Julien is busy with navigation stuff.
Flags: needinfo?(azasypkin)
Assignee | ||
Comment 3•9 years ago
|
||
Quick update: was blocked by https://github.com/gaia-components/threads/pull/52, now back to work on this :)
Assignee: felash → azasypkin
Status: NEW → ASSIGNED
Flags: needinfo?(azasypkin)
Comment 4•9 years ago
|
||
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8636623 [details] [review] [gaia] azasypkin:bug-1169576-conversation-service-get-threads > mozilla-b2g:master Hey guys, What do you think about this PR? Second commit is for demo purposes only if you want to run it locally in Fx Desktop (http://sms.gaiamobile.org:8080/views/inbox). I still see some weird issue on Desktop sometimes - when I refresh page with ctrl-f5 bridge doesn't work as expected and it's fine when I refresh with just f5, I'm looking into this. Thanks!
Attachment #8636623 -
Flags: feedback?(schung)
Attachment #8636623 -
Flags: feedback?(felash)
Assignee | ||
Comment 6•9 years ago
|
||
Added some performance numbers to PR [1]. [1] https://github.com/mozilla-b2g/gaia/pull/31051#issuecomment-124576360
Comment 7•9 years ago
|
||
Comment on attachment 8636623 [details] [review] [gaia] azasypkin:bug-1169576-conversation-service-get-threads > mozilla-b2g:master Looks good and I think we're in the right direction despite the possible performance regression and we might need to fix/tune it in both bridge and app, great job!
Attachment #8636623 -
Flags: feedback?(schung) → feedback+
Comment 8•9 years ago
|
||
(In reply to Oleg Zasypkin [:azasypkin] from comment #5) > > I still see some weird issue on Desktop sometimes - when I refresh page with > ctrl-f5 bridge doesn't work as expected and it's fine when I refresh with > just f5, I'm looking into this. Sounds like a race in the startup code (stating the obvious) related to caches (ctrl-f5 bypass the cache and force-reloads everything).
Comment 9•9 years ago
|
||
Comment on attachment 8636623 [details] [review] [gaia] azasypkin:bug-1169576-conversation-service-get-threads > mozilla-b2g:master left comments and thoughts on github (I haven't actually tried it TBH) If we do batching I think it's better to do it in a separate bug; I'm fine landing this now and iterating on the performance. great work !
Attachment #8636623 -
Flags: feedback?(felash) → feedback+
Updated•9 years ago
|
Blocks: sms-sprint-FxOS-S4
Whiteboard: [p=3] → [p(FxOS-S4)=3][p(FxOS-S3)=3]
Updated•8 years ago
|
Blocks: sms-sprint-FxOS-S5
Whiteboard: [p(FxOS-S4)=3][p(FxOS-S3)=3] → [p(FxOS-S5)=3][p(FxOS-S4)=3][p(FxOS-S3)=3]
Assignee | ||
Comment 10•8 years ago
|
||
Comment on attachment 8636623 [details] [review] [gaia] azasypkin:bug-1169576-conversation-service-get-threads > mozilla-b2g:master Hey guys, I think it should be ready for the first round of review. I tried to keep PR as small as I could and split integration work in separate bugs, it's not used by any code yet, but you can try it on device or in Fx Desktop using my WIP branch [1]. WIP branch is based on PR + some glue changes. As you know performance numbers are quite bad, so we can't use this service in master as is. At the moment I'm measuring and experimenting with different setups (use window/iframe instead of SharedWorker, get rid of BroadcastChannel and etc.). I'll let you know the results, I hope we can find trade off approach to start using conversation service in master as soon as possible. Thanks! [1] https://github.com/azasypkin/gaia/tree/bug-xxx-use-conversation-service
Attachment #8636623 -
Flags: review?(schung)
Attachment #8636623 -
Flags: review?(felash)
Comment 11•8 years ago
|
||
Comment on attachment 8636623 [details] [review] [gaia] azasypkin:bug-1169576-conversation-service-get-threads > mozilla-b2g:master I don't have much to say; I left some comments but they're not big issues. We mostly have to find out how we can cut the performance issue...
Attachment #8636623 -
Flags: review?(felash)
Comment 12•8 years ago
|
||
Comment on attachment 8636623 [details] [review] [gaia] azasypkin:bug-1169576-conversation-service-get-threads > mozilla-b2g:master Patch looks good and I only left some minor nits. Let's see the result after rebased. Thanks!
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8636623 [details] [review] [gaia] azasypkin:bug-1169576-conversation-service-get-threads > mozilla-b2g:master Hey guys, * PR is rebased on the Steve's MessagingClient PR; * I've handled your comments in a separate commit; * I've posted all measurements and analysis I had to GitHub. I'm going to handle performance problems in the follow-up to keep this work more maintainable and reviewable :) So, please tell me what you think! Thanks
Attachment #8636623 -
Flags: review?(felash)
Comment 14•8 years ago
|
||
Comment on attachment 8636623 [details] [review] [gaia] azasypkin:bug-1169576-conversation-service-get-threads > mozilla-b2g:master Just left some nits but this looks good to me. So r=me with nits fixed and a green try (latest try didn't run properly).
Attachment #8636623 -
Flags: review?(felash) → review+
Comment 15•8 years ago
|
||
Comment on attachment 8636623 [details] [review] [gaia] azasypkin:bug-1169576-conversation-service-get-threads > mozilla-b2g:master Overall looks good, let's move on for the profiling part. Thanks!
Attachment #8636623 -
Flags: review?(schung) → review+
Assignee | ||
Comment 16•8 years ago
|
||
Thanks for review guys! Treeherder is green (except for the one failing GijTV test that seems unrelated).
Keywords: checkin-needed
Comment 17•8 years ago
|
||
Master: https://github.com/mozilla-b2g/gaia/commit/664bed482e0f33f307e6f21e1341a0320ac81f13
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-b2g-master:
--- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S6 (04Sep)
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Comment 18•8 years ago
|
||
Backed out because of regression (see bug 1199612): https://github.com/mozilla-b2g/gaia/commit/e9c57d7d952c9466548ab75f67e54f9ccf028360. Should be relanded together with PR for bug 1199612.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 19•8 years ago
|
||
Assignee | ||
Comment 20•8 years ago
|
||
Patch for bug 1199612 is r+'ed, Treeherder is green, so re-landed. Master: https://github.com/mozilla-b2g/gaia/commit/26579a3ee39c5ecd1f35a8970ad1827be402d2e1
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•