(Merging on behalf of mdw@)
Add UMA to count the number of Google CAPTCHA pages shown and solved by users.
This is intended to track down problems where a large number of mobile
users in certain countries are apparently seeing CAPTCHA pages.
(http://b2.corp.google.com/23081866 for context.)
I realize this may not be the ideal place for this code - happy for suggestions as to where else this might go.
BUG=552470
Review URL: https://codereview.chromium.org/1441393002
Cr-Commit-Position: refs/heads/master@{#360011}
(cherry picked from commit 71c7cacbdb9cd4fa161909a72a9157f574a2b775)
Review URL: https://codereview.chromium.org/1465533002 .
Cr-Commit-Position: refs/branch-heads/2564@{#58}
Cr-Branched-From: 1283eca15bd9f772387f75241576cde7bdec7f54-refs/heads/master@{#359700}
diff --git a/chrome/browser/page_load_metrics/observers/google_captcha_observer.cc b/chrome/browser/page_load_metrics/observers/google_captcha_observer.cc
new file mode 100644
index 0000000..dd746a6d
--- /dev/null
+++ b/chrome/browser/page_load_metrics/observers/google_captcha_observer.cc
@@ -0,0 +1,65 @@
+// 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.
+
+#include "chrome/browser/page_load_metrics/observers/google_captcha_observer.h"
+
+#include "base/metrics/histogram.h"
+#include "base/strings/string_util.h"
+#include "components/page_load_metrics/browser/page_load_metrics_macros.h"
+#include "components/page_load_metrics/common/page_load_timing.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+
+namespace google_captcha_observer {
+
+namespace {
+
+const char kGoogleCaptchaEvents[] = "PageLoad.Clients.GoogleCaptcha.Events";
+
+enum GoogleCaptchaEvent {
+ // A Google CAPTCHA page was shown to the user.
+ GOOGLE_CAPTCHA_SHOWN,
+ // A Google CAPTCHA page was solved by the user.
+ GOOGLE_CAPTCHA_SOLVED,
+ // Add new values before this final count.
+ GOOGLE_CAPTCHA_EVENT_BOUNDARY,
+};
+
+void RecordGoogleCaptchaEvent(GoogleCaptchaEvent event) {
+ UMA_HISTOGRAM_ENUMERATION(
+ kGoogleCaptchaEvents, event,
+ GOOGLE_CAPTCHA_EVENT_BOUNDARY);
+}
+
+} // namespace
+
+bool IsGoogleCaptcha(const GURL& url) {
+ return (base::StartsWith(url.host(), "ipv4.google.",
+ base::CompareCase::SENSITIVE)
+ || base::StartsWith(url.host(), "ipv6.google.",
+ base::CompareCase::SENSITIVE))
+ && base::StartsWith(url.path(), "/sorry", base::CompareCase::SENSITIVE);
+}
+
+GoogleCaptchaObserver::GoogleCaptchaObserver(
+ page_load_metrics::PageLoadMetricsObservable* metrics)
+ : metrics_(metrics) {}
+
+void GoogleCaptchaObserver::OnCommit(
+ content::NavigationHandle* navigation_handle) {
+ if (IsGoogleCaptcha(navigation_handle->GetURL()))
+ RecordGoogleCaptchaEvent(GOOGLE_CAPTCHA_SHOWN);
+}
+
+void GoogleCaptchaObserver::OnRedirect(
+ content::NavigationHandle* navigation_handle) {
+ if (IsGoogleCaptcha(navigation_handle->GetReferrer().url))
+ RecordGoogleCaptchaEvent(GOOGLE_CAPTCHA_SOLVED);
+}
+
+void GoogleCaptchaObserver::OnPageLoadMetricsGoingAway() {
+ metrics_->RemoveObserver(this);
+ delete this;
+}
+
+} // namespace google_captcha_observer
diff --git a/chrome/browser/page_load_metrics/observers/google_captcha_observer.h b/chrome/browser/page_load_metrics/observers/google_captcha_observer.h
new file mode 100644
index 0000000..4b0cfc52
--- /dev/null
+++ b/chrome/browser/page_load_metrics/observers/google_captcha_observer.h
@@ -0,0 +1,35 @@
+// 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 CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_GOOGLE_CAPTCHA_OBSERVER_H_
+#define CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_GOOGLE_CAPTCHA_OBSERVER_H_
+
+#include "base/macros.h"
+#include "components/page_load_metrics/browser/page_load_metrics_observer.h"
+
+namespace google_captcha_observer {
+
+// Returns true if the given URL matches a Google CAPTCHA page.
+bool IsGoogleCaptcha(const GURL& url);
+
+class GoogleCaptchaObserver
+ : public page_load_metrics::PageLoadMetricsObserver {
+ public:
+ explicit GoogleCaptchaObserver(
+ page_load_metrics::PageLoadMetricsObservable* metrics);
+
+ // page_load_metrics::PageLoadMetricsObserver implementation:
+ void OnCommit(content::NavigationHandle* navigation_handle) override;
+ void OnRedirect(content::NavigationHandle* navigation_handle) override;
+ void OnPageLoadMetricsGoingAway() override;
+
+ private:
+ page_load_metrics::PageLoadMetricsObservable* const metrics_;
+ DISALLOW_COPY_AND_ASSIGN(GoogleCaptchaObserver);
+
+};
+
+} // namespace google_captcha_observer
+
+#endif // CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_GOOGLE_CAPTCHA_OBSERVER_H_
diff --git a/chrome/browser/page_load_metrics/observers/page_load_metrics_observers_unittest.cc b/chrome/browser/page_load_metrics/observers/page_load_metrics_observers_unittest.cc
index e6716ea..da6a7a443 100644
--- a/chrome/browser/page_load_metrics/observers/page_load_metrics_observers_unittest.cc
+++ b/chrome/browser/page_load_metrics/observers/page_load_metrics_observers_unittest.cc
@@ -5,6 +5,7 @@
#include "base/memory/scoped_ptr.h"
#include "base/test/histogram_tester.h"
#include "chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h"
+#include "chrome/browser/page_load_metrics/observers/google_captcha_observer.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "components/page_load_metrics/browser/metrics_web_contents_observer.h"
#include "components/page_load_metrics/common/page_load_metrics_messages.h"
@@ -230,3 +231,23 @@
NavigateAndCommit(GURL("https://www.google.com"));
histogram_tester_.ExpectTotalCount(kHistogramNameFromGWSFirstTextPaint, 0);
}
+
+TEST_F(PageLoadMetricsObserverTest, IsGoogleCaptcha) {
+ struct {
+ std::string url;
+ bool expected;
+ } test_cases[] = {
+ {"", false},
+ {"http://www.google.com/", false},
+ {"http://www.cnn.com/", false},
+ {"http://ipv4.google.com/", false},
+ {"https://ipv4.google.com/sorry/IndexRedirect?continue=http://a", true},
+ {"https://ipv6.google.com/sorry/IndexRedirect?continue=http://a", true},
+ {"https://ipv7.google.com/sorry/IndexRedirect?continue=http://a", false},
+ };
+ for (const auto& test : test_cases) {
+ EXPECT_EQ(test.expected,
+ google_captcha_observer::IsGoogleCaptcha(GURL(test.url)))
+ << "for URL: " << test.url;
+ }
+}
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc b/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
index 1f92497..16542e41 100644
--- a/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
+++ b/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
@@ -6,7 +6,9 @@
#include "chrome/browser/browser_process.h"
#include "chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h"
+#include "chrome/browser/page_load_metrics/observers/google_captcha_observer.h"
#include "chrome/browser/prerender/prerender_contents.h"
+#include "components/page_load_metrics/browser/metrics_web_contents_observer.h"
#include "components/rappor/rappor_service.h"
#include "content/public/browser/web_contents.h"
@@ -19,6 +21,8 @@
// This is a self-destruct class, and will delete itself when triggered by
// OnPageLoadMetricsGoingAway.
metrics->AddObserver(new FromGWSPageLoadMetricsObserver(metrics));
+ metrics->AddObserver(
+ new google_captcha_observer::GoogleCaptchaObserver(metrics));
}
} // namespace
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index d126685..7a71a70 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -678,6 +678,8 @@
'browser/net/predictor_tab_helper.h',
'browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc',
'browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h',
+ 'browser/page_load_metrics/observers/google_captcha_observer.cc',
+ 'browser/page_load_metrics/observers/google_captcha_observer.h',
'browser/page_load_metrics/page_load_metrics_initialize.cc',
'browser/page_load_metrics/page_load_metrics_initialize.h',
'browser/performance_monitor/performance_monitor.cc',
diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer.cc b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
index 7e753bca..42503e2 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.cc
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
@@ -7,6 +7,7 @@
#include "base/location.h"
#include "base/logging.h"
#include "base/metrics/histogram.h"
+#include "base/metrics/user_metrics.h"
#include "components/page_load_metrics/browser/page_load_metrics_macros.h"
#include "components/page_load_metrics/common/page_load_metrics_messages.h"
#include "components/page_load_metrics/common/page_load_timing.h"
@@ -160,6 +161,11 @@
OnCommit(navigation_handle));
}
+void PageLoadTracker::Redirect(content::NavigationHandle* navigation_handle) {
+ FOR_EACH_OBSERVER(PageLoadMetricsObserver, *observers_,
+ OnRedirect(navigation_handle));
+}
+
bool PageLoadTracker::UpdateTiming(const PageLoadTiming& new_timing) {
// Throw away IPCs that are not relevant to the current navigation.
// Two timing structures cannot refer to the same navigation if they indicate
@@ -487,6 +493,16 @@
committed_load_->Commit(navigation_handle);
}
+void MetricsWebContentsObserver::DidRedirectNavigation(
+ content::NavigationHandle* navigation_handle) {
+ if (!navigation_handle->IsInMainFrame())
+ return;
+ auto it = provisional_loads_.find(navigation_handle);
+ if (it == provisional_loads_.end())
+ return;
+ it->second->Redirect(navigation_handle);
+}
+
void MetricsWebContentsObserver::WasShown() {
in_foreground_ = true;
if (committed_load_)
diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer.h b/components/page_load_metrics/browser/metrics_web_contents_observer.h
index e6fd36e..ae61708 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.h
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.h
@@ -180,6 +180,7 @@
PageLoadMetricsEmbedderInterface* embedder_interface,
base::ObserverList<PageLoadMetricsObserver, true>* observers);
~PageLoadTracker();
+ void Redirect(content::NavigationHandle* navigation_handle);
void Commit(content::NavigationHandle* navigation_handle);
void WebContentsHidden();
void WebContentsShown();
@@ -248,6 +249,8 @@
content::NavigationHandle* navigation_handle) override;
void DidFinishNavigation(
content::NavigationHandle* navigation_handle) override;
+ void DidRedirectNavigation(
+ content::NavigationHandle* navigation_handle) override;
void WasShown() override;
void WasHidden() override;
diff --git a/components/page_load_metrics/browser/page_load_metrics_observer.h b/components/page_load_metrics/browser/page_load_metrics_observer.h
index d181d14..08a35dd 100644
--- a/components/page_load_metrics/browser/page_load_metrics_observer.h
+++ b/components/page_load_metrics/browser/page_load_metrics_observer.h
@@ -40,6 +40,11 @@
public:
virtual ~PageLoadMetricsObserver() {}
+ // OnRedirect is triggered when a page load redirects to another URL.
+ // The navigation handle holds relevant data for the navigation, but will
+ // be destroyed soon after this call. Don't hold a reference to it.
+ virtual void OnRedirect(content::NavigationHandle* navigation_handle) {}
+
// OnCommit is triggered when a page load commits, i.e. when we receive the
// first data for the request. The navigation handle holds relevant data for
// the navigation, but will be destroyed soon after this call. Don't hold a
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index d76d7574..09bcfb8a 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -30806,6 +30806,12 @@
</summary>
</histogram>
+<histogram name="PageLoad.Clients.GoogleCaptcha.Events"
+ enum="GoogleCaptchaEvent">
+ <owner>mdw@chromium.org</owner>
+ <summary>Events related to Google CAPTCHA pages being seen by users.</summary>
+</histogram>
+
<histogram name="PageLoad.EventCounts" enum="PageLoadEvent">
<owner>csharrison@chromium.org</owner>
<owner>bmcquade@chromium.org</owner>
@@ -63416,6 +63422,11 @@
<int value="6" label="UNSET"/>
</enum>
+<enum name="GoogleCaptchaEvent" type="int">
+ <int value="0" label="Google CAPTCHA shown"/>
+ <int value="1" label="Google CAPTCHA solved"/>
+</enum>
+
<enum name="GoogleNowCardTypeId" type="int">
<summary>
Represents a card type ID. See cardTypeId in
