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

Issue 576483003: Make proto enums canonical for ReadState/WriteState/ConnectState. (Closed)

Created:
6 years, 3 months ago by Kevin M
Modified:
6 years, 3 months ago
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Make protobuf enums canonical for ReadState/WriteState/ConnectState. Delete redundant non-proto ReadState/WriteState/ConnectState enums. Add cast protogen rule to extension registration deps. BUG= Committed: https://crrev.com/f1624ddce4b88a5a950f65b511677b914c9caefb Cr-Commit-Position: refs/heads/master@{#295602}

Patch Set 1 #

Total comments: 4

Patch Set 2 : Mirrored change in GN; moved conversion functions to logger_util #

Patch Set 3 : Merge in changes from master. #

Patch Set 4 : Moved proto gen rule to api/common to remove gyp cycle. #

Patch Set 5 : Moved proto files under extensions/common/api/cast_channel #

Patch Set 6 : Add missing BUILD.gn and proto files. #

Patch Set 7 : Update all path references to generated protobufs. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+156 lines, -502 lines) Patch
M extensions/browser/BUILD.gn View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M extensions/browser/api/api_registration.gyp View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
D extensions/browser/api/cast_channel/BUILD.gn View 1 2 3 4 1 chunk +0 lines, -13 lines 0 comments Download
M extensions/browser/api/cast_channel/cast_auth_util_nss.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
D extensions/browser/api/cast_channel/cast_channel.proto View 1 2 3 4 1 chunk +0 lines, -91 lines 0 comments Download
M extensions/browser/api/cast_channel/cast_channel_api.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M extensions/browser/api/cast_channel/cast_framer.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M extensions/browser/api/cast_channel/cast_framer_unittest.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M extensions/browser/api/cast_channel/cast_message_util.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M extensions/browser/api/cast_channel/cast_socket.h View 1 2 3 4 5 6 4 chunks +7 lines, -36 lines 0 comments Download
M extensions/browser/api/cast_channel/cast_socket.cc View 1 2 3 4 5 6 26 chunks +71 lines, -185 lines 0 comments Download
M extensions/browser/api/cast_channel/cast_socket_unittest.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M extensions/browser/api/cast_channel/logger.h View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M extensions/browser/api/cast_channel/logger_util.h View 1 2 3 4 5 6 1 chunk +6 lines, -3 lines 0 comments Download
M extensions/browser/api/cast_channel/logger_util.cc View 1 2 3 4 5 6 2 chunks +46 lines, -1 line 0 comments Download
D extensions/browser/api/cast_channel/logging.proto View 1 2 3 4 1 chunk +0 lines, -149 lines 0 comments Download
M extensions/common/api/BUILD.gn View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M extensions/common/api/api.gyp View 1 2 3 4 1 chunk +15 lines, -0 lines 0 comments Download
A + extensions/common/api/cast_channel/BUILD.gn View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
A + extensions/common/api/cast_channel/cast_channel.proto View 1 2 3 4 5 0 chunks +-1 lines, --1 lines 0 comments Download
A + extensions/common/api/cast_channel/logging.proto View 1 2 3 4 5 0 chunks +-1 lines, --1 lines 0 comments Download
M extensions/extensions.gyp View 1 2 3 3 chunks +2 lines, -17 lines 0 comments Download

Messages

Total messages: 23 (6 generated)
Kevin M
6 years, 3 months ago (2014-09-15 20:49:10 UTC) #2
Ken Rockot(use gerrit already)
gyp lgtm
6 years, 3 months ago (2014-09-16 00:35:08 UTC) #3
Kevin M
Adding Mark to R= list.
6 years, 3 months ago (2014-09-16 17:52:03 UTC) #5
mark a. foltz
This will entangle the socket implementation and the logging, but I don't see us removing ...
6 years, 3 months ago (2014-09-16 18:09:21 UTC) #6
Wez
LGTM w/ the current crop of comments resolved. https://codereview.chromium.org/576483003/diff/1/extensions/browser/api/api_registration.gyp File extensions/browser/api/api_registration.gyp (right): https://codereview.chromium.org/576483003/diff/1/extensions/browser/api/api_registration.gyp#newcode24 extensions/browser/api/api_registration.gyp:24: '<(DEPTH)/extensions/extensions.gyp:cast_channel_proto', ...
6 years, 3 months ago (2014-09-16 18:22:42 UTC) #7
Kevin M
Also added imcheng to R=. https://codereview.chromium.org/576483003/diff/1/extensions/browser/api/api_registration.gyp File extensions/browser/api/api_registration.gyp (right): https://codereview.chromium.org/576483003/diff/1/extensions/browser/api/api_registration.gyp#newcode24 extensions/browser/api/api_registration.gyp:24: '<(DEPTH)/extensions/extensions.gyp:cast_channel_proto', On 2014/09/16 18:22:42, ...
6 years, 3 months ago (2014-09-16 19:50:33 UTC) #9
imcheng
lgtm Thanks!
6 years, 3 months ago (2014-09-16 23:47:55 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/576483003/40001
6 years, 3 months ago (2014-09-16 23:56:07 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/56832) mac_chromium_compile_dbg on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg/builds/16154) mac_chromium_rel_swarming ...
6 years, 3 months ago (2014-09-17 00:01:50 UTC) #14
Kevin M
Hi Ken (Rockot), I had to refactor the gyps somewhat to remove a circular gyp ...
6 years, 3 months ago (2014-09-17 17:40:38 UTC) #15
Ken Rockot(use gerrit already)
On 2014/09/17 17:40:38, kmarshall wrote: > Hi Ken (Rockot), I had to refactor the gyps ...
6 years, 3 months ago (2014-09-18 18:59:36 UTC) #16
Kevin M
On 2014/09/18 18:59:36, Ken Rockot wrote: > On 2014/09/17 17:40:38, kmarshall wrote: > > Hi ...
6 years, 3 months ago (2014-09-18 20:47:46 UTC) #17
Ken Rockot(use gerrit already)
On 2014/09/18 20:47:46, kmarshall wrote: > On 2014/09/18 18:59:36, Ken Rockot wrote: > > On ...
6 years, 3 months ago (2014-09-18 20:51:36 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/576483003/100001
6 years, 3 months ago (2014-09-18 21:22:34 UTC) #20
commit-bot: I haz the power
Committed patchset #6 (id:100001) as 8d8d6868f5176f3c4ae735f5f864b8ca9521973d
6 years, 3 months ago (2014-09-18 23:38:49 UTC) #21
commit-bot: I haz the power
Patchset 6 (id:??) landed as https://crrev.com/f1624ddce4b88a5a950f65b511677b914c9caefb Cr-Commit-Position: refs/heads/master@{#295602}
6 years, 3 months ago (2014-09-18 23:39:28 UTC) #22
Elliot Glaysher
6 years, 3 months ago (2014-09-19 00:01:49 UTC) #23
Message was sent while issue was closed.
On 2014/09/18 23:39:28, I haz the power (commit-bot) wrote:
> Patchset 6 (id:??) landed as
> https://crrev.com/f1624ddce4b88a5a950f65b511677b914c9caefb
> Cr-Commit-Position: refs/heads/master@{#295602}

Manually reverted this patch in
https://chromium.googlesource.com/chromium/src/+/4b10ffbe3ef9bcc9a1007ae12b7e...

(Couldn't autorevert because of A+ in patch.)

Powered by Google App Engine
This is Rietveld 408576698