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

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: net/proxy/proxy_resolver_v8_tracing_unittest.cc

Issue 1944983002: Revert "Add a histogram (Net.PacResultForStrippedUrl) that estimates how often PAC scripts depend o… (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@2704
Patch Set: Created 4 years, 8 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
« no previous file with comments | « net/proxy/proxy_resolver_v8_tracing.cc ('k') | tools/metrics/histograms/histograms.xml » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 4d2b29660f2e0e684cf91ee508ce52d066f3889e..888dd18af57d30ddf26219bf0aa4b8166c4a607e 100644
--- a/net/proxy/proxy_resolver_v8_tracing_unittest.cc
+++ b/net/proxy/proxy_resolver_v8_tracing_unittest.cc
@@ -12,7 +12,6 @@
#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"
@@ -38,23 +37,6 @@ 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) {
@@ -178,15 +160,12 @@ TEST_F(ProxyResolverV8TracingTest, Simple) {
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());
@@ -196,36 +175,6 @@ 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);
@@ -236,14 +185,12 @@ TEST_F(ProxyResolverV8TracingTest, JavascriptError) {
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.
@@ -265,16 +212,12 @@ TEST_F(ProxyResolverV8TracingTest, TooManyAlerts) {
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
@@ -305,16 +248,12 @@ TEST_F(ProxyResolverV8TracingTest, TooManyEmptyAlerts) {
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());
@@ -355,15 +294,12 @@ TEST_F(ProxyResolverV8TracingTest, Dns) {
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());
@@ -410,18 +346,12 @@ TEST_F(ProxyResolverV8TracingTest, DnsChecksCache) {
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());
@@ -434,19 +364,10 @@ 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.
- // 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());
@@ -470,15 +391,11 @@ TEST_F(ProxyResolverV8TracingTest, FallBackToSynchronous1) {
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.
@@ -513,15 +430,11 @@ TEST_F(ProxyResolverV8TracingTest, FallBackToSynchronous2) {
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());
@@ -553,9 +466,6 @@ 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(
@@ -596,9 +506,6 @@ 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());
@@ -611,94 +518,6 @@ 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);
« no previous file with comments | « net/proxy/proxy_resolver_v8_tracing.cc ('k') | tools/metrics/histograms/histograms.xml » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698