Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(435)

Unified Diff: components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc

Issue 2132603002: [page_load_metrics] Add a NavigationThrottle for richer abort metrics (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Attach the throttle first so it gets all notifications before any DEFERs Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc
diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc b/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc
index 3c773cb94cf08942ded81dd8f04a8567d1b827a9..1bf8b030cadff52f123205147e2f320cba052dcf 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc
@@ -12,6 +12,7 @@
#include "base/process/kill.h"
#include "base/test/histogram_tester.h"
#include "base/time/time.h"
+#include "components/page_load_metrics/browser/metrics_navigation_throttle.h"
#include "components/page_load_metrics/browser/page_load_metrics_observer.h"
#include "components/page_load_metrics/common/page_load_metrics_messages.h"
#include "content/public/browser/navigation_handle.h"
@@ -103,6 +104,20 @@ class TestPageLoadMetricsEmbedderInterface
} // namespace
+class AddThrottleWebContentsObserver : public content::WebContentsObserver {
+ public:
+ AddThrottleWebContentsObserver(content::WebContents* contents)
+ : content::WebContentsObserver(contents) {}
+
+ void DidStartNavigation(content::NavigationHandle* handle) override {
+ handle->RegisterThrottleForTesting(
+ MetricsNavigationThrottle::Create(handle));
+ }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(AddThrottleWebContentsObserver);
+};
+
class MetricsWebContentsObserverTest
: public content::RenderViewHostTestHarness {
public:
@@ -127,9 +142,16 @@ class MetricsWebContentsObserverTest
void AttachObserver() {
embedder_interface_ = new TestPageLoadMetricsEmbedderInterface();
- observer_.reset(new MetricsWebContentsObserver(
- web_contents(), base::WrapUnique(embedder_interface_)));
+ // Owned by the web_contents. Tests must be careful not to call
+ // SimulateTimingUpdate after they call DeleteContents() without also
+ // calling AttachObserver() again. Otherwise they will use-after-free the
+ // observer_.
+ observer_ = MetricsWebContentsObserver::CreateForWebContents(
+ web_contents(), base::WrapUnique(embedder_interface_));
observer_->WasShown();
+
+ add_throttling_observer_.reset(
+ new AddThrottleWebContentsObserver(web_contents()));
}
void CheckErrorEvent(InternalErrorLoadEvent error, int count) {
@@ -168,7 +190,8 @@ class MetricsWebContentsObserverTest
protected:
base::HistogramTester histogram_tester_;
TestPageLoadMetricsEmbedderInterface* embedder_interface_;
- std::unique_ptr<MetricsWebContentsObserver> observer_;
+ MetricsWebContentsObserver* observer_;
+ std::unique_ptr<AddThrottleWebContentsObserver> add_throttling_observer_;
private:
int num_errors_;
@@ -362,8 +385,12 @@ TEST_F(MetricsWebContentsObserverTest, BadIPC) {
}
TEST_F(MetricsWebContentsObserverTest, ObservePartialNavigation) {
- // Delete the observer for this test, add it once the navigation has started.
- observer_.reset();
+ // Reset the state of the tests, and attach the MetricsWebContentsObserver in
+ // the middle of a navigation. This tests that the class is robust to only
+ // observing some of a navigation.
+ DeleteContents();
+ SetContents(CreateTestWebContents());
+
PageLoadTiming timing;
timing.navigation_start = base::Time::FromDoubleT(10);

Powered by Google App Engine
This is Rietveld 408576698