Network traffic annotation added to CloudPrintUrlFetcher.

Network traffic annotation added to CloudPrintUrlFetcher.

Network traffic annotation is added to network requests of:
chrome/service/cloud_print/cloud_print_auth.h
chrome/service/cloud_print/cloud_print_auth.cc
chrome/service/cloud_print/cloud_print_connector.h
chrome/service/cloud_print/cloud_print_connector.cc
chrome/service/cloud_print/cloud_print_proxy.cc
chrome/service/cloud_print/cloud_print_proxy_backend.cc
chrome/service/cloud_print/cloud_print_url_fetcher.h
chrome/service/cloud_print/cloud_print_url_fetcher.cc
chrome/service/cloud_print/cloud_print_url_fetcher_unittest.cc
chrome/service/cloud_print/cloud_print_wipeout.h
chrome/service/cloud_print/cloud_print_wipeout.cc
chrome/service/cloud_print/job_status_updater.h
chrome/service/cloud_print/job_status_updater.cc
chrome/service/cloud_print/printer_job_handler.cc
chrome/service/cloud_print/printer_job_handler_unittest.ccc

BUG=656607
Review-Url: https://codereview.chromium.org/2888763004
Cr-Commit-Position: refs/heads/master@{#480094}
Committed: https://chromium.googlesource.com/chromium/src/+/9aa67aaac041b7ed58f2db22ae81ab9872bd3e07

[Ramin Halavati, 2017-05-17 11:42:25]

Patchset #1 (id:1) has been deleted

[Ramin Halavati, 2017-05-30 06:49:56]

The CQ bit was checked by rhalavati@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-30 06:50:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-30 06:58:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-30 06:58:51]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ramin Halavati, 2017-05-30 07:04:27]

Patchset #1 (id:20001) has been deleted

[Ramin Halavati, 2017-05-30 07:04:38]

The CQ bit was checked by rhalavati@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-30 07:04:54]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-30 08:03:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-30 08:03:28]

Dry run: This issue passed the CQ dry run.

[Ramin Halavati, 2017-05-30 08:13:29]

rhalavati@chromium.org changed reviewers:
+ gene@chromium.org

[Ramin Halavati, 2017-05-30 08:13:30]

Hi,

We formerly tried to annotate network requests of CloudPrintURLFetcher in this
CL (https://codereview.chromium.org/2804613002) but it was blocked do to very
different requests that were handled through it.
I have put the templates for the newer version of annotations, which allows
parts of annotation to be in one function and the reset of it in another one.
Therefore you can collect all common parts of annotation of different calls in
the final code location which creates the request, and put different
descriptions for individual triggers.
Please suggest values for all missing parts, or tell me if should move anything
around.

I repeat the intro here:

[Ramin Halavati, 2017-06-07 08:17:05]

The CQ bit was checked by rhalavati@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-07 08:17:16]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ramin Halavati, 2017-06-07 08:56:18]

Description was changed from

==========
Network traffic annotation added to CloudPrintUrlFetcher.

Network traffic annotation is added to network requests of:
chrome/service/cloud_print/cloud_print_auth.cc
chrome/service/cloud_print/cloud_print_connector.cc
chrome/service/cloud_print/cloud_print_url_fetcher.cc
chrome/service/cloud_print/cloud_print_wipeout.cc
chrome/service/cloud_print/job_status_updater.cc
chrome/service/cloud_print/printer_job_handler.cc

BUG=656607
==========

to

==========
Network traffic annotation added to CloudPrintUrlFetcher.

Network traffic annotation is added to network requests of:
chrome/service/cloud_print/cloud_print_auth.h
chrome/service/cloud_print/cloud_print_auth.cc
chrome/service/cloud_print/cloud_print_connector.h
chrome/service/cloud_print/cloud_print_connector.cc
chrome/service/cloud_print/cloud_print_proxy.cc
chrome/service/cloud_print/cloud_print_proxy_backend.cc
chrome/service/cloud_print/cloud_print_url_fetcher.h
chrome/service/cloud_print/cloud_print_url_fetcher.cc
chrome/service/cloud_print/cloud_print_url_fetcher_unittest.cc
chrome/service/cloud_print/cloud_print_wipeout.h
chrome/service/cloud_print/cloud_print_wipeout.cc
chrome/service/cloud_print/job_status_updater.h
chrome/service/cloud_print/job_status_updater.cc
chrome/service/cloud_print/printer_job_handler.cc
chrome/service/cloud_print/printer_job_handler_unittest.ccc

BUG=656607
==========

[Ramin Halavati, 2017-06-07 08:58:08]

Gene,

Following our talk I moved and drafted the annotations based on what I
understood from the code and supplied documents. Annotations are now in the
following four files and the rest is just plumbing. Please review.

chrome/service/cloud_print/cloud_print_url_fetcher.cc
chrome/service/cloud_print/cloud_print_proxy_backend.cc
chrome/service/cloud_print/cloud_print_proxy.cc
chrome/service/cloud_print/printer_job_handler.cc

[gene1, 2017-06-07 17:50:05]

lgtm!

[Ramin Halavati, 2017-06-08 04:58:16]

rhalavati@chromium.org changed reviewers:
+ msramek@chromium.org

[Ramin Halavati, 2017-06-08 04:58:17]

Thank you Gene.

Martin,
Do you have any comments?

[Ramin Halavati, 2017-06-08 04:59:14]

On 2017/06/08 04:58:17, Ramin Halavati wrote:
> Thank you Gene.
> 
> Martin,
> Do you have any comments?

Only these files include annotation and the rest is plumbing:

chrome/service/cloud_print/cloud_print_url_fetcher.cc
chrome/service/cloud_print/cloud_print_proxy_backend.cc
chrome/service/cloud_print/cloud_print_proxy.cc
chrome/service/cloud_print/printer_job_handler.cc

[msramek, 2017-06-08 21:23:32]

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
File chrome/service/cloud_print/cloud_print_proxy.cc (right):

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
chrome/service/cloud_print/cloud_print_proxy.cc:239: "User request of
unregistering printers or policy change."
nit: Just to be a bit more explicit, can we say which policy? It's defined in
the other half of the annotation, so it's not obvious here.

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
File chrome/service/cloud_print/cloud_print_proxy_backend.cc (right):

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
chrome/service/cloud_print/cloud_print_proxy_backend.cc:274: "Cloud Print
service intialization."
nit: Fits on one line.

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
File chrome/service/cloud_print/printer_job_handler.cc (right):

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
chrome/service/cloud_print/printer_job_handler.cc:67: "connecting to printer,
sending jobs, updating jobs, and gettings "
typo: getting

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
chrome/service/cloud_print/printer_job_handler.cc:70: "Automatic checking if
printer is available, registering parinter, "
typo: printer

[Ramin Halavati, 2017-06-09 04:55:46]

Thanks Martin, comments addressed, please review.

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
File chrome/service/cloud_print/cloud_print_proxy.cc (right):

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
chrome/service/cloud_print/cloud_print_proxy.cc:239: "User request of
unregistering printers or policy change."
On 2017/06/08 21:23:32, msramek (slow) wrote:
> nit: Just to be a bit more explicit, can we say which policy? It's defined in
> the other half of the annotation, so it's not obvious here.

Done, but note that the report that is generated using partial annotations will
have full annotation for each call (two parts will be merged). Therefore what a
reader sees is the complete annotation including policy name, and I just pointed
towards that to avoid duplication and also need to update both parts if policy
changes.

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
File chrome/service/cloud_print/cloud_print_proxy_backend.cc (right):

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
chrome/service/cloud_print/cloud_print_proxy_backend.cc:274: "Cloud Print
service intialization."
On 2017/06/08 21:23:32, msramek (slow) wrote:
> nit: Fits on one line.

Done.

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
File chrome/service/cloud_print/printer_job_handler.cc (right):

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
chrome/service/cloud_print/printer_job_handler.cc:67: "connecting to printer,
sending jobs, updating jobs, and gettings "
On 2017/06/08 21:23:32, msramek (slow) wrote:
> typo: getting

Done.

https://codereview.chromium.org/2888763004/diff/80001/chrome/service/cloud_pr...
chrome/service/cloud_print/printer_job_handler.cc:70: "Automatic checking if
printer is available, registering parinter, "
On 2017/06/08 21:23:32, msramek (slow) wrote:
> typo: printer

Done.

[Ramin Halavati, 2017-06-09 04:55:53]

The CQ bit was checked by rhalavati@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-09 04:56:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-09 06:39:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-09 06:39:01]

Dry run: This issue passed the CQ dry run.

[msramek, 2017-06-13 13:52:20]

LGTM

[Ramin Halavati, 2017-06-13 13:54:13]

The CQ bit was checked by rhalavati@chromium.org

[Ramin Halavati, 2017-06-13 13:54:13]

The patchset sent to the CQ was uploaded after l-g-t-m from gene@google.com
Link to the patchset: https://codereview.chromium.org/2888763004/#ps100001
(title: "Comments addressed.")

[commit-bot: I haz the power, 2017-06-13 13:54:23]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-13 14:03:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-13 14:03:28]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ramin Halavati, 2017-06-14 04:39:26]

rhalavati@chromium.org changed reviewers:
+ scottbyer@chromium.org

[Ramin Halavati, 2017-06-14 04:39:27]

Scott,

Gene has approved this CL, but does not have access to his Chromium account to
LGTM it using that, could you please take a look.

[Scott Byer, 2017-06-16 16:09:20]

scottbyer@chromium.org changed reviewers:
+ gene@google.com

[Scott Byer, 2017-06-16 16:09:21]

lgtm

https://codereview.chromium.org/2888763004/diff/100001/chrome/service/cloud_p...
File chrome/service/cloud_print/cloud_print_proxy.cc (right):

https://codereview.chromium.org/2888763004/diff/100001/chrome/service/cloud_p...
chrome/service/cloud_print/cloud_print_proxy.cc:241: data: "OAuth2 token and
list of parinter ids to unregister."
nit: spelling

[Ramin Halavati, 2017-06-16 16:32:06]

Thank you Scott, comment addressed, landing.

https://codereview.chromium.org/2888763004/diff/100001/chrome/service/cloud_p...
File chrome/service/cloud_print/cloud_print_proxy.cc (right):

https://codereview.chromium.org/2888763004/diff/100001/chrome/service/cloud_p...
chrome/service/cloud_print/cloud_print_proxy.cc:241: data: "OAuth2 token and
list of parinter ids to unregister."
On 2017/06/16 16:09:21, Scott Byer wrote:
> nit: spelling

Done.

[Ramin Halavati, 2017-06-16 16:32:15]

The CQ bit was checked by rhalavati@chromium.org

[Ramin Halavati, 2017-06-16 16:32:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
msramek@chromium.org, gene@google.com, scottbyer@chromium.org
Link to the patchset: https://codereview.chromium.org/2888763004/#ps120001
(title: "nits")

[commit-bot: I haz the power, 2017-06-16 16:32:45]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-16 17:43:45]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1497630735616730,
"parent_rev": "0c6f374f5aa238d7562e9730cbf187e93850ead0", "commit_rev":
"9aa67aaac041b7ed58f2db22ae81ab9872bd3e07"}

[commit-bot: I haz the power, 2017-06-16 17:43:57]

Description was changed from

==========
Network traffic annotation added to CloudPrintUrlFetcher.

Network traffic annotation is added to network requests of:
chrome/service/cloud_print/cloud_print_auth.h
chrome/service/cloud_print/cloud_print_auth.cc
chrome/service/cloud_print/cloud_print_connector.h
chrome/service/cloud_print/cloud_print_connector.cc
chrome/service/cloud_print/cloud_print_proxy.cc
chrome/service/cloud_print/cloud_print_proxy_backend.cc
chrome/service/cloud_print/cloud_print_url_fetcher.h
chrome/service/cloud_print/cloud_print_url_fetcher.cc
chrome/service/cloud_print/cloud_print_url_fetcher_unittest.cc
chrome/service/cloud_print/cloud_print_wipeout.h
chrome/service/cloud_print/cloud_print_wipeout.cc
chrome/service/cloud_print/job_status_updater.h
chrome/service/cloud_print/job_status_updater.cc
chrome/service/cloud_print/printer_job_handler.cc
chrome/service/cloud_print/printer_job_handler_unittest.ccc

BUG=656607
==========

to

==========
Network traffic annotation added to CloudPrintUrlFetcher.

Network traffic annotation is added to network requests of:
chrome/service/cloud_print/cloud_print_auth.h
chrome/service/cloud_print/cloud_print_auth.cc
chrome/service/cloud_print/cloud_print_connector.h
chrome/service/cloud_print/cloud_print_connector.cc
chrome/service/cloud_print/cloud_print_proxy.cc
chrome/service/cloud_print/cloud_print_proxy_backend.cc
chrome/service/cloud_print/cloud_print_url_fetcher.h
chrome/service/cloud_print/cloud_print_url_fetcher.cc
chrome/service/cloud_print/cloud_print_url_fetcher_unittest.cc
chrome/service/cloud_print/cloud_print_wipeout.h
chrome/service/cloud_print/cloud_print_wipeout.cc
chrome/service/cloud_print/job_status_updater.h
chrome/service/cloud_print/job_status_updater.cc
chrome/service/cloud_print/printer_job_handler.cc
chrome/service/cloud_print/printer_job_handler_unittest.ccc

BUG=656607

Review-Url: https://codereview.chromium.org/2888763004
Cr-Commit-Position: refs/heads/master@{#480094}
Committed:
https://chromium.googlesource.com/chromium/src/+/9aa67aaac041b7ed58f2db22ae81...
==========

[commit-bot: I haz the power, 2017-06-16 17:43:59]

Committed patchset #5 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/9aa67aaac041b7ed58f2db22ae81...