Closed
Bug 1159310
Opened 9 years ago
Closed 9 years ago
remember the push count and last push time for push events
Categories
(Core :: DOM: Notifications, defect)
Core
DOM: Notifications
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: dougt, Assigned: dougt)
Details
Attachments
(2 files)
2.62 KB,
patch
|
lina
:
review+
|
Details | Diff | Splinter Review |
2.39 KB,
patch
|
dougt
:
review+
|
Details | Diff | Splinter Review |
In order to provide some more information to the user how about push events are being used, I'd like to start remembering the number of pushes and the last push time.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8598744 -
Flags: review?(kcambridge)
Comment 2•9 years ago
|
||
Comment on attachment 8598744 [details] [diff] [review] 0001-Bug-1159310-Remember-the-push-count-and-last-push-ti.patch Review of attachment 8598744 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! It'd be nice if the standard `PushEvent` interface exposed this, too. ::: dom/push/PushService.jsm @@ +1334,5 @@ > aPushRecord.version < aLatestVersion) { > debug("Version changed, notifying app and updating DB"); > aPushRecord.version = aLatestVersion; > + aPushRecord.pushCount = aPushRecord.pushCount + 1; > + aPushRecord.lastPush = new Date().getTime(); Noting for the future: this can be set to the `Last-Modified` header value once we switch to Web Push.
Attachment #8598744 -
Flags: review?(kcambridge) → review+
Comment 4•9 years ago
|
||
sorry had to back this out Doug for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=9393848&repo=mozilla-inbound
Flags: needinfo?(dougt)
Comment 6•9 years ago
|
||
Sorry, I should've caught this in review. Fixed the relevant tests to only compare the fields we care about, instead of using `deepEqual`.
Flags: needinfo?(dougt)
Attachment #8599277 -
Flags: review?(dougt)
Assignee | ||
Updated•9 years ago
|
Attachment #8599277 -
Flags: review?(dougt) → review+
Assignee | ||
Comment 7•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3917987da44f
Keywords: checkin-needed
Updated•9 years ago
|
Flags: in-testsuite+
https://hg.mozilla.org/integration/mozilla-inbound/rev/12e0846f854f https://hg.mozilla.org/integration/mozilla-inbound/rev/b995a74f067b
Keywords: checkin-needed
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/12e0846f854f https://hg.mozilla.org/mozilla-central/rev/b995a74f067b
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•