Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(35)

Issue 1476823003: Fix notifications. (Closed)

Created:
5 years ago by ksimbili
Modified:
5 years ago
CC:
mojo-reviews_chromium.org, gregsimon, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org
Base URL:
https://github.com/domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Fix notifications. Also add vibrate, sound and loghts to notificatins. R=qsr@chromium.org, vtl@google.com Committed: https://chromium.googlesource.com/external/mojo/+/92f8038e1caa9d5518cf029daee5b45b2320189f

Patch Set 1 #

Total comments: 2

Patch Set 2 : removed vibrate and lights. #

Patch Set 3 : modified mojom to allow client to device to play sound/vibrate/lights. #

Total comments: 3

Patch Set 4 : #

Patch Set 5 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -2 lines) Patch
M mojo/dart/packages/mojo_services/lib/notifications/notifications.mojom.dart View 1 2 3 chunks +29 lines, -2 lines 0 comments Download
M mojo/services/notifications/interfaces/notifications.mojom View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
M services/notifications/src/org/chromium/mojo/notifications/NotificationBuilder.java View 1 2 3 4 2 chunks +14 lines, -0 lines 0 comments Download

Messages

Total messages: 17 (7 generated)
ksimbili
5 years ago (2015-11-25 19:37:53 UTC) #2
ksimbili
5 years ago (2015-11-25 19:39:40 UTC) #3
ksimbili
5 years ago (2015-11-25 19:41:25 UTC) #5
anwilson
https://codereview.chromium.org/1476823003/diff/1/services/notifications/src/org/chromium/mojo/notifications/NotificationBuilder.java File services/notifications/src/org/chromium/mojo/notifications/NotificationBuilder.java (right): https://codereview.chromium.org/1476823003/diff/1/services/notifications/src/org/chromium/mojo/notifications/NotificationBuilder.java#newcode63 services/notifications/src/org/chromium/mojo/notifications/NotificationBuilder.java:63: notificationBuilder.setSound(alarmSound); vibrating, blinking led, and playing a sound might ...
5 years ago (2015-11-25 19:45:39 UTC) #7
ksimbili
https://codereview.chromium.org/1476823003/diff/1/services/notifications/src/org/chromium/mojo/notifications/NotificationBuilder.java File services/notifications/src/org/chromium/mojo/notifications/NotificationBuilder.java (right): https://codereview.chromium.org/1476823003/diff/1/services/notifications/src/org/chromium/mojo/notifications/NotificationBuilder.java#newcode63 services/notifications/src/org/chromium/mojo/notifications/NotificationBuilder.java:63: notificationBuilder.setSound(alarmSound); On 2015/11/25 19:45:39, anwilson wrote: > vibrating, blinking ...
5 years ago (2015-11-25 23:04:06 UTC) #9
qsr
lgtm
5 years ago (2015-11-26 08:28:48 UTC) #10
Andrew T Wilson (Slow)
https://codereview.chromium.org/1476823003/diff/40001/mojo/services/notifications/interfaces/notifications.mojom File mojo/services/notifications/interfaces/notifications.mojom (right): https://codereview.chromium.org/1476823003/diff/40001/mojo/services/notifications/interfaces/notifications.mojom#newcode15 mojo/services/notifications/interfaces/notifications.mojom:15: // Indicates if notification sound must be played when ...
5 years ago (2015-11-26 09:36:27 UTC) #12
ksimbili
Adding +vtl for permission to land
5 years ago (2015-11-26 14:50:27 UTC) #14
vtl
Rubberstamp lgtm (but qsr's lgtm was really enough). Is "git cl land" not working for ...
5 years ago (2015-11-30 17:44:32 UTC) #15
ksimbili
5 years ago (2015-11-30 19:21:51 UTC) #17
Message was sent while issue was closed.
Committed patchset #5 (id:80001) manually as
92f8038e1caa9d5518cf029daee5b45b2320189f (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698