PageLoadMetrics renderer and browser implementation.

components/page_load_metrics/browser/page_load_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/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"
+
+namespace content {
+class NavigationHandle;
+class RenderFrameHost;
+}  // namespace content
+
+namespace IPC {
+class Message;
+}  // namespace IPC
+
+namespace page_load_metrics {
+
+struct PageLoadTiming;
+
+// WebContentsObserver logs page load UMA metrics based on
+// IPC messages received from a MainRenderFrameObserver.
+class WebContentsObserver
+    : public content::WebContentsObserver,
+      public content::WebContentsUserData<WebContentsObserver> {
+ public:
+  ~WebContentsObserver() override;
+
+  // WebContentsObserver implementation
+  bool OnMessageReceived(const IPC::Message& message,
+                         content::RenderFrameHost* render_frame_host) override;
+  void DidCommitNavigation(
+      content::NavigationHandle* navigation_handle) override;
+

[Bryan McQuade, 2015/09/02 18:26:45]

randy had suggested keeping all of the overridden methods together (removing
newlines between them), so let's remove this one.

[Charlie Harrison, 2015/09/03 14:00:52]

Done.

+  void RenderProcessGone(base::TerminationStatus status) override;
+
+ private:
+  explicit WebContentsObserver(content::WebContents* web_contents);
+  friend class content::WebContentsUserData<WebContentsObserver>;
+  friend class WebContentsObserverTest;
+  friend class MockWebContentsObserver;
+
+  void OnTimingUpdated(const PageLoadTiming& timing);
+  void RecordTimingHistograms();
+  virtual const GURL& GetLastCommittedURL();

[Bryan McQuade, 2015/09/02 18:26:46]

a private method doesn't benefit from being virtual, since by being private it
cannot be overridden in base classes. so i'd suggest making this non-virtual.

[Charlie Harrison, 2015/09/03 14:00:52]

Done.

+
+  PageLoadTiming current_timing_;

[Bryan McQuade, 2015/09/02 18:26:46]

is it possible for us to get rid of having two members, and just have a single
'timing_' member?

if there's some need to keep temporary timing state around during certain
methods, perhaps that can be done using a variable local to a function, rather
than a second member.

i did not look closely, so if this is really needed (if the local variable
solution doesn't work) then we can keep it. but i do think it makes the code a
bit easier to reason about if there isn't both a current and pending timing. If
we do need to keep both, then please add a comment here to explain how they
differ (in what cases will pending be different from current etc).

[Charlie Harrison, 2015/09/03 14:00:52]

Pending timing is the timing information sent from a non-committed navigation.
It isn't fully necessary to keep track of this before commit, so I think it can
be safely removed. We will always get another timing notification from the
renderer post commit.

+  PageLoadTiming pending_timing_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebContentsObserver);
+};
+
+}  // namespace page_load_metrics
+
+#endif  // COMPONENTS_PAGE_LOAD_METRICS_BROWSER_PAGE_LOAD_METRICS_WEB_CONTENTS_OBSERVER_H_