PageLoadMetrics renderer and browser implementation.

components/page_load_metrics/browser/page_load_metrics_web_contents_observer_unittest.cc

Index: components/page_load_metrics/browser/page_load_metrics_web_contents_observer_unittest.cc
diff --git a/components/page_load_metrics/browser/page_load_metrics_web_contents_observer_unittest.cc b/components/page_load_metrics/browser/page_load_metrics_web_contents_observer_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..4eae2432b75304771e004cf169b996fa2ea4e242
--- /dev/null
+++ b/components/page_load_metrics/browser/page_load_metrics_web_contents_observer_unittest.cc
@@ -0,0 +1,187 @@
+// Copyright (c) 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.
+
+#include "components/page_load_metrics/browser/page_load_metrics_web_contents_observer.h"
+
+#include "base/memory/scoped_ptr.h"
+#include "base/process/kill.h"
+#include "base/test/histogram_tester.h"
+#include "base/time/time.h"
+#include "components/page_load_metrics/common/page_load_metrics_messages.h"
+#include "content/public/browser/navigation_handle.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::Return;
+using testing::ReturnRef;
+
+namespace page_load_metrics {
+
+class MockWebContentsObserver : public WebContentsObserver {

[Bryan McQuade, 2015/09/04 20:53:18]

this is a case where we should be explicit which WebContentsObserver
impementation we're using, just to ease readability of the code:

class MockWebContentsObserver : public page_load_metrics::WebContentsObserver {

if this doesn't work you can do:

class MockWebContentsObserver : public ::page_load_metrics::WebContentsObserver
{

which should work.

+ public:
+  MockWebContentsObserver()
+      : WebContentsObserver(nullptr) {}
+  bool OnMessageReceived(const IPC::Message& message,

[Bryan McQuade, 2015/09/04 20:53:18]

I wonder, if we just add a TestBrowserThreadBundle instance as a member of the
WebContentsObserverTest blass below, can we remove this override? Perhaps that
class magically sets up the current thread as the main thread? Can you test that
out? Would be nice if we could avoid overriding this.

[Charlie Harrison, 2015/09/08 23:05:15]

Yeah this works.

+                         content::RenderFrameHost* render_frame_host) {
+    return HandleMessageReceived(message, render_frame_host);
+  }
+  MOCK_METHOD0(GetLastCommittedURL, const GURL&());
+};
+
+class MockNavigationHandle : public content::NavigationHandle {

[Bryan McQuade, 2015/09/04 20:53:18]

this is a really nice mock class to have. one unfortunate drawback is that every
time someone adds a new pure virtual method to NavigationHandle, our code will
have to be updated as well, which is undesirable. I think this class should
actually be moved to content, under content/public/test/, similar to
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/tes....
That way, every time someone adds a new method to NavigationHandle, they can
also update Mocknavigationhandle to include the additional
MOCK_CONST_METHOD0(...) for those methods, and existing tests will continue to
compile.

[Charlie Harrison, 2015/09/08 23:05:15]

Yep, this is the right way to go.

+ public:
+  // Sane defaults for testing
+  MockNavigationHandle(GURL& url) {

[Bryan McQuade, 2015/09/04 20:53:18]

when moved to content, this constructor should be marked as explicit, and 'GURL&
url' should probably be changed to 'GURL url' - since in the general case we
probably cannot guarantee that the passed in url will continue to exist for the
lifetime of the mock, so we should default to making a copy, which is fine since
tests aren't perf critical.

[Charlie Harrison, 2015/09/09 17:32:03]

Done.

+    EXPECT_CALL(*this, GetURL()).WillRepeatedly(

[Bryan McQuade, 2015/09/04 20:53:18]

as part of moving this over to content/, I'd make these defaults more neutral:
1. switch to ON_CALL rather than EXPECT_CALL: EXPECT_CALL means you really
expect the method to be called, whereas ON_CALL is used for optional defaults
2. switch from WillRepeatedly to WillByDefault
3. make all boolean methods return false by default (this is the most consistent
and predictable default behavior).
4. add a comment in the MockNavigationHandle header explaining the default
behavior of newly instantiated MockNavigationHandle instances.

then in your code, you can have a
static void SetExpectations(MockNavigationHandle* handle) {
  EXPECT_CALL(*handle, ...)
  to set up any custom expectations that differ from the default expecatations
set up in content.
}

[Charlie Harrison, 2015/09/09 17:32:03]

Done.

+        ReturnRef(url));
+    EXPECT_CALL(*this, IsInMainFrame()).WillRepeatedly(
+        Return(true));
+    EXPECT_CALL(*this, IsSamePage()).WillRepeatedly(
+        Return(false));
+    EXPECT_CALL(*this, HasCommittedDocument()).WillRepeatedly(
+        Return(true));
+  }
+  MOCK_CONST_METHOD0(GetURL, const GURL&());
+  MOCK_CONST_METHOD0(IsInMainFrame, bool());
+  MOCK_CONST_METHOD0(IsSamePage, bool());
+  MOCK_CONST_METHOD0(HasCommittedDocument, bool());
+  MOCK_CONST_METHOD0(HasCommittedErrorPage, bool());
+  MOCK_CONST_METHOD0(GetNetErrorCode, net::Error());
+};
+
+class WebContentsObserverTest : public testing::Test {
+ public:
+  const char* kHistogramNameFirstLayout = "PageLoad.Timing.FirstLayout";
+  const char* kHistogramNameDomContent =
+      "PageLoad.Timing.DOMContentLoadedEventFired";
+  const char* kHistogramNameLoad = "PageLoad.Timing.LoadEventFired";
+
+  WebContentsObserverTest() : observer_() {}
+
+ protected:
+  base::HistogramTester histogram_tester_;
+  MockWebContentsObserver observer_;
+};
+
+TEST_F(WebContentsObserverTest, NoMetrics) {

[Bryan McQuade, 2015/09/04 20:53:18]

we may need to use a more specific classname just for the tests, since the
output of the test on the console will show only the class name IIRC and it'd be
good for the class name to uniquely identify the code being tested. take a look
at console output and see.

+  histogram_tester_.ExpectTotalCount(kHistogramNameDomContent, 0);
+  histogram_tester_.ExpectTotalCount(kHistogramNameLoad, 0);
+  histogram_tester_.ExpectTotalCount(kHistogramNameFirstLayout, 0);
+}
+
+TEST_F(WebContentsObserverTest, NotInMainFrame) {
+  base::TimeDelta first_layout = base::TimeDelta::FromMilliseconds(1);
+
+  PageLoadTiming timing;
+  timing.first_layout = first_layout;
+  GURL url("https://google.com");
+  EXPECT_CALL(observer_, GetLastCommittedURL()).WillRepeatedly(
+      ReturnRef(url));
+  MockNavigationHandle test_handle(url);
+  EXPECT_CALL(test_handle, IsInMainFrame()).WillRepeatedly(
+      Return(false));
+
+  observer_.DidCommitNavigation(&test_handle);
+  observer_.OnMessageReceived(
+      PageLoadMetricsMsg_TimingUpdated(observer_.routing_id(), timing),
+      nullptr);
+  // Navigate again to force histogram recording.
+  observer_.DidCommitNavigation(&test_handle);
+
+  histogram_tester_.ExpectTotalCount(kHistogramNameDomContent, 0);
+  histogram_tester_.ExpectTotalCount(kHistogramNameLoad, 0);
+  histogram_tester_.ExpectTotalCount(kHistogramNameFirstLayout, 0);
+}
+
+TEST_F(WebContentsObserverTest, SamePage) {
+  base::TimeDelta first_layout = base::TimeDelta::FromMilliseconds(1);
+
+  PageLoadTiming timing;
+  timing.first_layout = first_layout;
+  GURL url("https://google.com");
+  EXPECT_CALL(observer_, GetLastCommittedURL()).WillRepeatedly(
+      ReturnRef(url));
+  MockNavigationHandle test_handle(url);
+  EXPECT_CALL(test_handle, IsSamePage()).WillRepeatedly(
+      Return(true));
+
+  observer_.DidCommitNavigation(&test_handle);
+  observer_.OnMessageReceived(
+      PageLoadMetricsMsg_TimingUpdated(observer_.routing_id(), timing),
+      nullptr);
+  // Navigate again to force histogram recording.
+  observer_.DidCommitNavigation(&test_handle);
+
+  histogram_tester_.ExpectTotalCount(kHistogramNameDomContent, 0);
+  histogram_tester_.ExpectTotalCount(kHistogramNameLoad, 0);
+  histogram_tester_.ExpectTotalCount(kHistogramNameFirstLayout, 0);
+}
+
+TEST_F(WebContentsObserverTest, SingleMetricAfterCommit) {
+  base::TimeDelta first_layout = base::TimeDelta::FromMilliseconds(1);
+
+  PageLoadTiming timing;
+  timing.first_layout = first_layout;
+  GURL url("https://google.com");
+  EXPECT_CALL(observer_, GetLastCommittedURL()).WillRepeatedly(
+      ReturnRef(url));
+  MockNavigationHandle test_handle(url);
+  observer_.DidCommitNavigation(&test_handle);
+  observer_.OnMessageReceived(
+      PageLoadMetricsMsg_TimingUpdated(observer_.routing_id(), timing),
+      nullptr);
+
+  // Navigate again to force histogram recording.
+  observer_.DidCommitNavigation(&test_handle);
+
+  histogram_tester_.ExpectTotalCount(kHistogramNameDomContent, 0);
+  histogram_tester_.ExpectTotalCount(kHistogramNameLoad, 0);
+  histogram_tester_.ExpectTotalCount(kHistogramNameFirstLayout, 1);
+  histogram_tester_.ExpectBucketCount(kHistogramNameFirstLayout,
+                                      first_layout.InMilliseconds(), 1);
+}
+
+TEST_F(WebContentsObserverTest, MultipleMetricsAfterCommits) {
+  base::TimeDelta first_layout_1 = base::TimeDelta::FromMilliseconds(1);
+  base::TimeDelta first_layout_2 = base::TimeDelta::FromMilliseconds(20);
+  base::TimeDelta dom_content = base::TimeDelta::FromMilliseconds(40);
+  base::TimeDelta load = base::TimeDelta::FromMilliseconds(100);
+
+  PageLoadTiming timing;
+  timing.first_layout = first_layout_1;
+  timing.dom_content_loaded_event_start = dom_content;
+  timing.load_event_start = load;
+  GURL url("https://google.com");
+  EXPECT_CALL(observer_, GetLastCommittedURL()).WillRepeatedly(
+      ReturnRef(url));
+  MockNavigationHandle test_handle(url);
+  observer_.DidCommitNavigation(&test_handle);
+  observer_.OnMessageReceived(
+      PageLoadMetricsMsg_TimingUpdated(observer_.routing_id(), timing),
+      nullptr);
+
+  observer_.DidCommitNavigation(&test_handle);
+  PageLoadTiming timing2;
+  timing2.first_layout = first_layout_2;
+  observer_.OnMessageReceived(
+      PageLoadMetricsMsg_TimingUpdated(observer_.routing_id(), timing2),
+      nullptr);
+  observer_.DidCommitNavigation(&test_handle);
+
+  histogram_tester_.ExpectTotalCount(kHistogramNameFirstLayout, 2);
+  histogram_tester_.ExpectBucketCount(kHistogramNameFirstLayout,
+                                      first_layout_1.InMilliseconds(), 1);
+  histogram_tester_.ExpectBucketCount(kHistogramNameFirstLayout,
+                                      first_layout_2.InMilliseconds(), 1);
+
+  histogram_tester_.ExpectTotalCount(kHistogramNameDomContent, 1);
+  histogram_tester_.ExpectBucketCount(kHistogramNameDomContent,
+                                      dom_content.InMilliseconds(), 1);
+
+  histogram_tester_.ExpectTotalCount(kHistogramNameLoad, 1);
+  histogram_tester_.ExpectBucketCount(kHistogramNameLoad, load.InMilliseconds(),
+                                      1);
+}
+
+}  // namespace page_load_metrics