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

Issue 1078783002: Update the Notification Blink API to use integral persistent ids. (Closed)

Created:
5 years, 8 months ago by Peter Beverloo
Modified:
5 years, 8 months ago
Reviewers:
johnme, Mike West
CC:
blink-reviews, dglazkov+blink, peter+watch_chromium.org
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Update the Notification Blink API to use integral persistent ids. Chromium now maintains persistent notification ids as integers, so Blink should be updated to match this. This CL is part of a three-sided patch: [1] This CL. [2] https://codereview.chromium.org/1071203002/ [3] https://codereview.chromium.org/1072873002/ BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193707

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+68 lines, -10 lines) Patch
M Source/modules/notifications/Notification.h View 3 chunks +6 lines, -2 lines 0 comments Download
M Source/modules/notifications/Notification.cpp View 5 chunks +34 lines, -3 lines 0 comments Download
M Source/web/ServiceWorkerGlobalScopeProxy.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/web/ServiceWorkerGlobalScopeProxy.cpp View 1 chunk +10 lines, -0 lines 0 comments Download
M public/platform/modules/notifications/WebNotificationManager.h View 3 chunks +15 lines, -5 lines 0 comments Download
M public/web/WebServiceWorkerContextProxy.h View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
Peter Beverloo
Three-sided patches to change the type of an argument are painful :-(. The duplication is ...
5 years, 8 months ago (2015-04-09 16:18:40 UTC) #2
johnme
notifications lgtm, modulo comment at https://codereview.chromium.org/1072873002/diff/1/Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp#newcode41
5 years, 8 months ago (2015-04-10 11:08:44 UTC) #3
Mike West
LGTM.
5 years, 8 months ago (2015-04-10 11:12:26 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1078783002/1
5 years, 8 months ago (2015-04-14 12:48:51 UTC) #6
commit-bot: I haz the power
5 years, 8 months ago (2015-04-14 15:02:29 UTC) #7
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=193707

Powered by Google App Engine
This is Rietveld 408576698