Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2483003003:
Introduce CertificateReportingService class to handle certificate reports. (Closed)
DescriptionIntroduce CertificateReportingService class to handle certificate reports.
This class is eventually going to handle uploading and retrying of certificate
reports.
In this CL, it contains three sub classes:
Report: A struct that represents the report to be sent.
BoundedReportList: A container that contains a bounded number of certificate
reports, ordered by report creation date. First report is the newest, last
report is the oldest.
EventObserver: A class to observe events generated by the service and reporter.
Reporter: The class that contains the actual retry logic.
CertificateReportingService is not yet wired to any certificate reporting path
and does not contain reporting logic. Those will be added in follow up CLs along
with browser tests.
BUG=554323
Committed: https://crrev.com/35c6b90321c0174bf1a1841addddce21f7678041
Cr-Commit-Position: refs/heads/master@{#434540}
Patch Set 1 #Patch Set 2 : Rebase #
Total comments: 15
Patch Set 3 : estark comments #Patch Set 4 : Stashed changes are back #
Total comments: 1
Patch Set 5 : Move CertificateReportingService to safe_browsing #
Total comments: 11
Patch Set 6 : jialiul comments #
Messages
Total messages: 36 (12 generated)
meacer@chromium.org changed reviewers: + estark@chromium.org
estark: Another one. This is 3rd CL for report retries and is based on https://codereview.chromium.org/2483993002/.
Sorry for the rebase, had to do it since I changed ErrorReporter to take callbacks in the send method now.
Didn't make it through the whole thing yet but I left a few high-level comments and a few nits. Also, I know I asked you this before but my goldfish memory has forgotten... can you explain again why the KeyedService is necessary/useful? I don't really understand what a KeyedService is and the comments on the KeyedService class aren't helping me. https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... File chrome/browser/ssl/certificate_reporting_service.h (right): https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:24: // This service initiates uploads invalid certificate reports and retries any extra word in this sentence? https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:33: Report() {} do you need this constructor? https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:62: // oldest item in the list. This sentence "A newly added item is compared..." only applies when the number of items == max_size_, right? https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:69: // A class to observe events by the service. Used for testing. True confessions, I haven't read the tests yet... or the .cc file... so I'm not sure yet how this will be used. But it seems unfortunate that we need a whole interface just for testing. And also a little fragile, maybe -- like I can imagine that the code could change in such a way that reports don't get sent but the OnSendAttempt event still fires. Is it possible to test this in any other way? Like a URLRequestInterceptor that intercepts reports and keeps track of them? https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:76: // |sent| is false. Otherwise, it's true. sent => completed https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:91: class Reporter : public base::RefCountedThreadSafe<Reporter> { ALERT ALERT REFCOUNTED ALERT Glanced at the .cc file and it looks like this only needs to be refcounted for the cross-thread EventObserver interface, is that true? If indeed the EventObserver interface is necessary (see comment above) and this needs to be refcounted, might be worth documenting here what should be called on what thread. https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:104: // Sends all pending reports. Skips reports older than max_item_age_. Failed Tiny nit: referring to a private member on a public method's documentation seems weird. Maybe "older than the |max_item_age| provided in the constructor"? https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:106: void SendPending(); No code change necessary, but for my own understanding, can you explain a little how this will be called? Periodically? Will there be any kind of back-off?
KeyedServices are registered during browser initialization and are alive all the way to shutdown. They provide their own ShutDown code to do cleanup before being destructed. It's not necessary here so I removed it, but I'll need it follow up CLs. KeyedService provides profile-keyed access to the service, and we'll need to call into this service from ChromeContentBrowserClient using that. https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... File chrome/browser/ssl/certificate_reporting_service.h (right): https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:24: // This service initiates uploads invalid certificate reports and retries any On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > extra word in this sentence? Missing, in fact :) https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:33: Report() {} On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > do you need this constructor? [] operator of std::map needs it, but I modified the code to avoid using it. Slightly more code, but got rid of this constructor. https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:62: // oldest item in the list. On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > This sentence "A newly added item is compared..." only applies when the number > of items == max_size_, right? Correct, clarified the comment a bit. https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:69: // A class to observe events by the service. Used for testing. On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > True confessions, I haven't read the tests yet... or the .cc file... so I'm not > sure yet how this will be used. But it seems unfortunate that we need a whole > interface just for testing. And also a little fragile, maybe -- like I can > imagine that the code could change in such a way that reports don't get sent but > the OnSendAttempt event still fires. > > Is it possible to test this in any other way? Like a URLRequestInterceptor that > intercepts reports and keeps track of them? Looks like this isn't actually used yet, so I removed it. I'm most likely going to reintroduce it in the follow up CLs though. I have browser tests that have an interceptor for follow up CLs, but the interceptor can't determine the report ids from the URLRequests since we no longer associate report ids with the URLRequests. For that reason, the event observer will still be necessary. I hope the next CL will make it clear why we need it, but I agree it's confusing to have it here. https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:76: // |sent| is false. Otherwise, it's true. On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > sent => completed Done. https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:91: class Reporter : public base::RefCountedThreadSafe<Reporter> { On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > ALERT ALERT REFCOUNTED ALERT > > Glanced at the .cc file and it looks like this only needs to be refcounted for > the cross-thread EventObserver interface, is that true? > > If indeed the EventObserver interface is necessary (see comment above) and this > needs to be refcounted, might be worth documenting here what should be called on > what thread. Yeah, this was no longer necessary and I removed it in follow up CLs. Sorry that this is a bit old patchset, removed it from here as well. https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.h:106: void SendPending(); On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > No code change necessary, but for my own understanding, can you explain a little > how this will be called? Periodically? Will there be any kind of back-off? It will be called periodically by the Metrics service. They do have their own scheduling and back-off code: https://cs.chromium.org/chromium/src/components/metrics/metrics_service.cc?sq...
Looks like I messed up the patchsets. Please wait for Patchset #4 before reviewing.
Okay, I was missing some of the changes, it's ready for review now. Note that Reporter is now a WeakPtr, because it might go destroyed when the report upload is happening (doesn't happen here yet, but CertificateReportingService will recreate it when SafeBrowsing or extended reporting preferences change).
On 2016/11/17 20:09:08, Mustafa Emre Acer wrote: > KeyedServices are registered during browser initialization and are alive all the > way to shutdown. They provide their own ShutDown code to do cleanup before being > destructed. It's not necessary here so I removed it, but I'll need it follow up > CLs. KeyedService provides profile-keyed access to the service, and we'll need > to call into this service from ChromeContentBrowserClient using that. Ok, thanks, I also found an email thread about this... Is persisting reports on disk part of the motivation for using a service too? (Does it do that?) I'm trying to understand why we're not just having the safe browsing PingManager use an error callback that retries report-sending (say, up to 5 times per report, every 5 minutes or something). Are you wanting to take advantage of the Metrics service scheduling/backoff? > > https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... > File chrome/browser/ssl/certificate_reporting_service.h (right): > > https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... > chrome/browser/ssl/certificate_reporting_service.h:24: // This service initiates > uploads invalid certificate reports and retries any > On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > > extra word in this sentence? > > Missing, in fact :) > > https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... > chrome/browser/ssl/certificate_reporting_service.h:33: Report() {} > On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > > do you need this constructor? > > [] operator of std::map needs it, but I modified the code to avoid using it. > Slightly more code, but got rid of this constructor. > > https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... > chrome/browser/ssl/certificate_reporting_service.h:62: // oldest item in the > list. > On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > > This sentence "A newly added item is compared..." only applies when the number > > of items == max_size_, right? > > Correct, clarified the comment a bit. > > https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... > chrome/browser/ssl/certificate_reporting_service.h:69: // A class to observe > events by the service. Used for testing. > On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > > True confessions, I haven't read the tests yet... or the .cc file... so I'm > not > > sure yet how this will be used. But it seems unfortunate that we need a whole > > interface just for testing. And also a little fragile, maybe -- like I can > > imagine that the code could change in such a way that reports don't get sent > but > > the OnSendAttempt event still fires. > > > > Is it possible to test this in any other way? Like a URLRequestInterceptor > that > > intercepts reports and keeps track of them? > > Looks like this isn't actually used yet, so I removed it. > > I'm most likely going to reintroduce it in the follow up CLs though. I have > browser tests that have an interceptor for follow up CLs, but the interceptor > can't determine the report ids from the URLRequests since we no longer associate > report ids with the URLRequests. For that reason, the event observer will still > be necessary. I hope the next CL will make it clear why we need it, but I agree > it's confusing to have it here. > > https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... > chrome/browser/ssl/certificate_reporting_service.h:76: // |sent| is false. > Otherwise, it's true. > On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > > sent => completed > > Done. > > https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... > chrome/browser/ssl/certificate_reporting_service.h:91: class Reporter : public > base::RefCountedThreadSafe<Reporter> { > On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > > ALERT ALERT REFCOUNTED ALERT > > > > Glanced at the .cc file and it looks like this only needs to be refcounted for > > the cross-thread EventObserver interface, is that true? > > > > If indeed the EventObserver interface is necessary (see comment above) and > this > > needs to be refcounted, might be worth documenting here what should be called > on > > what thread. > > Yeah, this was no longer necessary and I removed it in follow up CLs. Sorry that > this is a bit old patchset, removed it from here as well. > > https://codereview.chromium.org/2483003003/diff/20001/chrome/browser/ssl/cert... > chrome/browser/ssl/certificate_reporting_service.h:106: void SendPending(); > On 2016/11/17 14:51:59, estark (slow thru Nov 18) wrote: > > No code change necessary, but for my own understanding, can you explain a > little > > how this will be called? Periodically? Will there be any kind of back-off? > > It will be called periodically by the Metrics service. They do have their own > scheduling and back-off code: > https://cs.chromium.org/chromium/src/components/metrics/metrics_service.cc?sq...
On 2016/11/18 02:55:17, estark (slow thru Nov 18) wrote: > On 2016/11/17 20:09:08, Mustafa Emre Acer wrote: > > KeyedServices are registered during browser initialization and are alive all > the > > way to shutdown. They provide their own ShutDown code to do cleanup before > being > > destructed. It's not necessary here so I removed it, but I'll need it follow > up > > CLs. KeyedService provides profile-keyed access to the service, and we'll need > > to call into this service from ChromeContentBrowserClient using that. > > Ok, thanks, I also found an email thread about this... Is persisting reports on > disk part of the motivation for using a service too? (Does it do that?) I'm > trying to understand why we're not just having the safe browsing PingManager use > an error callback that retries report-sending (say, up to 5 times per report, > every 5 minutes or something). Are you wanting to take advantage of the Metrics > service scheduling/backoff? > We don't persist reports anymore, they are in-memory. There is a cap on the maximum number of queued reports (currently 5) so this shouldn't be an issue in terms of memory usage. And as you said, I'm not using PingManager because I wanted to use the scheduling and backoff code in Metrics service.
Thanks for the explanations, lgtm https://codereview.chromium.org/2483003003/diff/60001/chrome/browser/ssl/cert... File chrome/browser/ssl/certificate_reporting_service.cc (right): https://codereview.chromium.org/2483003003/diff/60001/chrome/browser/ssl/cert... chrome/browser/ssl/certificate_reporting_service.cc:25: // Report older than the oldest item in the queue, ignore. The comments in this function are helpful 👍
Thanks, though, I probably won't land this CL (sorry!) For some reason I was focused on putting all this code under chrome/browser/ssl. This led to a lot of extra code since we don't want to depend on SafeBrowsing code from browser/ssl, but we also need to use SB request context and observe SB preference changes. You can see what you were about to review up next in here: https://codereview.chromium.org/2503243003/ I'm now moving the code to safe_browsing instead, which should get rid of the indirections and make the code much simpler. The reports will now go through ping manager as you said.
meacer@chromium.org changed reviewers: + jialiul@chromium.org
Okay, a change of plans again and I'm going to land this CL. I moved the code to chrome/browser/safe_browsing. jialul: Can you PTAL? The rationale for adding this new class is described here: https://docs.google.com/document/d/1pBKCrV9U1HfgGoapsbcrbf45w-nbjMWQD_IXMS7pQ... Thanks!
lgtm with some nits https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... File chrome/browser/safe_browsing/certificate_reporting_service.cc (right): https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... chrome/browser/safe_browsing/certificate_reporting_service.cc:11: // First item is ordered before the second item if it's newer. How about "Compare function that orders Reports in reverse chronological order." https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... chrome/browser/safe_browsing/certificate_reporting_service.cc:32: std::sort(items_.begin(), items_.end(), ReportCompareFunc); Sorting seems unnecessary. You only need to keep the freshest max_size_ reports. So when the vector is full, do a linear search for the oldest report, erase it, and push_back a new one O(n). it is more efficient than sort the vector (o(nlogn) every time you add a report. https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... chrome/browser/safe_browsing/certificate_reporting_service.cc:35: CertificateReportingService::BoundedReportList::BoundedReportList( nit: could you order the functions in certificate_reporting_service.cc file in the same order of certificate_reporting_service.h file? https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... chrome/browser/safe_browsing/certificate_reporting_service.cc:87: retry_list_->Clear(); retry_list_ got cleared twice? (one @ line 70, then @ line 87) https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... File chrome/browser/safe_browsing/certificate_reporting_service.h (right): https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... chrome/browser/safe_browsing/certificate_reporting_service.h:99: const base::TimeDelta max_item_age_; maybe call it report_ttl_?
Thanks Jialiu. https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... File chrome/browser/safe_browsing/certificate_reporting_service.cc (right): https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... chrome/browser/safe_browsing/certificate_reporting_service.cc:11: // First item is ordered before the second item if it's newer. On 2016/11/23 23:06:14, Jialiu Lin wrote: > How about "Compare function that orders Reports in reverse chronological order." Done. https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... chrome/browser/safe_browsing/certificate_reporting_service.cc:32: std::sort(items_.begin(), items_.end(), ReportCompareFunc); On 2016/11/23 23:06:14, Jialiu Lin wrote: > Sorting seems unnecessary. You only need to keep the freshest max_size_ reports. > So when the vector is full, do a linear search for the oldest report, erase it, > and push_back a new one O(n). it is more efficient than sort the vector > (o(nlogn) every time you add a report. It is indeed unnecessary to sort every time, but the list is small enough (currently 5 items, though we can bump it to 10 later on) that there shouldn't be any performance penalty here. Also, sorting makes testing easier as the tests know which report to check. I'd like to keep this as is, if you don't feel strongly. I added a check at the top to make sure max_size isn't more than 20. https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... chrome/browser/safe_browsing/certificate_reporting_service.cc:35: CertificateReportingService::BoundedReportList::BoundedReportList( On 2016/11/23 23:06:14, Jialiu Lin wrote: > nit: could you order the functions in certificate_reporting_service.cc > file in the same order of certificate_reporting_service.h file? Done. https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... chrome/browser/safe_browsing/certificate_reporting_service.cc:87: retry_list_->Clear(); On 2016/11/23 23:06:14, Jialiu Lin wrote: > retry_list_ got cleared twice? (one @ line 70, then @ line 87) Done, good catch! https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... File chrome/browser/safe_browsing/certificate_reporting_service.h (right): https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... chrome/browser/safe_browsing/certificate_reporting_service.h:99: const base::TimeDelta max_item_age_; On 2016/11/23 23:06:14, Jialiu Lin wrote: > maybe call it report_ttl_? Done.
still lgtm.:-) https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... File chrome/browser/safe_browsing/certificate_reporting_service.cc (right): https://codereview.chromium.org/2483003003/diff/80001/chrome/browser/safe_bro... chrome/browser/safe_browsing/certificate_reporting_service.cc:32: std::sort(items_.begin(), items_.end(), ReportCompareFunc); On 2016/11/23 at 23:59:02, Mustafa Emre Acer wrote: > On 2016/11/23 23:06:14, Jialiu Lin wrote: > > Sorting seems unnecessary. You only need to keep the freshest max_size_ reports. > > So when the vector is full, do a linear search for the oldest report, erase it, > > and push_back a new one O(n). it is more efficient than sort the vector > > (o(nlogn) every time you add a report. > > It is indeed unnecessary to sort every time, but the list is small enough (currently 5 items, though we can bump it to 10 later on) that there shouldn't be any performance penalty here. Also, sorting makes testing easier as the tests know which report to check. I'd like to keep this as is, if you don't feel strongly. > > I added a check at the top to make sure max_size isn't more than 20. SGTM. Acknowledged.
Thanks! Landing.
The CQ bit was checked by meacer@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from estark@chromium.org Link to the patchset: https://codereview.chromium.org/2483003003/#ps100001 (title: "jialiul comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by meacer@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by meacer@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by meacer@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1480097878453090,
"parent_rev": "f1f172b00623660979c5e21c8fe28cbd681d6c59", "commit_rev":
"5c81bc747b67a4018568485816722bc02c573056"}
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Introduce CertificateReportingService class to handle certificate reports. This class is eventually going to handle uploading and retrying of certificate reports. In this CL, it contains three sub classes: Report: A struct that represents the report to be sent. BoundedReportList: A container that contains a bounded number of certificate reports, ordered by report creation date. First report is the newest, last report is the oldest. EventObserver: A class to observe events generated by the service and reporter. Reporter: The class that contains the actual retry logic. CertificateReportingService is not yet wired to any certificate reporting path and does not contain reporting logic. Those will be added in follow up CLs along with browser tests. BUG=554323 ========== to ========== Introduce CertificateReportingService class to handle certificate reports. This class is eventually going to handle uploading and retrying of certificate reports. In this CL, it contains three sub classes: Report: A struct that represents the report to be sent. BoundedReportList: A container that contains a bounded number of certificate reports, ordered by report creation date. First report is the newest, last report is the oldest. EventObserver: A class to observe events generated by the service and reporter. Reporter: The class that contains the actual retry logic. CertificateReportingService is not yet wired to any certificate reporting path and does not contain reporting logic. Those will be added in follow up CLs along with browser tests. BUG=554323 Committed: https://crrev.com/35c6b90321c0174bf1a1841addddce21f7678041 Cr-Commit-Position: refs/heads/master@{#434540} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/35c6b90321c0174bf1a1841addddce21f7678041 Cr-Commit-Position: refs/heads/master@{#434540} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
