Reporting: Wrap existing classes in context.

Reporting: Wrap existing classes in context.

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 takes the pile of small classes that I've written so far and
wraps them in a single "context" object.

It also changes the classes to accept a pointer to the context instead
of several pointers to individual dependencies.

It *also* creates a ReportingTestBase class that provides a properly
configured context for all of the existing unittests to use, and
switches the tests over.

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

[Julia Tuttle, 2017-03-15 13:58:45]

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

[commit-bot: I haz the power, 2017-03-15 13:58:59]

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

[Julia Tuttle, 2017-03-15 14:00:39]

Description was changed from

==========
Reporting: Create context to wrap existing classes.

Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/.

This CL takes the pile of small classes that I've written so far and
wraps them in a single "context" object.

It also changes the classes to accept a pointer to the context instead
of several pointers to individual dependencies.

It *also* creates a ReportingTestBase class that provides a properly
configured context for all of the existing unittests to use, and
switches the tests over.

BUG=
==========

to

==========
Reporting: Wrap existing classes in context.

Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/.

This CL takes the pile of small classes that I've written so far and
wraps them in a single "context" object.

It also changes the classes to accept a pointer to the context instead
of several pointers to individual dependencies.

It *also* creates a ReportingTestBase class that provides a properly
configured context for all of the existing unittests to use, and
switches the tests over.

BUG=
==========

[commit-bot: I haz the power, 2017-03-15 14:38:09]

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

[commit-bot: I haz the power, 2017-03-15 14:38:10]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Julia Tuttle, 2017-03-22 19:58:56]

Description was changed from

==========
Reporting: Wrap existing classes in context.

Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/.

This CL takes the pile of small classes that I've written so far and
wraps them in a single "context" object.

It also changes the classes to accept a pointer to the context instead
of several pointers to individual dependencies.

It *also* creates a ReportingTestBase class that provides a properly
configured context for all of the existing unittests to use, and
switches the tests over.

BUG=
==========

to

==========
Reporting: Wrap existing classes in context.

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 takes the pile of small classes that I've written so far and
wraps them in a single "context" object.

It also changes the classes to accept a pointer to the context instead
of several pointers to individual dependencies.

It *also* creates a ReportingTestBase class that provides a properly
configured context for all of the existing unittests to use, and
switches the tests over.

BUG=
==========

[Julia Tuttle, 2017-03-22 19:58:57]

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

[Julia Tuttle, 2017-03-22 20:01:41]

Description was changed from

==========
Reporting: Wrap existing classes in context.

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 takes the pile of small classes that I've written so far and
wraps them in a single "context" object.

It also changes the classes to accept a pointer to the context instead
of several pointers to individual dependencies.

It *also* creates a ReportingTestBase class that provides a properly
configured context for all of the existing unittests to use, and
switches the tests over.

BUG=
==========

to

==========
Reporting: Wrap existing classes in context.

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 takes the pile of small classes that I've written so far and
wraps them in a single "context" object.

It also changes the classes to accept a pointer to the context instead
of several pointers to individual dependencies.

It *also* creates a ReportingTestBase class that provides a properly
configured context for all of the existing unittests to use, and
switches the tests over.

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

[Julia Tuttle, 2017-03-22 20:07:06]

PTAL, shivanisha.

[Julia Tuttle, 2017-03-31 16:21:15]

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

[commit-bot: I haz the power, 2017-03-31 16:22:00]

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-03-31 16:27:20]

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

[commit-bot: I haz the power, 2017-03-31 16:27:20]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[shivanisha, 2017-04-03 15:39:05]

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
File net/reporting/reporting_context.h (right):

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_context.h:76: private:
nit: Can remove the extra private qualifier here.

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
File net/reporting/reporting_delegate.h (right):

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_delegate.h:20: // persisting data across embedder
restarts.
How will this class interact with https://codereview.chromium.org/2751883003/
ReportingSerializer?

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
File net/reporting/reporting_policy.h (right):

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_policy.h:22: // oldest-quieued.
nit: spelling of queued

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_policy.h:27: int max_endpoint_count;
Which classes implement the max_report_count , max_endpoint_count restrictions?

[Julia Tuttle, 2017-04-04 18:28:02]

PTAL, shivanisha.

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
File net/reporting/reporting_context.h (right):

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_context.h:76: private:
On 2017/04/03 15:39:05, shivanisha wrote:
> nit: Can remove the extra private qualifier here.

Oops!

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
File net/reporting/reporting_delegate.h (right):

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_delegate.h:20: // persisting data across embedder
restarts.
On 2017/04/03 15:39:05, shivanisha wrote:
> How will this class interact with https://codereview.chromium.org/2751883003/
> ReportingSerializer?

The Serializer (which will eventually be renamed the Persister, and will gain
the ability to schedule its own work) will use GetPersistedData() to load data
from a past session once at startup, and PersistData() to save data to be
reloaded by a future session periodically.

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
File net/reporting/reporting_policy.h (right):

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_policy.h:22: // oldest-quieued.
On 2017/04/03 15:39:05, shivanisha wrote:
> nit: spelling of queued

Obsolete.

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_policy.h:27: int max_endpoint_count;
On 2017/04/03 15:39:05, shivanisha wrote:
> Which classes implement the max_report_count , max_endpoint_count
restrictions?

They will be implemented in the cache, but I'm actually removing the
unimplemented policy knobs to avoid me or anyone else accidentally thinking
they're implemented.

[Julia Tuttle, 2017-04-04 18:28:04]

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

[commit-bot: I haz the power, 2017-04-04 18:28:39]

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-05 16:52:57]

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
File net/reporting/reporting_delegate.h (right):

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_delegate.h:20: // persisting data across embedder
restarts.
On 2017/04/04 at 18:28:02, Julia Tuttle wrote:
> On 2017/04/03 15:39:05, shivanisha wrote:
> > How will this class interact with
https://codereview.chromium.org/2751883003/
> > ReportingSerializer?
> 
> The Serializer (which will eventually be renamed the Persister, and will gain
the ability to schedule its own work) will use GetPersistedData() to load data
from a past session once at startup, and PersistData() to save data to be
reloaded by a future session periodically.

A design level question:
- Currently it looks like the ReportingDelegate class is only associated with
the serialization aspects of reporting. Can we not just use the
ReportingSerializer for the same or is the ReportingDelegate being created for
any future enhancements?

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
File net/reporting/reporting_policy.h (right):

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_policy.h:27: int max_endpoint_count;
On 2017/04/04 at 18:28:02, Julia Tuttle wrote:
> On 2017/04/03 15:39:05, shivanisha wrote:
> > Which classes implement the max_report_count , max_endpoint_count
restrictions?
> 
> They will be implemented in the cache, but I'm actually removing the
unimplemented policy knobs to avoid me or anyone else accidentally thinking
they're implemented.

Sounds good.

[Julia Tuttle, 2017-04-06 17:08:16]

PTAL, shivanisha.

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
File net/reporting/reporting_delegate.h (right):

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_delegate.h:20: // persisting data across embedder
restarts.
On 2017/04/05 16:52:57, shivanisha wrote:
> On 2017/04/04 at 18:28:02, Julia Tuttle wrote:
> > On 2017/04/03 15:39:05, shivanisha wrote:
> > > How will this class interact with
> https://codereview.chromium.org/2751883003/
> > > ReportingSerializer?
> > 
> > The Serializer (which will eventually be renamed the Persister, and will
gain
> the ability to schedule its own work) will use GetPersistedData() to load data
> from a past session once at startup, and PersistData() to save data to be
> reloaded by a future session periodically.
> 
> A design level question:
> - Currently it looks like the ReportingDelegate class is only associated with
> the serialization aspects of reporting. Can we not just use the
> ReportingSerializer for the same or is the ReportingDelegate being created for
> any future enhancements?

We can't use the ReportingSerializer for that; net can't directly modify prefs.
The ReportingDelegate is an interface that will be implemented in Chromium (or
other embedders) to allow reading/writing prefs.

[shivanisha, 2017-04-06 17:15:30]

lgtm

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
File net/reporting/reporting_delegate.h (right):

https://codereview.chromium.org/2751103002/diff/60001/net/reporting/reporting...
net/reporting/reporting_delegate.h:20: // persisting data across embedder
restarts.
On 2017/04/06 at 17:08:16, Julia Tuttle wrote:
> On 2017/04/05 16:52:57, shivanisha wrote:
> > On 2017/04/04 at 18:28:02, Julia Tuttle wrote:
> > > On 2017/04/03 15:39:05, shivanisha wrote:
> > > > How will this class interact with
> > https://codereview.chromium.org/2751883003/
> > > > ReportingSerializer?
> > > 
> > > The Serializer (which will eventually be renamed the Persister, and will
gain
> > the ability to schedule its own work) will use GetPersistedData() to load
data
> > from a past session once at startup, and PersistData() to save data to be
> > reloaded by a future session periodically.
> > 
> > A design level question:
> > - Currently it looks like the ReportingDelegate class is only associated
with
> > the serialization aspects of reporting. Can we not just use the
> > ReportingSerializer for the same or is the ReportingDelegate being created
for
> > any future enhancements?
> 
> We can't use the ReportingSerializer for that; net can't directly modify
prefs. The ReportingDelegate is an interface that will be implemented in
Chromium (or other embedders) to allow reading/writing prefs.

Thanks.

[Julia Tuttle, 2017-04-06 18:20:16]

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

[Julia Tuttle, 2017-04-06 18:20:17]

PTAL for committer signoff, csharrison.

[Charlie Harrison, 2017-04-06 18:21:41]

RS LGTM

[Julia Tuttle, 2017-04-06 20:21:23]

The CQ bit was checked by juliatuttle@chromium.org

[commit-bot: I haz the power, 2017-04-06 20:22:26]

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-06 20:27:31]

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

[commit-bot: I haz the power, 2017-04-06 20:27:31]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Julia Tuttle, 2017-04-07 16:04:48]

The CQ bit was checked by juliatuttle@chromium.org

[Julia Tuttle, 2017-04-07 16:04:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
shivanisha@chromium.org, csharrison@chromium.org
Link to the patchset: https://codereview.chromium.org/2751103002/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-07 16:05:13]

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-07 17:09:49]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1491581088751300,
"parent_rev": "1dbe1136b6651ba54f52c2569ef95a62d31aac8f", "commit_rev":
"ee4b55e3526065ac1d0f09a80372e17f26892c8e"}

[commit-bot: I haz the power, 2017-04-07 17:10:33]

Description was changed from

==========
Reporting: Wrap existing classes in context.

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 takes the pile of small classes that I've written so far and
wraps them in a single "context" object.

It also changes the classes to accept a pointer to the context instead
of several pointers to individual dependencies.

It *also* creates a ReportingTestBase class that provides a properly
configured context for all of the existing unittests to use, and
switches the tests over.

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

to

==========
Reporting: Wrap existing classes in context.

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 takes the pile of small classes that I've written so far and
wraps them in a single "context" object.

It also changes the classes to accept a pointer to the context instead
of several pointers to individual dependencies.

It *also* creates a ReportingTestBase class that provides a properly
configured context for all of the existing unittests to use, and
switches the tests over.

BUG=704259

Review-Url: https://codereview.chromium.org/2751103002
Cr-Commit-Position: refs/heads/master@{#462901}
Committed:
https://chromium.googlesource.com/chromium/src/+/ee4b55e3526065ac1d0f09a80372...
==========

[commit-bot: I haz the power, 2017-04-07 17:10:34]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/ee4b55e3526065ac1d0f09a80372...