Add a histogram (Net.PacResultForStrippedUrl) that estimates how often PAC scripts depend on the UR…

net/proxy/proxy_resolver_v8_tracing_unittest.cc

Index: net/proxy/proxy_resolver_v8_tracing_unittest.cc
diff --git a/net/proxy/proxy_resolver_v8_tracing_unittest.cc b/net/proxy/proxy_resolver_v8_tracing_unittest.cc
index 888dd18af57d30ddf26219bf0aa4b8166c4a607e..4d2b29660f2e0e684cf91ee508ce52d066f3889e 100644
--- a/net/proxy/proxy_resolver_v8_tracing_unittest.cc
+++ b/net/proxy/proxy_resolver_v8_tracing_unittest.cc
@@ -12,6 +12,7 @@
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/test/histogram_tester.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_checker.h"
 #include "base/values.h"
@@ -37,6 +38,23 @@ class ProxyResolverV8TracingTest : public testing::Test {
     // spilling into the next test's execution.
     base::RunLoop().RunUntilIdle();
   }
+
+ protected:
+  // TODO(eroman): Remove when done gathering data for crbug.com/593759
+  void ExpectHistogramBucketCount(PacResultForStrippedUrl bucket,
+                                  size_t expected_total) {
+    histograms_.ExpectUniqueSample(kHistogramPacResultForStrippedUrl,
+                                   static_cast<int>(bucket), expected_total);
+  }
+
+  // TODO(eroman): Remove when done gathering data for crbug.com/593759
+  void ExpectHistogramTotal(size_t expected_total) {
+    histograms_.ExpectTotalCount(kHistogramPacResultForStrippedUrl,
+                                 expected_total);
+  }
+
+ private:
+  base::HistogramTester histograms_;
 };
 
 scoped_refptr<ProxyResolverScriptData> LoadScriptData(const char* filename) {
@@ -160,12 +178,15 @@ TEST_F(ProxyResolverV8TracingTest, Simple) {
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("http://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
                            callback.callback(), NULL,
                            mock_bindings.CreateBindings());
 
   EXPECT_EQ(OK, callback.WaitForResult());
 
+  ExpectHistogramBucketCount(PacResultForStrippedUrl::SUCCESS, 1);
+  ExpectHistogramTotal(1);
+
   EXPECT_EQ("foo:99", proxy_info.proxy_server().ToURI());
 
   EXPECT_EQ(0u, host_resolver.num_resolve());
@@ -175,6 +196,36 @@ TEST_F(ProxyResolverV8TracingTest, Simple) {
   EXPECT_TRUE(mock_bindings.GetErrors().empty());
 }
 
+TEST_F(ProxyResolverV8TracingTest, AlertUrl) {
+  MockCachingHostResolver host_resolver;
+  MockBindings mock_bindings(&host_resolver);
+
+  scoped_ptr<ProxyResolverV8Tracing> resolver =
+      CreateResolver(mock_bindings.CreateBindings(), "alert_url.js");
+
+  TestCompletionCallback callback;
+  ProxyInfo proxy_info;
+
+  resolver->GetProxyForURL(GURL("https://foo/path"), &proxy_info,
+                           callback.callback(), NULL,
+                           mock_bindings.CreateBindings());
+
+  EXPECT_EQ(OK, callback.WaitForResult());
+
+  ExpectHistogramBucketCount(PacResultForStrippedUrl::SUCCESS_DIFFERENT_ALERTS,
+                             1);
+  ExpectHistogramTotal(1);
+
+  EXPECT_EQ("foobar:99", proxy_info.proxy_server().ToURI());
+
+  EXPECT_EQ(0u, host_resolver.num_resolve());
+
+  // There was 1 alerts and no errors.
+  EXPECT_EQ(1u, mock_bindings.GetAlerts().size());
+  EXPECT_EQ("https://foo/path", mock_bindings.GetAlerts()[0]);
+  EXPECT_TRUE(mock_bindings.GetErrors().empty());
+}
+
 TEST_F(ProxyResolverV8TracingTest, JavascriptError) {
   MockCachingHostResolver host_resolver;
   MockBindings mock_bindings(&host_resolver);
@@ -185,12 +236,14 @@ TEST_F(ProxyResolverV8TracingTest, JavascriptError) {
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("http://throw-an-error/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("https://throw-an-error/"), &proxy_info,
                            callback.callback(), NULL,
                            mock_bindings.CreateBindings());
 
   EXPECT_EQ(ERR_PAC_SCRIPT_FAILED, callback.WaitForResult());
 
+  ExpectHistogramTotal(0);
+
   EXPECT_EQ(0u, host_resolver.num_resolve());
 
   // Check the output -- there was 1 alert and 1 javascript error.
@@ -212,12 +265,16 @@ TEST_F(ProxyResolverV8TracingTest, TooManyAlerts) {
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("http://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
                            callback.callback(), NULL,
                            mock_bindings.CreateBindings());
 
   EXPECT_EQ(OK, callback.WaitForResult());
 
+  ExpectHistogramBucketCount(
+      PacResultForStrippedUrl::SKIPPED_FALLBACK_BLOCKING_DNS, 1);
+  ExpectHistogramTotal(1);
+
   // Iteration1 does a DNS resolve
   // Iteration2 exceeds the alert buffer
   // Iteration3 runs in blocking mode and completes
@@ -248,12 +305,16 @@ TEST_F(ProxyResolverV8TracingTest, TooManyEmptyAlerts) {
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("http://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
                            callback.callback(), NULL,
                            mock_bindings.CreateBindings());
 
   EXPECT_EQ(OK, callback.WaitForResult());
 
+  ExpectHistogramBucketCount(
+      PacResultForStrippedUrl::SKIPPED_FALLBACK_BLOCKING_DNS, 1);
+  ExpectHistogramTotal(1);
+
   EXPECT_EQ("foo:3", proxy_info.proxy_server().ToURI());
 
   EXPECT_EQ(1u, host_resolver.num_resolve());
@@ -294,12 +355,15 @@ TEST_F(ProxyResolverV8TracingTest, Dns) {
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("http://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
                            callback.callback(), NULL,
                            mock_bindings.CreateBindings());
 
   EXPECT_EQ(OK, callback.WaitForResult());
 
+  ExpectHistogramBucketCount(PacResultForStrippedUrl::SUCCESS, 1);
+  ExpectHistogramTotal(1);
+
   // The test does 13 DNS resolution, however only 7 of them are unique.
   EXPECT_EQ(7u, host_resolver.num_resolve());
 
@@ -346,12 +410,18 @@ TEST_F(ProxyResolverV8TracingTest, DnsChecksCache) {
   TestCompletionCallback callback2;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("http://foopy/req1"), &proxy_info,
+  resolver->GetProxyForURL(GURL("https://foopy/req1"), &proxy_info,
                            callback1.callback(), NULL,
                            mock_bindings.CreateBindings());
 
   EXPECT_EQ(OK, callback1.WaitForResult());
 
+  // This fails because executing FindProxyForURL() PAC script modifies global
+  // state each time, changing the result that is returned.
+  ExpectHistogramBucketCount(PacResultForStrippedUrl::FAIL_DIFFERENT_PROXY_LIST,
+                             1);
+  ExpectHistogramTotal(1);
+
   // The test does 2 DNS resolutions.
   EXPECT_EQ(2u, host_resolver.num_resolve());
 
@@ -364,10 +434,19 @@ TEST_F(ProxyResolverV8TracingTest, DnsChecksCache) {
 
   EXPECT_EQ(OK, callback2.WaitForResult());
 
+  // The histograms are unchanged because the second invocation is for an
+  // http:// URL.
+  ExpectHistogramBucketCount(PacResultForStrippedUrl::FAIL_DIFFERENT_PROXY_LIST,
+                             1);
+  ExpectHistogramTotal(1);
+
   EXPECT_EQ(4u, host_resolver.num_resolve());
 
   // This time no restarts were required, so g_iteration incremented by 1.
-  EXPECT_EQ("166.155.144.11:4", proxy_info.proxy_server().ToURI());
+  // TODO(eroman): Additionally the counter was incremented once by the
+  // diagnostics code that ran FindProxyForURL() with a stripped URL
+  // (should really be :4 and not :5).
+  EXPECT_EQ("166.155.144.11:5", proxy_info.proxy_server().ToURI());
 
   // There were no alerts or errors.
   EXPECT_TRUE(mock_bindings.GetAlerts().empty());
@@ -391,11 +470,15 @@ TEST_F(ProxyResolverV8TracingTest, FallBackToSynchronous1) {
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("http://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
                            callback.callback(), NULL,
                            mock_bindings.CreateBindings());
   EXPECT_EQ(OK, callback.WaitForResult());
 
+  ExpectHistogramBucketCount(
+      PacResultForStrippedUrl::SKIPPED_FALLBACK_BLOCKING_DNS, 1);
+  ExpectHistogramTotal(1);
+
   // The script itself only does 2 DNS resolves per execution, however it
   // constructs the hostname using a global counter which changes on each
   // invocation.
@@ -430,11 +513,15 @@ TEST_F(ProxyResolverV8TracingTest, FallBackToSynchronous2) {
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("http://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
                            callback.callback(), NULL,
                            mock_bindings.CreateBindings());
   EXPECT_EQ(OK, callback.WaitForResult());
 
+  ExpectHistogramBucketCount(
+      PacResultForStrippedUrl::SKIPPED_FALLBACK_BLOCKING_DNS, 1);
+  ExpectHistogramTotal(1);
+
   EXPECT_EQ(3u, host_resolver.num_resolve());
 
   EXPECT_EQ("166.155.144.44:100", proxy_info.proxy_server().ToURI());
@@ -466,6 +553,9 @@ TEST_F(ProxyResolverV8TracingTest, InfiniteDNSSequence) {
                            mock_bindings.CreateBindings());
   EXPECT_EQ(OK, callback.WaitForResult());
 
+  // Was not called because this is an http:// URL.
+  ExpectHistogramTotal(0);
+
   EXPECT_EQ(20u, host_resolver.num_resolve());
 
   EXPECT_EQ(
@@ -506,6 +596,9 @@ TEST_F(ProxyResolverV8TracingTest, InfiniteDNSSequence2) {
                            mock_bindings.CreateBindings());
   EXPECT_EQ(OK, callback.WaitForResult());
 
+  // Was not called because this is an http:// URL.
+  ExpectHistogramTotal(0);
+
   EXPECT_EQ(20u, host_resolver.num_resolve());
 
   EXPECT_EQ("null21:34", proxy_info.proxy_server().ToURI());
@@ -518,6 +611,94 @@ TEST_F(ProxyResolverV8TracingTest, InfiniteDNSSequence2) {
   EXPECT_EQ("iteration: 21", mock_bindings.GetAlerts()[0]);
 }
 
+TEST_F(ProxyResolverV8TracingTest, DifferentResultBasedOnUrl) {
+  MockCachingHostResolver host_resolver;
+  MockBindings mock_bindings(&host_resolver);
+
+  scoped_ptr<ProxyResolverV8Tracing> resolver =
+      CreateResolver(mock_bindings.CreateBindings(), "return_url_as_proxy.js");
+
+  TestCompletionCallback callback;
+  ProxyInfo proxy_info;
+
+  resolver->GetProxyForURL(GURL("https://foo/path1"), &proxy_info,
+                           callback.callback(), NULL,
+                           mock_bindings.CreateBindings());
+
+  EXPECT_EQ(OK, callback.WaitForResult());
+
+  ExpectHistogramTotal(1);
+  ExpectHistogramBucketCount(PacResultForStrippedUrl::FAIL_DIFFERENT_PROXY_LIST,
+                             1);
+
+  EXPECT_EQ("httpsx3Ax2Fx2Ffoox2Fpath1:99", proxy_info.proxy_server().ToURI());
+
+  EXPECT_EQ(0u, host_resolver.num_resolve());
+
+  // There were no alerts or errors.
+  EXPECT_TRUE(mock_bindings.GetAlerts().empty());
+  EXPECT_TRUE(mock_bindings.GetErrors().empty());
+}
+
+TEST_F(ProxyResolverV8TracingTest, ErrorDependingOnUrl) {
+  MockCachingHostResolver host_resolver;
+  MockBindings mock_bindings(&host_resolver);
+
+  scoped_ptr<ProxyResolverV8Tracing> resolver = CreateResolver(
+      mock_bindings.CreateBindings(), "error_depending_on_url.js");
+
+  TestCompletionCallback callback;
+  ProxyInfo proxy_info;
+
+  resolver->GetProxyForURL(GURL("https://foo/DontThrowError"), &proxy_info,
+                           callback.callback(), NULL,
+                           mock_bindings.CreateBindings());
+
+  EXPECT_EQ(OK, callback.WaitForResult());
+
+  ExpectHistogramTotal(1);
+  ExpectHistogramBucketCount(PacResultForStrippedUrl::FAIL_ERROR, 1);
+
+  EXPECT_EQ("foopy:42", proxy_info.proxy_server().ToURI());
+
+  EXPECT_EQ(0u, host_resolver.num_resolve());
+
+  // There were no alerts or errors.
+  EXPECT_TRUE(mock_bindings.GetAlerts().empty());
+  EXPECT_TRUE(mock_bindings.GetErrors().empty());
+}
+
+TEST_F(ProxyResolverV8TracingTest, DnsDependingOnUrl) {
+  MockCachingHostResolver host_resolver;
+  MockBindings mock_bindings(&host_resolver);
+
+  host_resolver.rules()->AddRule("host", "166.155.144.55");
+
+  // Catch-all that will be used for myIpAddress().
+  host_resolver.rules()->AddRule("*", "133.122.100.200");
+
+  scoped_ptr<ProxyResolverV8Tracing> resolver =
+      CreateResolver(mock_bindings.CreateBindings(), "dns_depending_on_url.js");
+
+  TestCompletionCallback callback;
+  ProxyInfo proxy_info;
+
+  resolver->GetProxyForURL(GURL("https://foo/UseMyIpAddress"), &proxy_info,
+                           callback.callback(), NULL,
+                           mock_bindings.CreateBindings());
+
+  EXPECT_EQ(OK, callback.WaitForResult());
+
+  ExpectHistogramBucketCount(PacResultForStrippedUrl::FAIL_ABANDONED, 1);
+  ExpectHistogramTotal(1);
+
+  EXPECT_EQ("foopy:47", proxy_info.proxy_server().ToURI());
+
+  // No errors.
+  EXPECT_TRUE(mock_bindings.GetErrors().empty());
+  ASSERT_EQ(0u, mock_bindings.GetAlerts().size());
+}
+
 void DnsDuringInitHelper(bool synchronous_host_resolver) {
   MockCachingHostResolver host_resolver;
   MockBindings mock_bindings(&host_resolver);