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/builds/172935)
3 years, 9 months ago
(2017-03-15 14:20:39 UTC)
#4
3 years, 9 months ago
(2017-03-22 19:19:24 UTC)
#6
PTAL, shivanisha.
Julia Tuttle
Description was changed from ========== Reporting: Create serializer. Reporting is a spec for delivering out-of-band ...
3 years, 9 months ago
(2017-03-22 19:58:01 UTC)
#7
Description was changed from
==========
Reporting: Create serializer.
Reporting is a spec for delivering out-of-band reports from various
other parts of the browser. See http://wicg.github.io/reporting/.
The serializer can serialize and deserialize the data in the cache (and
potentially other places eventually, like the endpoint manager's state).
It will be used to persist selected state of the Reporting system
across browser/embedder restarts.
==========
to
==========
Reporting: Implement serializer.
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.
The serializer can serialize and deserialize the data in the cache (and
potentially other places eventually, like the endpoint manager's state).
It will be used to persist selected state of the Reporting system
across browser/embedder restarts.
==========
Julia Tuttle
Description was changed from ========== Reporting: Implement serializer. Reporting is a spec for delivering out-of-band ...
3 years, 9 months ago
(2017-03-22 20:00:40 UTC)
#8
Description was changed from
==========
Reporting: Implement serializer.
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.
The serializer can serialize and deserialize the data in the cache (and
potentially other places eventually, like the endpoint manager's state).
It will be used to persist selected state of the Reporting system
across browser/embedder restarts.
==========
to
==========
Reporting: Implement serializer.
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.
The serializer can serialize and deserialize the data in the cache (and
potentially other places eventually, like the endpoint manager's state).
It will be used to persist selected state of the Reporting system
across browser/embedder restarts.
BUG=704259
==========
Julia Tuttle
The CQ bit was checked by juliatuttle@chromium.org to run a CQ dry run
3 years, 8 months ago
(2017-03-31 16:09:29 UTC)
#9
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/419505)
3 years, 8 months ago
(2017-03-31 17:03:14 UTC)
#12
3 years, 8 months ago
(2017-03-31 18:25:56 UTC)
#16
Dry run: This issue passed the CQ dry run.
shivanisha
https://codereview.chromium.org/2751883003/diff/60001/net/reporting/reporting_serializer.cc File net/reporting/reporting_serializer.cc (right): https://codereview.chromium.org/2751883003/diff/60001/net/reporting/reporting_serializer.cc#newcode70 net/reporting/reporting_serializer.cc:70: return false; dcheck group is not empty. Also type ...
3 years, 8 months ago
(2017-04-05 18:47:31 UTC)
#17
On 2017/04/06 at 20:20:55, juliatuttle wrote: > PTAL, shivanisha. > > https://codereview.chromium.org/2751883003/diff/60001/net/reporting/reporting_serializer.cc > File net/reporting/reporting_serializer.cc ...
3 years, 8 months ago
(2017-04-07 14:30:47 UTC)
#19
On 2017/04/06 at 20:20:55, juliatuttle wrote:
> PTAL, shivanisha.
>
>
https://codereview.chromium.org/2751883003/diff/60001/net/reporting/reporting...
> File net/reporting/reporting_serializer.cc (right):
>
>
https://codereview.chromium.org/2751883003/diff/60001/net/reporting/reporting...
> net/reporting/reporting_serializer.cc:70: return false;
> On 2017/04/05 18:47:31, shivanisha wrote:
> > dcheck group is not empty. Also type and body below.
>
> There's nothing actually disallowing those members from being empty in the
spec.
>
>
https://codereview.chromium.org/2751883003/diff/60001/net/reporting/reporting...
> net/reporting/reporting_serializer.cc:94: attempts);
> On 2017/04/05 18:47:31, shivanisha wrote:
> > I believe the invocation of this call will only be once at startup for every
> > report. Since here report is getting added to the cache unconditionally, can
we
> > DCHECK before this is called for the first report that cache does not
contain
> > any report. My concern here is 2 same reports getting added to the cache.
>
> Yeah, I'll check in DeserializeFromValue that there are no reports or clients
in the cache yet.
>
> This may change if we move to loading data asynchronously, but I can change
the check if so.
>
>
https://codereview.chromium.org/2751883003/diff/60001/net/reporting/reporting...
> net/reporting/reporting_serializer.cc:242:
serialized->SetInteger("reporting_serialized_cache_version", 1);
> On 2017/04/05 18:47:31, shivanisha wrote:
> > May be have a static const as the version instead of hardcoding 1 , here and
in
> > DeserializeFromValue?
>
> Done.
>
>
https://codereview.chromium.org/2751883003/diff/60001/net/reporting/reporting...
> net/reporting/reporting_serializer.cc:278:
> On 2017/04/05 18:47:31, shivanisha wrote:
> > DCHECK that the cache does not contain any reports or clients.
>
> Done.
>
>
https://codereview.chromium.org/2751883003/diff/60001/net/reporting/reporting...
> File net/reporting/reporting_serializer.h (right):
>
>
https://codereview.chromium.org/2751883003/diff/60001/net/reporting/reporting...
> net/reporting/reporting_serializer.h:18: class NET_EXPORT ReportingSerializer
{
> On 2017/04/05 18:47:31, shivanisha wrote:
> > Comment describing the overall purpose of the class.
>
> Done.
lgtm
3 years, 8 months ago
(2017-04-07 23:45:04 UTC)
#27
Dry run: This issue passed the CQ dry run.
shivanisha
https://codereview.chromium.org/2751883003/diff/140002/net/reporting/reporting_observer.cc File net/reporting/reporting_observer.cc (right): https://codereview.chromium.org/2751883003/diff/140002/net/reporting/reporting_observer.cc#newcode11 net/reporting/reporting_observer.cc:11: void ReportingObserver::OnContextInitialized() {} There's no implementation of this in ...
3 years, 8 months ago
(2017-04-10 18:29:29 UTC)
#28
3 years, 8 months ago
(2017-04-11 19:20:47 UTC)
#32
PTAL, shivanisha.
https://codereview.chromium.org/2751883003/diff/140002/net/reporting/reportin...
File net/reporting/reporting_observer.cc (right):
https://codereview.chromium.org/2751883003/diff/140002/net/reporting/reportin...
net/reporting/reporting_observer.cc:11: void
ReportingObserver::OnContextInitialized() {}
On 2017/04/10 18:29:29, shivanisha wrote:
> There's no implementation of this in any of the observers?
> Will the ReportingGarbageCollectorImpl need to implement any of these?
The DeliveryAgent will need to implement OnContextInitialized to examine queued
reports from prior sessions (if any; currently our default policy prohibits
persisting them) and attempt to deliver them.
The GarbageCollector could also use it as the first time to do garbage
collection.
Come to think of it, though, there's a pretty particular order in which the
classes should be initialized (Persister, then GarbageCollector, then
DeliveryAgent), so maybe I should just hard-code that instead of using observer
callbacks.
https://codereview.chromium.org/2751883003/diff/140002/net/reporting/reportin...
File net/reporting/reporting_persister.cc (right):
https://codereview.chromium.org/2751883003/diff/140002/net/reporting/reportin...
net/reporting/reporting_persister.cc:90: void OnCacheUpdated() override {
On 2017/04/10 18:29:29, shivanisha wrote:
> dcheck that context is initialized. Also in ReportingGarbageCollectorImpl.
Done.
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2751883003/210001
3 years, 8 months ago
(2017-04-11 19:21:01 UTC)
#33
3 years, 8 months ago
(2017-04-11 20:25:47 UTC)
#35
Dry run: This issue passed the CQ dry run.
shivanisha
On 2017/04/11 at 19:20:47, juliatuttle wrote: > PTAL, shivanisha. > > https://codereview.chromium.org/2751883003/diff/140002/net/reporting/reporting_observer.cc > File net/reporting/reporting_observer.cc ...
3 years, 8 months ago
(2017-04-13 17:05:10 UTC)
#36
On 2017/04/11 at 19:20:47, juliatuttle wrote:
> PTAL, shivanisha.
>
>
https://codereview.chromium.org/2751883003/diff/140002/net/reporting/reportin...
> File net/reporting/reporting_observer.cc (right):
>
>
https://codereview.chromium.org/2751883003/diff/140002/net/reporting/reportin...
> net/reporting/reporting_observer.cc:11: void
ReportingObserver::OnContextInitialized() {}
> On 2017/04/10 18:29:29, shivanisha wrote:
> > There's no implementation of this in any of the observers?
> > Will the ReportingGarbageCollectorImpl need to implement any of these?
>
> The DeliveryAgent will need to implement OnContextInitialized to examine
queued reports from prior sessions (if any; currently our default policy
prohibits persisting them) and attempt to deliver them.
>
> The GarbageCollector could also use it as the first time to do garbage
collection.
>
> Come to think of it, though, there's a pretty particular order in which the
classes should be initialized (Persister, then GarbageCollector, then
DeliveryAgent), so maybe I should just hard-code that instead of using observer
callbacks.
>
>
https://codereview.chromium.org/2751883003/diff/140002/net/reporting/reportin...
> File net/reporting/reporting_persister.cc (right):
>
>
https://codereview.chromium.org/2751883003/diff/140002/net/reporting/reportin...
> net/reporting/reporting_persister.cc:90: void OnCacheUpdated() override {
> On 2017/04/10 18:29:29, shivanisha wrote:
> > dcheck that context is initialized. Also in ReportingGarbageCollectorImpl.
>
> Done.
lgtm
Julia Tuttle
Thanks, shivanisha! PTAL csharrison for committer signoff.
3 years, 8 months ago
(2017-04-13 17:08:34 UTC)
#37
Thanks, shivanisha! PTAL csharrison for committer signoff.
Charlie Harrison
LGTM
3 years, 8 months ago
(2017-04-13 17:20:32 UTC)
#38
LGTM
Julia Tuttle
The CQ bit was checked by juliatuttle@chromium.org
3 years, 8 months ago
(2017-04-13 17:22:22 UTC)
#39
CQ is committing da patch. Bot data: {"patchset_id": 210001, "attempt_start_ts": 1492104142668720, "parent_rev": "786d0251c33344501eed4dc016e84416bad11fe1", "commit_rev": "7da138164fb7dc30540f1340df0fd93b4d417a34"}
3 years, 8 months ago
(2017-04-13 19:18:24 UTC)
#41
CQ is committing da patch.
Bot data: {"patchset_id": 210001, "attempt_start_ts": 1492104142668720,
"parent_rev": "786d0251c33344501eed4dc016e84416bad11fe1", "commit_rev":
"7da138164fb7dc30540f1340df0fd93b4d417a34"}
commit-bot: I haz the power
Description was changed from ========== Reporting: Implement serializer. Reporting is a spec for delivering out-of-band ...
3 years, 8 months ago
(2017-04-13 19:19:13 UTC)
#42
Message was sent while issue was closed.
Description was changed from
==========
Reporting: Implement serializer.
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.
The serializer can serialize and deserialize the data in the cache (and
potentially other places eventually, like the endpoint manager's state).
It will be used to persist selected state of the Reporting system
across browser/embedder restarts.
BUG=704259
==========
to
==========
Reporting: Implement serializer.
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.
The serializer can serialize and deserialize the data in the cache (and
potentially other places eventually, like the endpoint manager's state).
It will be used to persist selected state of the Reporting system
across browser/embedder restarts.
BUG=704259
Review-Url: https://codereview.chromium.org/2751883003
Cr-Commit-Position: refs/heads/master@{#464496}
Committed:
https://chromium.googlesource.com/chromium/src/+/7da138164fb7dc30540f1340df0f...
==========
commit-bot: I haz the power
Committed patchset #12 (id:210001) as https://chromium.googlesource.com/chromium/src/+/7da138164fb7dc30540f1340df0fd93b4d417a34
3 years, 8 months ago
(2017-04-13 19:19:14 UTC)
#43
Issue 2751883003: Reporting: Implement serializer.
(Closed)
Created 3 years, 9 months ago by Julia Tuttle
Modified 3 years, 8 months ago
Reviewers: shivanisha, Charlie Harrison
Base URL:
Comments: 14
Chromium Code Reviews
Issue
2751883003:
Reporting: Implement serializer. (Closed)
