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

Issue 2448133006: Tool added to extract network traffic annotations. (Closed)

Created:
4 years, 1 month ago by Ramin Halavati
Modified:
3 years, 8 months ago
CC:
chromium-reviews, asanka
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Tool added to extract network traffic annotations. This is a clang tool that extracts all occurrences of network traffic annotattions and produces a report of annotated locations and text. BUG=656607 Review-Url: https://codereview.chromium.org/2448133006 Cr-Commit-Position: refs/heads/master@{#466281} Committed: https://chromium.googlesource.com/chromium/src/+/3051a40f7c5567d6bbee847227e7a31880fad143

Patch Set 1 #

Total comments: 84

Patch Set 2 : Comments addressed. #

Total comments: 22

Patch Set 3 : Comments addressed. #

Patch Set 4 : nits #

Patch Set 5 : nits + bug fixed when a directory was specified for extracted partial files. #

Patch Set 6 : Windows compatibility. #

Patch Set 7 : DEPS file corrected. #

Patch Set 8 : Windows running steps reduced. #

Patch Set 9 : nits #

Patch Set 10 : nits #

Total comments: 8

Patch Set 11 : nits. #

Patch Set 12 : Resolving protobuf lite and full conflict #

Patch Set 13 : nits #

Patch Set 14 : nits #

Patch Set 15 : minor bug fixed. #

Total comments: 1

Patch Set 16 : nits #

Patch Set 17 : Updated due to protobuf changes #

Patch Set 18 : Missing files added. #

Patch Set 19 : merged. #

Patch Set 20 : nits #

Patch Set 21 : nits #

Total comments: 44

Patch Set 22 : Comments addressed. #

Total comments: 6

Patch Set 23 : Compatibility issues with changes in proto and net/ files solved. #

Patch Set 24 : nits #

Total comments: 62

Patch Set 25 : Comments addressed. #

Total comments: 16

Patch Set 26 : Extractor simplified. #

Patch Set 27 : Minor bug fixed. #

Patch Set 28 : Windows compatibility issues fixed. #

Patch Set 29 : Clang tool comments addressed. #

Patch Set 30 : nits #

Patch Set 31 : Tests added. #

Patch Set 32 : nits #

Total comments: 53

Patch Set 33 : Comments addressed. #

Patch Set 34 : nits #

Total comments: 10

Patch Set 35 : Comments addressed More Windows Corrections. #

Total comments: 14

Patch Set 36 : Clang tool updated. #

Total comments: 14

Patch Set 37 : Clang tool updated. #

Total comments: 4

Patch Set 38 : Clang tool updated. #

Patch Set 39 : nits #

Patch Set 40 : Auditor replaced. #

Patch Set 41 : nits #

Patch Set 42 : DEPS updated. #

Patch Set 43 : Protobuf preparation separated. #

Patch Set 44 : Windows compatibility update. #

Total comments: 20

Patch Set 45 : Comments addressed. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+560 lines, -0 lines) Patch
M BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 1 chunk +1 line, -0 lines 0 comments Download
M third_party/protobuf/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 1 chunk +4 lines, -0 lines 0 comments Download
A tools/clang/traffic_annotation_extractor/CMakeLists.txt View 1 2 3 4 5 1 chunk +26 lines, -0 lines 0 comments Download
A tools/clang/traffic_annotation_extractor/README.md View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 1 chunk +26 lines, -0 lines 0 comments Download
A tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 1 chunk +175 lines, -0 lines 0 comments Download
A tools/traffic_annotation/auditor/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 1 chunk +49 lines, -0 lines 0 comments Download
A tools/traffic_annotation/auditor/DEPS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 1 chunk +4 lines, -0 lines 0 comments Download
A tools/traffic_annotation/auditor/README.md View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 1 chunk +27 lines, -0 lines 0 comments Download
A tools/traffic_annotation/auditor/prepare_protobuf.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 1 chunk +36 lines, -0 lines 0 comments Download
A tools/traffic_annotation/auditor/traffic_annotation_auditor.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 1 chunk +212 lines, -0 lines 0 comments Download

Messages

Total messages: 50 (14 generated)
Ramin Halavati
Please review this.
4 years, 1 month ago (2016-10-26 13:22:50 UTC) #2
battre
https://codereview.chromium.org/2448133006/diff/1/third_party/protobuf/BUILD.gn File third_party/protobuf/BUILD.gn (right): https://codereview.chromium.org/2448133006/diff/1/third_party/protobuf/BUILD.gn#newcode184 third_party/protobuf/BUILD.gn:184: "//tools/traffic_annotation/*", Turned out that this is still problematic and ...
4 years, 1 month ago (2016-10-26 14:29:20 UTC) #3
Ramin Halavati
All comments addressed. Please review. https://codereview.chromium.org/2448133006/diff/1/third_party/protobuf/BUILD.gn File third_party/protobuf/BUILD.gn (right): https://codereview.chromium.org/2448133006/diff/1/third_party/protobuf/BUILD.gn#newcode184 third_party/protobuf/BUILD.gn:184: "//tools/traffic_annotation/*", On 2016/10/26 14:29:18, ...
4 years, 1 month ago (2016-10-27 09:40:11 UTC) #4
battre
Nice. A few more minor things. https://codereview.chromium.org/2448133006/diff/20001/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp File tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp (right): https://codereview.chromium.org/2448133006/diff/20001/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp#newcode4 tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp:4: Do you want ...
4 years, 1 month ago (2016-10-27 11:49:06 UTC) #5
Ramin Halavati
Comments addressed, please review. https://codereview.chromium.org/2448133006/diff/20001/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp File tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp (right): https://codereview.chromium.org/2448133006/diff/20001/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp#newcode4 tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp:4: On 2016/10/27 11:49:06, battre wrote: ...
4 years, 1 month ago (2016-10-27 12:51:20 UTC) #6
Ramin Halavati
Comments addressed, please review.
4 years, 1 month ago (2016-10-27 12:51:21 UTC) #7
Ramin Halavati
The code is now fully Windows compatible with just one required manual modification of the ...
4 years ago (2016-11-21 10:07:45 UTC) #8
Peter Kasting
LGTM for third_party/protobuf OWNERS. https://codereview.chromium.org/2448133006/diff/180001/third_party/protobuf/BUILD.gn File third_party/protobuf/BUILD.gn (right): https://codereview.chromium.org/2448133006/diff/180001/third_party/protobuf/BUILD.gn#newcode183 third_party/protobuf/BUILD.gn:183: # Traffic_annotation is not part ...
4 years ago (2016-11-23 17:01:52 UTC) #10
Ramin Halavati
Comments addressed. https://codereview.chromium.org/2448133006/diff/180001/third_party/protobuf/BUILD.gn File third_party/protobuf/BUILD.gn (right): https://codereview.chromium.org/2448133006/diff/180001/third_party/protobuf/BUILD.gn#newcode183 third_party/protobuf/BUILD.gn:183: # Traffic_annotation is not part of the ...
4 years ago (2016-11-24 06:34:53 UTC) #12
Peter Kasting
LGTM https://codereview.chromium.org/2448133006/diff/180001/tools/traffic_annotation/README.txt File tools/traffic_annotation/README.txt (right): https://codereview.chromium.org/2448133006/diff/180001/tools/traffic_annotation/README.txt#newcode38 tools/traffic_annotation/README.txt:38: clang with keeprsp switch as follows: On 2016/11/23 ...
4 years ago (2016-11-26 05:44:18 UTC) #14
Ramin Halavati
Dominic, Martin, The tool for extraction of network annotations and syntax checking is ready to ...
3 years, 10 months ago (2017-02-20 15:15:50 UTC) #16
msramek
Hi Ramin! Conceptually looks good, I mostly commented on style. https://codereview.chromium.org/2448133006/diff/390001/tools/traffic_annotation/auditor/README.txt File tools/traffic_annotation/auditor/README.txt (right): https://codereview.chromium.org/2448133006/diff/390001/tools/traffic_annotation/auditor/README.txt#newcode1 ...
3 years, 10 months ago (2017-02-21 15:57:55 UTC) #17
Ramin Halavati
msramek@: All comments addressed, please review. dcheng@: I've added a clang tool for extraction of ...
3 years, 9 months ago (2017-02-23 12:09:50 UTC) #19
msramek
Thanks for the improvements. tools/traffic_annotation/auditor/ LGTM % a few more comments. https://codereview.chromium.org/2448133006/diff/410001/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc File tools/traffic_annotation/auditor/traffic_annotation_auditor.cc (right): ...
3 years, 9 months ago (2017-02-27 20:00:52 UTC) #21
Ramin Halavati
msramek@: Comments addressed. dcheng@: Please review tools/clang/traffic_annotation_extractor/* asanka@, pastarmovj@: Please review DEPS as owners of ...
3 years, 9 months ago (2017-02-28 12:58:23 UTC) #23
pastarmovj
lgtm
3 years, 9 months ago (2017-02-28 13:04:08 UTC) #24
battre
I did not check until the very end... https://codereview.chromium.org/2448133006/diff/450001/tools/clang/traffic_annotation_extractor/README.md File tools/clang/traffic_annotation_extractor/README.md (right): https://codereview.chromium.org/2448133006/diff/450001/tools/clang/traffic_annotation_extractor/README.md#newcode3 tools/clang/traffic_annotation_extractor/README.md:3: tools/traffic_annotation/traffic_annotaion_auditor. ...
3 years, 9 months ago (2017-02-28 18:25:12 UTC) #25
dcheng
Sorry, I've been a bit swamped. Some high-level comments are: - add some tests (see ...
3 years, 9 months ago (2017-03-02 07:57:42 UTC) #26
Ramin Halavati
Daniel, Dominic, Clang tool code is refactored and simplified by removing an unnecessary feature. All ...
3 years, 8 months ago (2017-04-06 13:32:29 UTC) #27
battre
https://codereview.chromium.org/2448133006/diff/610001/third_party/protobuf/BUILD.gn File third_party/protobuf/BUILD.gn (right): https://codereview.chromium.org/2448133006/diff/610001/third_party/protobuf/BUILD.gn#newcode232 third_party/protobuf/BUILD.gn:232: "src/google/protobuf/any.cc", I think that this indentation with tabs is ...
3 years, 8 months ago (2017-04-07 08:42:28 UTC) #28
Ramin Halavati
All comments addressed, please review. https://codereview.chromium.org/2448133006/diff/610001/third_party/protobuf/BUILD.gn File third_party/protobuf/BUILD.gn (right): https://codereview.chromium.org/2448133006/diff/610001/third_party/protobuf/BUILD.gn#newcode232 third_party/protobuf/BUILD.gn:232: "src/google/protobuf/any.cc", On 2017/04/07 08:42:27, ...
3 years, 8 months ago (2017-04-07 11:33:33 UTC) #29
battre
https://codereview.chromium.org/2448133006/diff/610001/tools/clang/traffic_annotation_extractor/tests/inputs/autofill_download_manager.cc File tools/clang/traffic_annotation_extractor/tests/inputs/autofill_download_manager.cc (right): https://codereview.chromium.org/2448133006/diff/610001/tools/clang/traffic_annotation_extractor/tests/inputs/autofill_download_manager.cc#newcode1 tools/clang/traffic_annotation_extractor/tests/inputs/autofill_download_manager.cc:1: // Copyright 2014 The Chromium Authors. All rights reserved. ...
3 years, 8 months ago (2017-04-10 10:58:31 UTC) #30
Ramin Halavati
Comments addressed, please review. https://codereview.chromium.org/2448133006/diff/650001/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp File tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp (right): https://codereview.chromium.org/2448133006/diff/650001/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp#newcode139 tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp:139: // If DefineNetworkTrafficAnnotation is used ...
3 years, 8 months ago (2017-04-10 13:05:58 UTC) #31
dcheng
https://codereview.chromium.org/2448133006/diff/670001/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp File tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp (right): https://codereview.chromium.org/2448133006/diff/670001/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp#newcode60 tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp:60: llvm::StringRef GetDeclarationCoveringFunction(const clang::Decl* token, It's usually nicer to express ...
3 years, 8 months ago (2017-04-11 07:26:42 UTC) #32
Ramin Halavati
Thanks for the wonderful suggestions Daniel. I have updated the clang too, please review. I ...
3 years, 8 months ago (2017-04-11 09:15:39 UTC) #33
dcheng
https://codereview.chromium.org/2448133006/diff/690001/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp File tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp (right): https://codereview.chromium.org/2448133006/diff/690001/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp#newcode12 tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp:12: #include <fstream> Nit: I think this is unused now ...
3 years, 8 months ago (2017-04-12 07:07:05 UTC) #34
Ramin Halavati
Daniel, Thank you very much, Clang tool updated, please review. Do you have any suggestions ...
3 years, 8 months ago (2017-04-12 13:24:59 UTC) #35
dcheng
On 2017/04/12 13:24:59, Ramin Halavati wrote: > Daniel, > > Thank you very much, Clang ...
3 years, 8 months ago (2017-04-12 22:03:59 UTC) #36
Ramin Halavati
Martin, Dominic, I replaced traffic_annotation_auditor.cc with .py script. I also changed test set for clang ...
3 years, 8 months ago (2017-04-19 12:29:27 UTC) #37
battre
Getting there... I still don't like the testing. The rest are just nits. https://codereview.chromium.org/2448133006/diff/870001/tools/clang/traffic_annotation_extractor/tests/README.md File ...
3 years, 8 months ago (2017-04-20 13:41:04 UTC) #39
Ramin Halavati
Comments addressed, please review. https://codereview.chromium.org/2448133006/diff/870001/tools/clang/traffic_annotation_extractor/tests/README.md File tools/clang/traffic_annotation_extractor/tests/README.md (right): https://codereview.chromium.org/2448133006/diff/870001/tools/clang/traffic_annotation_extractor/tests/README.md#newcode1 tools/clang/traffic_annotation_extractor/tests/README.md:1: # Manual tests for Traffic ...
3 years, 8 months ago (2017-04-20 13:53:46 UTC) #40
battre
lgtm
3 years, 8 months ago (2017-04-20 14:01:41 UTC) #41
Ramin Halavati
ben@chromium.org: Please review changes in src/BUILD.gn We have added a tool to extract network traffic ...
3 years, 8 months ago (2017-04-20 14:06:10 UTC) #43
Ben Goodger (Google)
lgtm
3 years, 8 months ago (2017-04-21 05:25:01 UTC) #44
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2448133006/890001
3 years, 8 months ago (2017-04-21 05:26:10 UTC) #47
commit-bot: I haz the power
3 years, 8 months ago (2017-04-21 07:12:59 UTC) #50
Message was sent while issue was closed.
Committed patchset #45 (id:890001) as
https://chromium.googlesource.com/chromium/src/+/3051a40f7c5567d6bbee847227e7...

Powered by Google App Engine
This is Rietveld 408576698