Domain Reliability: More security review.

components/domain_reliability/scheduler.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_DOMAIN_RELIABILITY_SCHEDULER_H_
 #define COMPONENTS_DOMAIN_RELIABILITY_SCHEDULER_H_
 
 #include <vector>
 
 #include "base/callback.h"
@@ skipping 32 matching lines @@
     static Params GetFromFieldTrialsOrDefaults();
   };
 
   DomainReliabilityScheduler(MockableTime* time,
                              size_t num_collectors,
                              const Params& params,
                              const ScheduleUploadCallback& callback);
   ~DomainReliabilityScheduler();
 
   // Called when a beacon is added to the context. May call the provided
   // ScheduleUploadCallback (if all previous beacons have been uploaded).

[Ryan Sleevi, 2014/04/26 02:37:39]

Comment nit: Don't describe how this *is* used, describe what it does and how to
use it.

For example, my first reaction is "Called by who?"
My next reaction is "Hrm, On* methods are usually used for delegate. Is this a
delegate? No? So when do I call this?"

[Deprecated (see juliatuttle), 2014/04/28 22:01:13]

Done.

   void OnBeaconAdded();
 
   // Called when an upload is about to start.
   void OnUploadStart(int* collector_index_out);

[Ryan Sleevi, 2014/04/26 02:37:39]

BUG: You define |num_collectors| as a size_t, but then co-erce to an int (also
line 78). Your types should match - and, since they're indices, they should be
size_t

[Deprecated (see juliatuttle), 2014/04/28 22:01:13]

Done.

 
   // Called when an upload has finished. May call the provided
   // ScheduleUploadCallback (if the upload failed, or if beacons arrived
   // during the upload).
   void OnUploadComplete(bool success);
 
  private:
   struct CollectorState {
     CollectorState();
 
     // The number of consecutive failures to upload to this collector, or 0 if
     // the most recent upload succeeded.
     int failures;
     base::TimeTicks next_upload;
   };
 
   void MaybeScheduleUpload();
 
   void GetNextUploadTimeAndCollector(base::TimeTicks now,
                                      base::TimeTicks* upload_time_out,
                                      int* collector_index_out);
+
   base::TimeDelta GetUploadRetryInterval(int failures);
 
   MockableTime* time_;
   std::vector<CollectorState> collectors_;
   Params params_;
   ScheduleUploadCallback callback_;
 
   // Whether there are beacons that have not yet been uploaded. Set when a
   // beacon arrives or an upload fails, and cleared when an upload starts.
   bool upload_pending_;
@@ skipping 15 matching lines @@
   // upload.
   base::TimeTicks first_beacon_time_;
 
   // first_beacon_time_ saved during uploads.  Restored if upload fails.
   base::TimeTicks old_first_beacon_time_;
 };
 
 }  // namespace domain_reliability
 
 #endif  // COMPONENTS_DOMAIN_RELIABILITY_SCHEDULER_H_