Revert "Add a histogram (Net.PacResultForStrippedUrl) that estimates how often PAC scripts depend o…

net/proxy/proxy_resolver_v8_tracing_unittest.cc

 // Copyright (c) 2013 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 "net/proxy/proxy_resolver_v8_tracing.h"
 
 #include <string>
 #include <utility>
 
 #include "base/files/file_util.h"
 #include "base/path_service.h"
 #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"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
 #include "net/dns/host_cache.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/log/net_log.h"
 #include "net/proxy/proxy_info.h"
 #include "net/test/event_waiter.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 namespace net {
 
 namespace {
 
 class ProxyResolverV8TracingTest : public testing::Test {
  public:
   void TearDown() override {
     // Drain any pending messages, which may be left over from cancellation.
     // This way they get reliably run as part of the current test, rather than
     // 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) {
   base::FilePath path;
   PathService::Get(base::DIR_SOURCE_ROOT, &path);
   path = path.AppendASCII("net");
   path = path.AppendASCII("data");
   path = path.AppendASCII("proxy_resolver_v8_tracing_unittest");
   path = path.AppendASCII(filename);
 
@@ skipping 103 matching lines @@
 TEST_F(ProxyResolverV8TracingTest, Simple) {
   MockCachingHostResolver host_resolver;
   MockBindings mock_bindings(&host_resolver);
 
   scoped_ptr<ProxyResolverV8Tracing> resolver =
       CreateResolver(mock_bindings.CreateBindings(), "simple.js");
 
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("http://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());
 
   // There were no alerts or errors.
   EXPECT_TRUE(mock_bindings.GetAlerts().empty());
   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);
 
   scoped_ptr<ProxyResolverV8Tracing> resolver =
       CreateResolver(mock_bindings.CreateBindings(), "error.js");
 
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("https://throw-an-error/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("http://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.
   ASSERT_EQ(1u, mock_bindings.GetAlerts().size());
   EXPECT_EQ("Prepare to DIE!", mock_bindings.GetAlerts()[0]);
   ASSERT_EQ(1u, mock_bindings.GetErrors().size());
   EXPECT_EQ(5, mock_bindings.GetErrors()[0].first);
   EXPECT_EQ("Uncaught TypeError: Cannot read property 'split' of null",
             mock_bindings.GetErrors()[0].second);
 }
 
 TEST_F(ProxyResolverV8TracingTest, TooManyAlerts) {
   MockCachingHostResolver host_resolver;
   MockBindings mock_bindings(&host_resolver);
 
   scoped_ptr<ProxyResolverV8Tracing> resolver =
       CreateResolver(mock_bindings.CreateBindings(), "too_many_alerts.js");
 
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("http://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
   EXPECT_EQ("foo:3", proxy_info.proxy_server().ToURI());
 
   EXPECT_EQ(1u, host_resolver.num_resolve());
 
   // No errors.
   EXPECT_TRUE(mock_bindings.GetErrors().empty());
 
@@ skipping 10 matching lines @@
 TEST_F(ProxyResolverV8TracingTest, TooManyEmptyAlerts) {
   MockCachingHostResolver host_resolver;
   MockBindings mock_bindings(&host_resolver);
 
   scoped_ptr<ProxyResolverV8Tracing> resolver = CreateResolver(
       mock_bindings.CreateBindings(), "too_many_empty_alerts.js");
 
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("http://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());
 
   // No errors.
   EXPECT_TRUE(mock_bindings.GetErrors().empty());
 
   // Check the alerts -- the script generated 1000 alerts.
   std::vector<std::string> alerts = mock_bindings.GetAlerts();
   ASSERT_EQ(1000u, alerts.size());
@@ skipping 20 matching lines @@
   host_resolver.rules()->AddRuleForAddressFamily(
       "*", ADDRESS_FAMILY_IPV4, "122.133.144.155");
   host_resolver.rules()->AddRule("*", "133.122.100.200");
 
   scoped_ptr<ProxyResolverV8Tracing> resolver =
       CreateResolver(mock_bindings.CreateBindings(), "dns.js");
 
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("http://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());
 
   const char* kExpectedResult =
     "122.133.144.155-"  // myIpAddress()
     "null-"  // dnsResolve('')
     "__1_192.168.1.1-"  // dnsResolveEx('host1')
     "null-"  // dnsResolve('host2')
     "166.155.144.33-"  // dnsResolve('host3')
     "122.133.144.155-"  // myIpAddress()
@@ skipping 26 matching lines @@
   host_resolver.rules()->AddRule("foopy", "166.155.144.11");
   host_resolver.rules()->AddRule("*", "122.133.144.155");
 
   scoped_ptr<ProxyResolverV8Tracing> resolver =
       CreateResolver(mock_bindings.CreateBindings(), "simple_dns.js");
 
   TestCompletionCallback callback1;
   TestCompletionCallback callback2;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("https://foopy/req1"), &proxy_info,
+  resolver->GetProxyForURL(GURL("http://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());
 
   // The first request took 2 restarts, hence on g_iteration=3.
   EXPECT_EQ("166.155.144.11:3", proxy_info.proxy_server().ToURI());
 
   resolver->GetProxyForURL(GURL("http://foopy/req2"), &proxy_info,
                            callback2.callback(), NULL,
                            mock_bindings.CreateBindings());
 
   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.
-  // 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());
+  EXPECT_EQ("166.155.144.11:4", proxy_info.proxy_server().ToURI());
 
   // There were no alerts or errors.
   EXPECT_TRUE(mock_bindings.GetAlerts().empty());
   EXPECT_TRUE(mock_bindings.GetErrors().empty());
 }
 
 // This test runs a weird PAC script that was designed to defeat the DNS tracing
 // optimization. The proxy resolver should detect the inconsistency and
 // fall-back to synchronous mode execution.
 TEST_F(ProxyResolverV8TracingTest, FallBackToSynchronous1) {
   MockCachingHostResolver host_resolver;
   MockBindings mock_bindings(&host_resolver);
 
   host_resolver.rules()->AddRule("host1", "166.155.144.11");
   host_resolver.rules()->AddRule("crazy4", "133.199.111.4");
   host_resolver.rules()->AddRule("*", "122.133.144.155");
 
   scoped_ptr<ProxyResolverV8Tracing> resolver =
       CreateResolver(mock_bindings.CreateBindings(), "global_sideffects1.js");
 
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("http://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.
   EXPECT_EQ(3u, host_resolver.num_resolve());
 
   EXPECT_EQ("166.155.144.11-133.199.111.4:100",
             proxy_info.proxy_server().ToURI());
 
   // No errors.
   EXPECT_TRUE(mock_bindings.GetErrors().empty());
@@ skipping 14 matching lines @@
   host_resolver.rules()->AddRule("host3", "166.155.144.33");
   host_resolver.rules()->AddRule("host4", "166.155.144.44");
   host_resolver.rules()->AddRule("*", "122.133.144.155");
 
   scoped_ptr<ProxyResolverV8Tracing> resolver =
       CreateResolver(mock_bindings.CreateBindings(), "global_sideffects2.js");
 
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
-  resolver->GetProxyForURL(GURL("https://foo/"), &proxy_info,
+  resolver->GetProxyForURL(GURL("http://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());
 
   // There were no alerts or errors.
   EXPECT_TRUE(mock_bindings.GetAlerts().empty());
   EXPECT_TRUE(mock_bindings.GetErrors().empty());
 }
 
 // This test runs a weird PAC script that yields a never ending sequence
@@ skipping 11 matching lines @@
       CreateResolver(mock_bindings.CreateBindings(), "global_sideffects3.js");
 
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
   resolver->GetProxyForURL(GURL("http://foo/"), &proxy_info,
                            callback.callback(), NULL,
                            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(
       "166.155.144.11-166.155.144.11-166.155.144.11-166.155.144.11-"
       "166.155.144.11-166.155.144.11-166.155.144.11-166.155.144.11-"
       "166.155.144.11-166.155.144.11-166.155.144.11-166.155.144.11-"
       "166.155.144.11-166.155.144.11-166.155.144.11-166.155.144.11-"
       "166.155.144.11-166.155.144.11-166.155.144.11-166.155.144.11-"
       "null:21", proxy_info.proxy_server().ToURI());
 
@@ skipping 20 matching lines @@
       CreateResolver(mock_bindings.CreateBindings(), "global_sideffects4.js");
 
   TestCompletionCallback callback;
   ProxyInfo proxy_info;
 
   resolver->GetProxyForURL(GURL("http://foo/"), &proxy_info,
                            callback.callback(), NULL,
                            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());
 
   // No errors.
   EXPECT_TRUE(mock_bindings.GetErrors().empty());
 
   // 1 alert.
   EXPECT_EQ(1u, mock_bindings.GetAlerts().size());
   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);
   host_resolver.set_synchronous_mode(synchronous_host_resolver);
 
   host_resolver.rules()->AddRule("host1", "91.13.12.1");
   host_resolver.rules()->AddRule("host2", "91.13.12.2");
 
   scoped_ptr<ProxyResolverV8Tracing> resolver =
       CreateResolver(mock_bindings.CreateBindings(), "dns_during_init.js");
@@ skipping 484 matching lines @@
                 proxy_uri.substr(0, proxy_uri.find(':') + 1));
     } else {
       NOTREACHED();
     }
   }
 }
 
 }  // namespace
 
 }  // namespace net