Reporting: Observe network changes and clear reports/clients

Reporting: Observe network changes and clear reports/clients

Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/ for
the spec, or https://goo.gl/pygX5I for details of the planned
implementation in Chromium.

This CL adds a ReportingNetworkChangeObserver that can clear reports
and/or clients (as configured in ReportingPolicy) on network changes.

BUG=704259
Review-Url: https://codereview.chromium.org/2813103005
Cr-Commit-Position: refs/heads/master@{#466113}
Committed: https://chromium.googlesource.com/chromium/src/+/164e4cdfd53297fe3801ae893bbaa48651e1b339

[Julia Tuttle, 2017-04-13 14:51:40]

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

[commit-bot: I haz the power, 2017-04-13 14:51:55]

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-04-13 16:09:46]

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

[commit-bot: I haz the power, 2017-04-13 16:09:47]

Dry run: This issue passed the CQ dry run.

[Julia Tuttle, 2017-04-13 19:27:12]

juliatuttle@chromium.org changed reviewers:
+ shivanisha@chromium.org

[Julia Tuttle, 2017-04-13 19:27:12]

PTAL, shivanisha.

[Julia Tuttle, 2017-04-18 18:49:00]

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

[commit-bot: I haz the power, 2017-04-18 18:49:47]

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-04-18 20:24:54]

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

[commit-bot: I haz the power, 2017-04-18 20:24:55]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[shivanisha, 2017-04-20 17:39:37]

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
File net/reporting/reporting_network_change_observer.cc (right):

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
net/reporting/reporting_network_change_observer.cc:34: return;
May be a comment here before the if statement saying that every network
connection change will be preceeded by this function call with
ConnectionType::CONNECTION_NONE. Perform the cleanup at that time.

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
File net/reporting/reporting_network_change_observer.h (right):

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
net/reporting/reporting_network_change_observer.h:17: // ReportingPolicy says
to.
nit: if enabled in ReportingPolicy.

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
File net/reporting/reporting_network_change_observer_unittest.cc (right):

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
net/reporting/reporting_network_change_observer_unittest.cc:88: 
ASSERT_EQ(1u, report_count());
ASSERT_EQ(1u, client_count());

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
net/reporting/reporting_network_change_observer_unittest.cc:107: 
ASSERT_EQ(1u, report_count());
ASSERT_EQ(1u, client_count());

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
net/reporting/reporting_network_change_observer_unittest.cc:126: 
ASSERT_EQ(1u, report_count());
ASSERT_EQ(1u, client_count());

[Julia Tuttle, 2017-04-20 17:52:46]

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

[Julia Tuttle, 2017-04-20 17:53:05]

PTAL, shivanisha.

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
File net/reporting/reporting_network_change_observer.cc (right):

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
net/reporting/reporting_network_change_observer.cc:34: return;
On 2017/04/20 17:39:37, shivanisha wrote:
> May be a comment here before the if statement saying that every network
> connection change will be preceeded by this function call with
> ConnectionType::CONNECTION_NONE. Perform the cleanup at that time.

Done.

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
File net/reporting/reporting_network_change_observer.h (right):

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
net/reporting/reporting_network_change_observer.h:17: // ReportingPolicy says
to.
On 2017/04/20 17:39:37, shivanisha wrote:
> nit: if enabled in ReportingPolicy.

Done.

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
File net/reporting/reporting_network_change_observer_unittest.cc (right):

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
net/reporting/reporting_network_change_observer_unittest.cc:88: 
On 2017/04/20 17:39:37, shivanisha wrote:
> ASSERT_EQ(1u, report_count());
> ASSERT_EQ(1u, client_count());

Done.

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
net/reporting/reporting_network_change_observer_unittest.cc:107: 
On 2017/04/20 17:39:37, shivanisha wrote:
> ASSERT_EQ(1u, report_count());
> ASSERT_EQ(1u, client_count());

Done.

https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
net/reporting/reporting_network_change_observer_unittest.cc:126: 
On 2017/04/20 17:39:37, shivanisha wrote:
> ASSERT_EQ(1u, report_count());
> ASSERT_EQ(1u, client_count());

Done.

[commit-bot: I haz the power, 2017-04-20 17:53:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[shivanisha, 2017-04-20 18:02:01]

On 2017/04/20 at 17:53:05, juliatuttle wrote:
> PTAL, shivanisha.
> 
>
https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
> File net/reporting/reporting_network_change_observer.cc (right):
> 
>
https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
> net/reporting/reporting_network_change_observer.cc:34: return;
> On 2017/04/20 17:39:37, shivanisha wrote:
> > May be a comment here before the if statement saying that every network
> > connection change will be preceeded by this function call with
> > ConnectionType::CONNECTION_NONE. Perform the cleanup at that time.
> 
> Done.
> 
>
https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
> File net/reporting/reporting_network_change_observer.h (right):
> 
>
https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
> net/reporting/reporting_network_change_observer.h:17: // ReportingPolicy says
to.
> On 2017/04/20 17:39:37, shivanisha wrote:
> > nit: if enabled in ReportingPolicy.
> 
> Done.
> 
>
https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
> File net/reporting/reporting_network_change_observer_unittest.cc (right):
> 
>
https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
> net/reporting/reporting_network_change_observer_unittest.cc:88: 
> On 2017/04/20 17:39:37, shivanisha wrote:
> > ASSERT_EQ(1u, report_count());
> > ASSERT_EQ(1u, client_count());
> 
> Done.
> 
>
https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
> net/reporting/reporting_network_change_observer_unittest.cc:107: 
> On 2017/04/20 17:39:37, shivanisha wrote:
> > ASSERT_EQ(1u, report_count());
> > ASSERT_EQ(1u, client_count());
> 
> Done.
> 
>
https://codereview.chromium.org/2813103005/diff/20001/net/reporting/reporting...
> net/reporting/reporting_network_change_observer_unittest.cc:126: 
> On 2017/04/20 17:39:37, shivanisha wrote:
> > ASSERT_EQ(1u, report_count());
> > ASSERT_EQ(1u, client_count());
> 
> Done.

lgtm.

[Julia Tuttle, 2017-04-20 18:04:22]

juliatuttle@chromium.org changed reviewers:
+ csharrison@chromium.org

[Julia Tuttle, 2017-04-20 18:04:23]

Thanks, shivanisha! PTAL csharrison for committer signoff.

[Charlie Harrison, 2017-04-20 18:06:30]

On 2017/04/20 18:04:23, Julia Tuttle wrote:
> Thanks, shivanisha! PTAL csharrison for committer signoff.

RS LGTM

[Julia Tuttle, 2017-04-20 18:07:40]

Thanks, csharrison!

[Julia Tuttle, 2017-04-20 20:04:58]

The CQ bit was checked by juliatuttle@chromium.org

[Julia Tuttle, 2017-04-20 20:04:59]

The CQ bit was unchecked by juliatuttle@chromium.org

[Julia Tuttle, 2017-04-20 20:05:03]

The CQ bit was checked by juliatuttle@chromium.org

[commit-bot: I haz the power, 2017-04-20 20:05:42]

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-04-20 20:30:32]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492718703842430,
"parent_rev": "ded9a228e452e84df5aa818e0c8f67c72ff971dc", "commit_rev":
"164e4cdfd53297fe3801ae893bbaa48651e1b339"}

[commit-bot: I haz the power, 2017-04-20 20:30:41]

Description was changed from

==========
Reporting: Observe network changes and clear reports/clients

Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/ for
the spec, or https://goo.gl/pygX5I for details of the planned
implementation in Chromium.

This CL adds a ReportingNetworkChangeObserver that can clear reports
and/or clients (as configured in ReportingPolicy) on network changes.

BUG=704259
==========

to

==========
Reporting: Observe network changes and clear reports/clients

Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/ for
the spec, or https://goo.gl/pygX5I for details of the planned
implementation in Chromium.

This CL adds a ReportingNetworkChangeObserver that can clear reports
and/or clients (as configured in ReportingPolicy) on network changes.

BUG=704259

Review-Url: https://codereview.chromium.org/2813103005
Cr-Commit-Position: refs/heads/master@{#466113}
Committed:
https://chromium.googlesource.com/chromium/src/+/164e4cdfd53297fe3801ae893bba...
==========

[commit-bot: I haz the power, 2017-04-20 20:30:42]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/164e4cdfd53297fe3801ae893bba...