Add page load abort metrics to PageLoadMetrics system

components/page_load_metrics/browser/metrics_web_contents_observer.h

 // Copyright 2015 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_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_WEB_CONTENTS_OBSERVER_H_
 #define COMPONENTS_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_WEB_CONTENTS_OBSERVER_H_
 
 #include "base/containers/scoped_ptr_map.h"
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "components/page_load_metrics/common/page_load_timing.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
+#include "net/base/net_errors.h"
 
 namespace content {
 class NavigationHandle;
 class RenderFrameHost;
 }  // namespace content
 
 namespace IPC {
 class Message;
 }  // namespace IPC
 
 namespace page_load_metrics {
 
+enum PageLoadEvent {
+  STARTED_PROVISIONAL_LOAD,

[Randy Smith (Not in Mondays), 2015/10/01 16:07:39]

I'd like to have comments on these values that explain to someone who isn't
knowledgeable in the renderer space (like, say, me :-}) what the enums mean. 
What is a provisional load and how does it compare with a non-provisional load? 


Also, arguably even more important: It should be clear to the reader (either
here or in histograms.xml) which of these events are exclusive, which are
subsets of each other, and which are independent.  I'm assuming that
STARTED_PROVISIONAL_LOAD will be recorded if any of the other three are
recorded, and that the other three are disjoint and (at least in theory?) at
least one of them will be recorded for any page.  That right?

[Charlie Harrison, 2015/10/01 16:59:55]

Will do, good suggestions.

+  ABORTED_PROVISIONAL_LOAD,
+  ABORTED_LOAD_BEFORE_FIRST_LAYOUT,
+  SUCCESSFUL_FIRST_LAYOUT,
+
+  // Add values before this final count.
+  EVENT_COUNT

[Randy Smith (Not in Mondays), 2015/10/01 16:07:39]

I *think* that the custom for these is the enum name (in cap/underscore format)
with _LAST_ENTRY appended to it.  That's what appears to be used in cookies and
downloads (which may mean I've created the custom, which is why I'm being
hesitant).  Also, usually all entries in an enum have a prefix associated with
the enum type.  Both of these are for avoiding namespace collisions.  See, e.g.,
https://chromium.googlesource.com/chromium/src/+blame/master/content/browser/....

+};
+
 class PageLoadTracker {
  public:
   explicit PageLoadTracker(bool in_foreground);
   ~PageLoadTracker();
   void Commit();
   void WebContentsHidden();
 
   // Returns true if the timing was successfully updated.
   bool UpdateTiming(const PageLoadTiming& timing);
+  void RecordEvent(PageLoadEvent event);
 
  private:
   void RecordTimingHistograms();
 
   bool has_commit_;
 
   // We record separate metrics for events that occur after a background,
   // because metrics like layout/paint are delayed artificially
   // when they occur in the bacground.
   base::TimeTicks background_time_;
@@ skipping 44 matching lines @@
   base::ScopedPtrMap<content::NavigationHandle*, scoped_ptr<PageLoadTracker>>
       provisional_loads_;
   scoped_ptr<PageLoadTracker> committed_load_;
 
   DISALLOW_COPY_AND_ASSIGN(MetricsWebContentsObserver);
 };
 
 }  // namespace page_load_metrics
 
 #endif  // COMPONENTS_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_WEB_CONTENTS_OBSERVER_H_