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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 658783002: Split PushMessagingStatus enum according to use case (Closed)

Created:
6 years, 2 months ago by johnme
Modified:
6 years, 2 months ago
CC:
chromium-reviews, darin-cc_chromium.org, jam, mkwst+moarreviews-ipc_chromium.org, mkwst+moarreviews-renderer_chromium.org, zea+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Split PushMessagingStatus enum according to use case This will make it easier to log these statuses to UMA. BUG=401438 Committed: https://crrev.com/a80c2557ca443cd45fe7769ea9b4e5c2d58d2710 Cr-Commit-Position: refs/heads/master@{#300104}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Style tweaks #

Total comments: 2

Patch Set 3 : Update comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -80 lines) Patch
M chrome/browser/services/gcm/push_messaging_service_impl.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/services/gcm/push_messaging_service_impl.cc View 1 6 chunks +15 lines, -18 lines 0 comments Download
M content/browser/browser_context.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/push_messaging_message_filter.h View 1 1 chunk +1 line, -1 line 0 comments Download
M content/browser/push_messaging_message_filter.cc View 3 chunks +4 lines, -4 lines 0 comments Download
M content/browser/push_messaging_router.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M content/browser/push_messaging_router.cc View 1 2 chunks +8 lines, -10 lines 0 comments Download
M content/common/push_messaging_messages.h View 2 chunks +3 lines, -3 lines 0 comments Download
M content/public/browser/browser_context.h View 1 1 chunk +1 line, -1 line 0 comments Download
M content/public/browser/push_messaging_service.h View 1 chunk +1 line, -1 line 0 comments Download
M content/public/common/push_messaging_status.h View 1 chunk +23 lines, -18 lines 0 comments Download
M content/public/common/push_messaging_status.cc View 1 chunk +8 lines, -17 lines 0 comments Download
M content/renderer/push_messaging_dispatcher.h View 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/push_messaging_dispatcher.cc View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 12 (2 generated)
johnme
content/common/push_messaging_messages.h
6 years, 2 months ago (2014-10-16 10:38:23 UTC) #2
johnme
mvanouwerkerk: please review (enum variable naming etc) jochen: please give owners approval palmer: please rubber-stamp ...
6 years, 2 months ago (2014-10-16 10:41:13 UTC) #3
palmer
IPC security rubberstamp LGTM https://codereview.chromium.org/658783002/diff/1/content/public/common/push_messaging_status.cc File content/public/common/push_messaging_status.cc (right): https://codereview.chromium.org/658783002/diff/1/content/public/common/push_messaging_status.cc#newcode15 content/public/common/push_messaging_status.cc:15: Whitespace nit: Whatever git cl ...
6 years, 2 months ago (2014-10-16 23:07:01 UTC) #4
jochen (gone - plz use gerrit)
lgtm
6 years, 2 months ago (2014-10-17 12:05:04 UTC) #5
johnme
Thanks both. Tweaked style according to git cl format. https://codereview.chromium.org/658783002/diff/1/content/public/common/push_messaging_status.cc File content/public/common/push_messaging_status.cc (right): https://codereview.chromium.org/658783002/diff/1/content/public/common/push_messaging_status.cc#newcode15 content/public/common/push_messaging_status.cc:15: ...
6 years, 2 months ago (2014-10-17 12:18:25 UTC) #6
Michael van Ouwerkerk
lgtm with nit https://codereview.chromium.org/658783002/diff/20001/content/browser/push_messaging_router.h File content/browser/push_messaging_router.h (right): https://codereview.chromium.org/658783002/diff/20001/content/browser/push_messaging_router.h#newcode22 content/browser/push_messaging_router.h:22: typedef base::Callback<void(PushDeliveryStatus /* push_messaging_status */)> Update ...
6 years, 2 months ago (2014-10-17 12:34:13 UTC) #7
johnme
https://codereview.chromium.org/658783002/diff/20001/content/browser/push_messaging_router.h File content/browser/push_messaging_router.h (right): https://codereview.chromium.org/658783002/diff/20001/content/browser/push_messaging_router.h#newcode22 content/browser/push_messaging_router.h:22: typedef base::Callback<void(PushDeliveryStatus /* push_messaging_status */)> On 2014/10/17 12:34:13, Michael ...
6 years, 2 months ago (2014-10-17 13:53:38 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/658783002/40001
6 years, 2 months ago (2014-10-17 13:55:14 UTC) #10
commit-bot: I haz the power
Committed patchset #3 (id:40001)
6 years, 2 months ago (2014-10-17 14:51:53 UTC) #11
commit-bot: I haz the power
6 years, 2 months ago (2014-10-17 14:52:35 UTC) #12
Message was sent while issue was closed. 
Patchset 3 (id:??) landed as
https://crrev.com/a80c2557ca443cd45fe7769ea9b4e5c2d58d2710
Cr-Commit-Position: refs/heads/master@{#300104}

Powered by Google App Engine
This is Rietveld 408576698