Domain Reliability: Simplify configs and reports

components/domain_reliability/context.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_CONTEXT_H_
 #define COMPONENTS_DOMAIN_RELIABILITY_CONTEXT_H_
 
 #include <deque>
 #include <vector>
 
@@ skipping 12 matching lines @@
 class Value;
 }
 
 namespace domain_reliability {
 
 class DomainReliabilityDispatcher;
 class DomainReliabilityUploader;
 class MockableTime;
 
 // The per-domain context for the Domain Reliability client; includes the
 // domain's config and per-resource beacon queues.

[Randy Smith (Not in Mondays), 2015/10/29 22:36:10]

Is this comment still valid?  I see a beacon queue, but it doesn't look
per-resource.

[Deprecated (see juliatuttle), 2015/11/02 23:19:30]

Wow, no, that is super out of date. Fixed.

 class DOMAIN_RELIABILITY_EXPORT DomainReliabilityContext {
  public:
   class DOMAIN_RELIABILITY_EXPORT Factory {
    public:
     virtual ~Factory();
     virtual scoped_ptr<DomainReliabilityContext> CreateContextForConfig(
         scoped_ptr<const DomainReliabilityConfig> config) = 0;
   };
 
   DomainReliabilityContext(
       MockableTime* time,
       const DomainReliabilityScheduler::Params& scheduler_params,
       const std::string& upload_reporter_string,
       const base::TimeTicks* last_network_change_time,
       DomainReliabilityDispatcher* dispatcher,
       DomainReliabilityUploader* uploader,
       scoped_ptr<const DomainReliabilityConfig> config);
   ~DomainReliabilityContext();
 
   // Notifies the context of a beacon on its domain(s); may or may not save the
   // actual beacon to be uploaded, depending on the sample rates in the config,
   // but will increment one of the request counters in any case.
-  void OnBeacon(const GURL& url, const DomainReliabilityBeacon& beacon);
+  void OnBeacon(scoped_ptr<DomainReliabilityBeacon> beacon);
 
   // Called to clear browsing data, since beacons are like browsing history.
   void ClearBeacons();
 
   // Gets a Value containing data that can be formatted into a web page for
   // debugging purposes.
   scoped_ptr<base::Value> GetWebUIData() const;
 
+  // Gets the beacons queued for upload in this context. |beacons_out| will be

[Randy Smith (Not in Mondays), 2015/10/29 22:36:11]

nit: |*beacons_out|.

[Deprecated (see juliatuttle), 2015/11/02 23:19:30]

Done.

+  // cleared and filled with pointers to the beacons; the pointers remain valid
+  // as long as no other requests are reported to the DomainReliabilityMonitor.

[Randy Smith (Not in Mondays), 2015/10/29 22:36:11]

nit, suggestion: These lifetime related requirements make me uncomfortable
because they aren't very simple.  If this wasn't test code, I'd be trying hard
to get you to change them--given that it is test code, I'll just suggest you
change them.  Looking at the usage points, I'd think you could replace this
method with two other methods, one getting the number of elements, and the other
getting a const reference to a particular element.  Const references *can* be
stored beyond the lifetime of the thing referred to, but it's a bit harder; they
tend to be used right where they are.  Just a thought.

[Deprecated (see juliatuttle), 2015/11/02 23:19:30]

Would you be okay if I punted this to another CL to avoid more churn in the unit
tests?

[Randy Smith (Not in Mondays), 2015/11/03 21:48:12]

Sure.

   void GetQueuedBeaconsForTesting(
-      std::vector<DomainReliabilityBeacon>* beacons_out) const;
-
-  void GetRequestCountsForTesting(
-      size_t resource_index,
-      uint32* successful_requests_out,
-      uint32* failed_requests_out) const;
+      std::vector<const DomainReliabilityBeacon*>* beacons_out) const;
 
   const DomainReliabilityConfig& config() const { return *config_.get(); }
 
   // Maximum number of beacons queued per context; if more than this many are
   // queued; the oldest beacons will be removed.
   static const size_t kMaxQueuedBeacons;
 
  private:
-  class ResourceState;
+  typedef std::deque<DomainReliabilityBeacon*> BeaconDeque;
 
-  typedef std::deque<DomainReliabilityBeacon> BeaconDeque;
-  typedef ScopedVector<ResourceState> ResourceStateVector;
-  typedef ResourceStateVector::const_iterator ResourceStateIterator;
-
-  void InitializeResourceStates();
   void ScheduleUpload(base::TimeDelta min_delay, base::TimeDelta max_delay);
   void StartUpload();
   void OnUploadComplete(const DomainReliabilityUploader::UploadResult& result);
 
-  scoped_ptr<const base::Value> CreateReport(base::TimeTicks upload_time) const;
+  scoped_ptr<const base::Value> CreateReport(base::TimeTicks upload_time,
+                                             const GURL& collector_url) const;
 
   // Remembers the current state of the context when an upload starts. Can be
   // called multiple times in a row (without |CommitUpload|) if uploads fail
   // and are retried.
   void MarkUpload();
 
   // Uses the state remembered by |MarkUpload| to remove successfully uploaded
   // data but keep beacons and request counts added after the upload started.
   void CommitUpload();
 
   void RollbackUpload();
 
   // Finds and removes the oldest beacon. DCHECKs if there is none. (Called
   // when there are too many beacons queued.)
   void RemoveOldestBeacon();
 
   scoped_ptr<const DomainReliabilityConfig> config_;
   MockableTime* time_;
   const std::string& upload_reporter_string_;
   DomainReliabilityScheduler scheduler_;
   DomainReliabilityDispatcher* dispatcher_;
   DomainReliabilityUploader* uploader_;
 
   BeaconDeque beacons_;
   size_t uploading_beacons_size_;
-  // Each ResourceState in |states_| corresponds to the Resource of the same
-  // index in the config.
-  ResourceStateVector states_;
   base::TimeTicks upload_time_;
   base::TimeTicks last_upload_time_;
   // The last network change time is not tracked per-context, so this is a
   // pointer to that value in a wider (e.g. per-Monitor or unittest) scope.
   const base::TimeTicks* last_network_change_time_;
 
   base::WeakPtrFactory<DomainReliabilityContext> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DomainReliabilityContext);
 };
 
 }  // namespace domain_reliability
 
 #endif  // COMPONENTS_DOMAIN_RELIABILITY_CONTEXT_H_