Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(3930)

Issue 2800653002: Network traffic annotation added to common/cloud. (Closed)

Created:
2 years, 5 months ago by Ramin Halavati
Modified:
2 years, 3 months ago
CC:
chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Network traffic annotation added to policy/core/common/cloud. Network traffic annotation is added to network request of components/policy/core/common/cloud/device_management_service.cc components/policy/core/common/cloud/external_policy_data_fetcher.cc components/policy/core/common/cloud/user_info_fetcher.cc BUG=656607 Review-Url: https://codereview.chromium.org/2800653002 Cr-Commit-Position: refs/heads/master@{#468999} Committed: https://chromium.googlesource.com/chromium/src/+/9cde810d897d01c57b889a39977b4f39b68cdfb1

Patch Set 1 #

Total comments: 91

Patch Set 2 : Annotations Updated. #

Patch Set 3 : Annotation updated. #

Patch Set 4 : Annotations updated. #

Total comments: 2

Patch Set 5 : Annotation updated. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+86 lines, -4 lines) Patch
M components/policy/core/common/cloud/device_management_service.cc View 1 2 3 4 2 chunks +34 lines, -1 line 0 comments Download
M components/policy/core/common/cloud/external_policy_data_fetcher.cc View 1 2 3 2 chunks +27 lines, -2 lines 0 comments Download
M components/policy/core/common/cloud/user_info_fetcher.cc View 1 2 3 4 2 chunks +25 lines, -1 line 0 comments Download

Messages

Total messages: 25 (7 generated)
Ramin Halavati
Julian, I've added annotation templates to 3 files in policy folder, please suggest values for ...
2 years, 5 months ago (2017-04-05 09:48:51 UTC) #3
pastarmovj
I put some of the values I am fairly sure as comments. Where I was ...
2 years, 5 months ago (2017-04-07 09:58:46 UTC) #5
Ramin Halavati
On 2017/04/07 09:58:46, pastarmovj wrote: > I put some of the values I am fairly ...
2 years, 5 months ago (2017-04-18 11:31:27 UTC) #6
Andrew T Wilson (Slow)
https://codereview.chromium.org/2800653002/diff/1/components/policy/core/common/cloud/device_management_service.cc File components/policy/core/common/cloud/device_management_service.cc (right): https://codereview.chromium.org/2800653002/diff/1/components/policy/core/common/cloud/device_management_service.cc#newcode597 components/policy/core/common/cloud/device_management_service.cc:597: description: "..." Communication with the Cloud Policy backend - ...
2 years, 5 months ago (2017-04-18 12:01:32 UTC) #7
Ramin Halavati
Thank you very much, annotations updated, please review. I've also added some inline questions. https://codereview.chromium.org/2800653002/diff/1/components/policy/core/common/cloud/device_management_service.cc ...
2 years, 5 months ago (2017-04-19 05:21:58 UTC) #8
Andrew T Wilson (Slow)
lgtm if you're comfortable with the data provided. https://codereview.chromium.org/2800653002/diff/1/components/policy/core/common/cloud/device_management_service.cc File components/policy/core/common/cloud/device_management_service.cc (right): https://codereview.chromium.org/2800653002/diff/1/components/policy/core/common/cloud/device_management_service.cc#newcode599 components/policy/core/common/cloud/device_management_service.cc:599: data: ...
2 years, 4 months ago (2017-04-25 13:48:01 UTC) #9
Ramin Halavati
Thank you Andrew. I disabled the cookies in another CL, made some small changes, and ...
2 years, 4 months ago (2017-04-26 05:28:06 UTC) #10
Andrew T Wilson (Slow)
Thanks again for providing all this information on our behalf. https://codereview.chromium.org/2800653002/diff/1/components/policy/core/common/cloud/device_management_service.cc File components/policy/core/common/cloud/device_management_service.cc (right): https://codereview.chromium.org/2800653002/diff/1/components/policy/core/common/cloud/device_management_service.cc#newcode599 ...
2 years, 4 months ago (2017-04-26 09:34:11 UTC) #11
Ramin Halavati
Thank you Andrew. Annotation updated, please review. Also that would be awesome if you rephrase ...
2 years, 4 months ago (2017-04-26 09:44:41 UTC) #12
Andrew T Wilson (Slow)
https://codereview.chromium.org/2800653002/diff/60001/components/policy/core/common/cloud/user_info_fetcher.cc File components/policy/core/common/cloud/user_info_fetcher.cc (right): https://codereview.chromium.org/2800653002/diff/60001/components/policy/core/common/cloud/user_info_fetcher.cc#newcode55 components/policy/core/common/cloud/user_info_fetcher.cc:55: "managed, along with basic profile information from their Google ...
2 years, 4 months ago (2017-04-27 12:15:31 UTC) #13
Ramin Halavati
Thank you very much, comment addressed, please review. https://codereview.chromium.org/2800653002/diff/60001/components/policy/core/common/cloud/user_info_fetcher.cc File components/policy/core/common/cloud/user_info_fetcher.cc (right): https://codereview.chromium.org/2800653002/diff/60001/components/policy/core/common/cloud/user_info_fetcher.cc#newcode55 components/policy/core/common/cloud/user_info_fetcher.cc:55: "managed, ...
2 years, 4 months ago (2017-04-27 12:25:36 UTC) #14
Andrew T Wilson (Slow)
still lgtm
2 years, 4 months ago (2017-04-27 12:47:10 UTC) #15
Ramin Halavati
Thank you very much. Dominic, Martin, Any comments?
2 years, 4 months ago (2017-04-27 12:48:28 UTC) #17
msramek
LGTM
2 years, 4 months ago (2017-05-03 15:05:15 UTC) #18
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/2800653002/80001
2 years, 4 months ago (2017-05-03 15:21:32 UTC) #20
commit-bot: I haz the power
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/9cde810d897d01c57b889a39977b4f39b68cdfb1
2 years, 4 months ago (2017-05-03 16:20:08 UTC) #23
Thiemo Nagel
From what I can see, the annotation is missing details about the forced re-enrollment protocol ...
2 years, 3 months ago (2017-06-07 16:47:40 UTC) #24
Ramin Halavati
2 years, 3 months ago (2017-06-08 04:54:07 UTC) #25
Message was sent while issue was closed.
On 2017/06/07 16:47:40, Thiemo Nagel wrote:
> From what I can see, the annotation is missing details about the forced
> re-enrollment protocol (FRE).  This happens during OOBE and it can't be turned
> off by "signing out of Chrome".

Thank you Thiemo.
Could you please be more specific about the required changes?

Powered by Google App Engine
This is Rietveld 408576698