Closed
Bug 821604
Opened 12 years ago
Closed 10 years ago
Don't start periodic sync until we have a calendar that needs a real sync operation.
Categories
(Firefox OS Graveyard :: Gaia::Calendar, defect, P2)
Firefox OS Graveyard
Gaia::Calendar
Tracking
(blocking-b2g:2.0M+, blocking-basecamp:-, tracking-b2g:+, b2g18+, b2g-v2.0 affected, b2g-v2.0M fixed, b2g-v2.1 affected, b2g-v2.1S affected, b2g-v2.2 fixed, b2g-master fixed)
RESOLVED
FIXED
2.1 S8 (7Nov)
People
(Reporter: jlal, Assigned: gaye)
References
Details
(Keywords: perf, verifyme, Whiteboard: [priority][p=8][c= p= s= u=])
Attachments
(4 files)
This has a CPU/battery impact as we wake up the device to attempt to sync when there are no real calendars we need to sync.
Reporter | ||
Comment 1•12 years ago
|
||
Followup from periodic sync there is code that does very similar things in modify event and settings. We basically need to watch for new calendars to be added then turn on/off periodic sync as need.
blocking-basecamp: --- → ?
Keywords: perf
Updated•12 years ago
|
blocking-basecamp: ? → +
Priority: -- → P3
Updated•12 years ago
|
Assignee: nobody → cbrocious
Updated•12 years ago
|
Target Milestone: --- → B2G C3 (12dec-1jan)
Comment 2•12 years ago
|
||
James, can you clarify the CPU and battery impact?
Updated•12 years ago
|
tracking-b2g18:
--- → ?
Comment 3•12 years ago
|
||
Hmm...I'm going to renom. Not sure if this blocks unless there's enough evidence of significant risk here.
blocking-basecamp: + → ?
Reporter | ||
Comment 4•12 years ago
|
||
Without this work even though there are no calendars that _need_ to be synced (like the offline one) we will wake up the calendar (and cpu / wifi) every N minutes (defaults to on and 15).
Obviously we need to do this for users with synced calendars so the cost is not massive but simply excessive for users who don't need this functionality. We essentially drain they battery for no reason. I suspect the drain is minimal but everything adds up.
Given our market it may even make sense to have periodic sync off by default with or without the fix suggested here.
Cody I think has started on this so b- or + we may end up with this functionality as a pending patch.
Updated•12 years ago
|
blocking-basecamp: ? → -
Comment 5•12 years ago
|
||
The pull request is at https://github.com/mozilla-b2g/gaia/pull/7134
The test is very simple: add a caldav account and see that periodic sync is enabled, then remove it and see that sync is disabled.
Attachment #694764 -
Flags: review?(jlal)
Reporter | ||
Comment 6•12 years ago
|
||
Comment on attachment 694764 [details] [diff] [review]
Implemented functionality and test
Reviewed this awhile ago there are some comments that need to be addressed on GH but the approach makes sense...
Attachment #694764 -
Flags: review?(jlal) → review+
Updated•12 years ago
|
Assignee: cody.brocious+bugzilla → jlal
Reporter | ||
Comment 7•12 years ago
|
||
I will pick this up for next release (post Jan 15)
Reporter | ||
Updated•12 years ago
|
Assignee: jlal → nobody
Comment 8•12 years ago
|
||
I believe this bug to be causing calendar app to be started automatically without user intention.
Within 5-10 minutes from starting fresh profile gaia I see calendar in cardview without ever starting it.
Updated•12 years ago
|
Whiteboard: [FFOS_perf]
Comment 10•12 years ago
|
||
Sorry, ignore blocking req until comment 8 is addressed.
blocking-b2g: tef? → ---
Comment 12•12 years ago
|
||
We've already got a r+ patch here...don't think it's worthwhile to do QA investigation. Let's rebase the patch and get it landed.
Keywords: qawanted
Comment 13•12 years ago
|
||
James - Can you rebase the patch and get this landed?
Flags: needinfo?(jlal)
Reporter | ||
Updated•12 years ago
|
Assignee: nobody → jlal
Updated•12 years ago
|
Flags: needinfo?(jlal)
Comment 14•11 years ago
|
||
J-lal, is this long ago bitrotterdam'd?
Flags: needinfo?(jlal)
Whiteboard: [FFOS_perf] → [FFOS_perf] c=
Reporter | ||
Comment 15•11 years ago
|
||
Yep- this patch is not viable anymore... I had a conditional r+ before Cody left.
I have another patch pending review (bug 832258) which has the majority of the work needed for this.
Flags: needinfo?(jlal)
Comment 16•11 years ago
|
||
So, close this as a dup of bug 832258?
Comment 17•11 years ago
|
||
James(In reply to James Lal [:lightsofapollo] from comment #15)
> Yep- this patch is not viable anymore... I had a conditional r+ before Cody
> left.
>
> I have another patch pending review (bug 832258) which has the majority of
> the work needed for this.
So close this as a dup of bug 832258?
Flags: needinfo?(jlal)
Whiteboard: [FFOS_perf] c= → [c= ]
Reporter | ||
Comment 18•11 years ago
|
||
Ah no- this still needs some glue cleanup now that that bug has landed.
Assignee: jlal → nobody
Flags: needinfo?(jlal)
Updated•11 years ago
|
Priority: P3 → P2
Target Milestone: B2G C3 (12dec-1jan) → ---
Comment 20•11 years ago
|
||
Dylan,
Are these Calendar app improvements still wanted by the Productivity team?
FxOS Perf Triage
Flags: needinfo?(doliver)
Whiteboard: [c= ] → [c= p= s= u=]
Comment 21•11 years ago
|
||
Gareth, can you comment on what we want to do with this bug and bug 832258?
Flags: needinfo?(doliver) → needinfo?(gaye)
Assignee | ||
Comment 22•11 years ago
|
||
I just landed James' patch for 832258 on gaia master. We can go ahead and work on this now with help from some of that code. Perhaps this is the sort of thing we might want to triage?
Flags: needinfo?(gaye)
Updated•11 years ago
|
blocking-b2g: --- → backlog
Updated•11 years ago
|
Whiteboard: [c= p= s= u=] → [priority][p=8]
Updated•11 years ago
|
Whiteboard: [priority][p=8] → [priority][p=8][c= p= s= u=]
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → gaye
Assignee | ||
Updated•11 years ago
|
Target Milestone: --- → 1.4 S1 (14feb)
Assignee | ||
Updated•11 years ago
|
Target Milestone: 1.4 S1 (14feb) → ---
Assignee | ||
Updated•11 years ago
|
Assignee: gaye → nobody
Comment 23•11 years ago
|
||
(In reply to James Lal [:lightsofapollo] Less responsive until Jan 1 (please email for urgent issues) from comment #18)
> Ah no- this still needs some glue cleanup now that that bug has landed.
Hi James,
Sorry, I'm confused with this bug.
After Bug 832258 is landed, what should we do for the sync things?
Excuse me, do what cleanup?
Currently, the Calendar doesn't not wake up automatically in the gaia 02ba36a5 (2014-03-07) build.
It means that this issue is not existed.
So we could just close this bug, right?
Or anything I missed?
Thanks.
Flags: needinfo?(jlal)
Updated•11 years ago
|
Assignee: nobody → evanxd
Updated•11 years ago
|
Assignee: evanxd → nobody
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → gaye
Updated•11 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(jlal)
Assignee | ||
Comment 25•10 years ago
|
||
Haven't tested yet, but pretty sure this is what we want :)
Attachment #8509701 -
Flags: review?(mmedeiros)
Comment 26•10 years ago
|
||
Comment on attachment 8509701 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/25420
just a few nits but LGTM! nice work.
Attachment #8509701 -
Flags: review?(mmedeiros) → review+
Assignee | ||
Comment 27•10 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/3b98c6a903e79ea16a51832fa4be23f817ab5829 landed on master
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•10 years ago
|
Target Milestone: --- → 2.1 S8 (7Nov)
I had to revert this in https://github.com/mozilla-b2g/gaia/commit/eb8a140841f2fb2d8f28f45cbfd0bb053d8aa187 for Gu test failures:
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=730707&repo=b2g-inbound
Status: RESOLVED → REOPENED
Flags: needinfo?(gaye)
Resolution: FIXED → ---
Assignee | ||
Comment 29•10 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/a07994714f0552f89801d6097982308d8b0a1ee1 relanded on master and thanks Wes for catching.
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Flags: needinfo?(gaye)
Resolution: --- → FIXED
Updated•10 years ago
|
Updated•10 years ago
|
Blocks: Woodduck, Woodduck_P2
blocking-b2g: backlog → 2.0M+
Updated•10 years ago
|
Comment 32•10 years ago
|
||
Hi Gary,
This bugs has also found by partner in bug 1129278. Could you check whether we can pick it back to 2.0M?
Thanks!
Flags: needinfo?(gchen)
Comment 33•10 years ago
|
||
Comment 34•10 years ago
|
||
Hi Josh and Kai-Zhen,
I've prepared the patch for v2.0m, please help to land in v2.0m.
Thanks.
Flags: needinfo?(kli)
Flags: needinfo?(jocheng)
Flags: needinfo?(gchen)
Comment 35•10 years ago
|
||
Flags: needinfo?(kli)
Updated•9 years ago
|
QA Whiteboard: QAExclude
Flags: needinfo?(jmercado)
Updated•9 years ago
|
Flags: needinfo?(jmercado)
You need to log in
before you can comment on or make changes to this bug.
Description
•