diff options
author | lzheng@chromium.org <lzheng@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-26 03:57:57 +0000 |
---|---|---|
committer | lzheng@chromium.org <lzheng@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-26 03:57:57 +0000 |
commit | 193b0b89b4b49f8e8eb985b8f1a16e3c8506ee66 (patch) | |
tree | 5c7fe26f96ac2654fbb9e47e6b54b2e1d6faa29a | |
parent | e22fbfc7ddc9e6539d734f6d976f54eeca5111ec (diff) | |
download | chromium_src-193b0b89b4b49f8e8eb985b8f1a16e3c8506ee66.zip chromium_src-193b0b89b4b49f8e8eb985b8f1a16e3c8506ee66.tar.gz chromium_src-193b0b89b4b49f8e8eb985b8f1a16e3c8506ee66.tar.bz2 |
Add field trial stats for alternate_protocol. The histogram we collected are:
http stats when alternate protocol is available but npn was not negotiated (
which means not usng spdy), and when alternate is available AND spdy is used.
Noticable changes:
1. In http_network_transaction.cc, changed the logic that always parse
response from server for alternate protocol and remember that in
HttpAlternateProtocols strucuture. We need to remember this to collect
stats for servers with alternate protocol support but used http for.
2. In spdy_stream.cc, get rid of the response copy from spdy_stream. This
copy overwrites some early status in response set in http_network_transaction.
TEST=http_network_transaction_unittest.cc
BUG=46689
Review URL: http://codereview.chromium.org/2808010
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50927 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/renderer_host/resource_dispatcher_host.cc | 2 | ||||
-rw-r--r-- | chrome/common/render_messages.h | 4 | ||||
-rw-r--r-- | chrome/renderer/loadtimes_extension_bindings.cc | 4 | ||||
-rw-r--r-- | chrome/renderer/navigation_state.h | 8 | ||||
-rw-r--r-- | chrome/renderer/render_view.cc | 59 | ||||
-rw-r--r-- | net/http/http_network_transaction.cc | 14 | ||||
-rw-r--r-- | net/http/http_network_transaction_unittest.cc | 4 | ||||
-rw-r--r-- | net/http/http_response_info.cc | 10 | ||||
-rw-r--r-- | net/http/http_response_info.h | 4 | ||||
-rw-r--r-- | net/url_request/url_request.h | 9 | ||||
-rw-r--r-- | webkit/glue/resource_loader_bridge.cc | 1 | ||||
-rw-r--r-- | webkit/glue/resource_loader_bridge.h | 4 | ||||
-rw-r--r-- | webkit/glue/weburlloader_impl.cc | 2 |
13 files changed, 114 insertions, 11 deletions
diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.cc b/chrome/browser/renderer_host/resource_dispatcher_host.cc index 1b42072..4b3421e 100644 --- a/chrome/browser/renderer_host/resource_dispatcher_host.cc +++ b/chrome/browser/renderer_host/resource_dispatcher_host.cc @@ -164,6 +164,8 @@ void PopulateResourceResponse(URLRequest* request, response->response_head.was_fetched_via_spdy = request->was_fetched_via_spdy(); response->response_head.was_npn_negotiated = request->was_npn_negotiated(); + response->response_head.was_alternate_protocol_available = + request->was_alternate_protocol_available(); response->response_head.was_fetched_via_proxy = request->was_fetched_via_proxy(); appcache::AppCacheInterceptor::GetExtraResponseInfo( diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h index 061a236..fc54b29 100644 --- a/chrome/common/render_messages.h +++ b/chrome/common/render_messages.h @@ -1366,6 +1366,7 @@ struct ParamTraits<webkit_glue::ResourceLoaderBridge::ResponseInfo> { WriteParam(m, p.appcache_manifest_url); WriteParam(m, p.was_fetched_via_spdy); WriteParam(m, p.was_npn_negotiated); + WriteParam(m, p.was_alternate_protocol_available); WriteParam(m, p.was_fetched_via_proxy); } static bool Read(const Message* m, void** iter, param_type* r) { @@ -1381,6 +1382,7 @@ struct ParamTraits<webkit_glue::ResourceLoaderBridge::ResponseInfo> { ReadParam(m, iter, &r->appcache_manifest_url) && ReadParam(m, iter, &r->was_fetched_via_spdy) && ReadParam(m, iter, &r->was_npn_negotiated) && + ReadParam(m, iter, &r->was_alternate_protocol_available) && ReadParam(m, iter, &r->was_fetched_via_proxy); } static void Log(const param_type& p, std::wstring* l) { @@ -1407,6 +1409,8 @@ struct ParamTraits<webkit_glue::ResourceLoaderBridge::ResponseInfo> { l->append(L", "); LogParam(p.was_npn_negotiated, l); l->append(L", "); + LogParam(p.was_alternate_protocol_available, l); + l->append(L", "); LogParam(p.was_fetched_via_proxy, l); l->append(L")"); } diff --git a/chrome/renderer/loadtimes_extension_bindings.cc b/chrome/renderer/loadtimes_extension_bindings.cc index 151e78d..1ad71ed 100644 --- a/chrome/renderer/loadtimes_extension_bindings.cc +++ b/chrome/renderer/loadtimes_extension_bindings.cc @@ -134,6 +134,10 @@ class LoadTimesExtensionWrapper : public v8::Extension { load_times->Set( v8::String::New("wasNpnNegotiated"), v8::Boolean::New(navigation_state->was_npn_negotiated())); + load_times->Set( + v8::String::New("wasAlternateProtocolAvailable"), + v8::Boolean::New( + navigation_state->was_alternate_protocol_available())); return load_times; } } diff --git a/chrome/renderer/navigation_state.h b/chrome/renderer/navigation_state.h index ef3a54e..694057d 100644 --- a/chrome/renderer/navigation_state.h +++ b/chrome/renderer/navigation_state.h @@ -223,6 +223,13 @@ class NavigationState : public WebKit::WebDataSource::ExtraData { void set_was_npn_negotiated(bool value) { was_npn_negotiated_ = value; } bool was_npn_negotiated() const { return was_npn_negotiated_; } + void set_was_alternate_protocol_available(bool value) { + was_alternate_protocol_available_ = value; + } + bool was_alternate_protocol_available() const { + return was_alternate_protocol_available_; + } + void set_was_fetched_via_proxy(bool value) { was_fetched_via_proxy_ = value; } @@ -287,6 +294,7 @@ class NavigationState : public WebKit::WebDataSource::ExtraData { bool was_npn_negotiated_; bool was_fetched_via_proxy_; bool was_translated_; + bool was_alternate_protocol_available_; DISALLOW_COPY_AND_ASSIGN(NavigationState); }; diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc index 59f600e..aa6fc64 100644 --- a/chrome/renderer/render_view.cc +++ b/chrome/renderer/render_view.cc @@ -3005,6 +3005,8 @@ void RenderView::didReceiveResponse( // Record page load flags. navigation_state->set_was_fetched_via_spdy(response.wasFetchedViaSPDY()); navigation_state->set_was_npn_negotiated(response.wasNpnNegotiated()); + navigation_state->set_was_alternate_protocol_available( + response.wasAlternateProtocolAvailable()); navigation_state->set_was_fetched_via_proxy(response.wasFetchedViaProxy()); // Consider loading an alternate error page for 404 responses. @@ -4715,7 +4717,12 @@ void RenderView::DumpLoadHistograms() const { // and StartToFinish, consider removing one or the other. static bool use_spdy_histogram(FieldTrialList::Find("SpdyImpact") && !FieldTrialList::Find("SpdyImpact")->group_name().empty()); - if (use_spdy_histogram && navigation_state->was_npn_negotiated()) { + // Spdy requests triggered by alternate protocol are excluded here. + // This is because when alternate protocol is avaiable, FieldTrial will + // either use npn_spdy or pure http, not npn_http via TLS. That will cause + // bias for npn_spdy and npn_http. + if (use_spdy_histogram && navigation_state->was_npn_negotiated() && + !navigation_state->was_alternate_protocol_available()) { UMA_HISTOGRAM_ENUMERATION( FieldTrial::MakeName("PLT.Abandoned", "SpdyImpact"), abandoned_page ? 1 : 0, 2); @@ -4747,6 +4754,56 @@ void RenderView::DumpLoadHistograms() const { } } + // Histograms to compare the impact of alternate protocol: when the + // protocol(spdy) is used vs. when it is ignored and http is used. + if (navigation_state->was_alternate_protocol_available()) { + if (!navigation_state->was_npn_negotiated()) { + // This means that even there is alternate protocols for npn_http or + // npn_spdy, they are not taken (due to the fieldtrial). + switch (load_type) { + case NavigationState::LINK_LOAD_NORMAL: + PLT_HISTOGRAM( + "PLT.StartToFinish_LinkLoadNormal_AlternateProtocol_http", + start_to_finish); + PLT_HISTOGRAM( + "PLT.StartToCommit_LinkLoadNormal_AlternateProtocol_http", + start_to_commit); + break; + case NavigationState::NORMAL_LOAD: + PLT_HISTOGRAM( + "PLT.StartToFinish_NormalLoad_AlternateProtocol_http", + start_to_finish); + PLT_HISTOGRAM( + "PLT.StartToCommit_NormalLoad_AlternateProtocol_http", + start_to_commit); + break; + default: + break; + } + } else if (navigation_state->was_fetched_via_spdy()) { + switch (load_type) { + case NavigationState::LINK_LOAD_NORMAL: + PLT_HISTOGRAM( + "PLT.StartToFinish_LinkLoadNormal_AlternateProtocol_spdy", + start_to_finish); + PLT_HISTOGRAM( + "PLT.StartToCommit_LinkLoadNormal_AlternateProtocol_spdy", + start_to_commit); + break; + case NavigationState::NORMAL_LOAD: + PLT_HISTOGRAM( + "PLT.StartToFinish_NormalLoad_AlternateProtocol_spdy", + start_to_finish); + PLT_HISTOGRAM( + "PLT.StartToCommit_NormalLoad_AlternateProtocol_spdy", + start_to_commit); + break; + default: + break; + } + } + } + // Record page load time and abandonment rates for proxy cases. GURL url = GURL(main_frame->url()); if (navigation_state->was_fetched_via_proxy()) { diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index 016cd58..2b14d02 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -155,11 +155,6 @@ void BuildTunnelRequest(const HttpRequestInfo* request_info, void ProcessAlternateProtocol(const HttpResponseHeaders& headers, const HostPortPair& http_host_port_pair, HttpAlternateProtocols* alternate_protocols) { - if (!g_next_protos || g_next_protos->empty()) { - // This implies that NPN is not supported. We don't currently support any - // alternate protocols that don't use NPN. - return; - } std::string alternate_protocol_str; if (!headers.EnumerateHeader(NULL, HttpAlternateProtocols::kHeader, @@ -763,10 +758,11 @@ int HttpNetworkTransaction::DoResolveProxy() { curr_endpoint_url = &alternate_endpoint_url; } - if (alternate_protocol_mode_ == kUnspecified) { - const HttpAlternateProtocols& alternate_protocols = - session_->alternate_protocols(); - if (alternate_protocols.HasAlternateProtocolFor(endpoint_)) { + const HttpAlternateProtocols& alternate_protocols = + session_->alternate_protocols(); + if (alternate_protocols.HasAlternateProtocolFor(endpoint_)) { + response_.was_alternate_protocol_available = true; + if (alternate_protocol_mode_ == kUnspecified) { HttpAlternateProtocols::PortProtocolPair alternate = alternate_protocols.GetAlternateProtocolFor(endpoint_); if (alternate.protocol != HttpAlternateProtocols::BROKEN) { diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index dd04773..991e697 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -4926,6 +4926,7 @@ TEST_F(HttpNetworkTransactionTest, HonorAlternateProtocolHeader) { EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine()); EXPECT_FALSE(response->was_fetched_via_spdy); EXPECT_FALSE(response->was_npn_negotiated); + EXPECT_FALSE(response->was_alternate_protocol_available); std::string response_data; ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data)); @@ -5195,6 +5196,7 @@ TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForNpnSpdy) { EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine()); EXPECT_TRUE(response->was_fetched_via_spdy); EXPECT_TRUE(response->was_npn_negotiated); + EXPECT_TRUE(response->was_alternate_protocol_available); ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data)); EXPECT_EQ("hello!", response_data); @@ -5433,6 +5435,7 @@ TEST_F(HttpNetworkTransactionTest, EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine()); EXPECT_TRUE(response->was_fetched_via_spdy); EXPECT_TRUE(response->was_npn_negotiated); + EXPECT_TRUE(response->was_alternate_protocol_available); ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data)); EXPECT_EQ("hello!", response_data); @@ -6090,6 +6093,7 @@ TEST_F(HttpNetworkTransactionTest, NpnWithHttpOverSSL) { EXPECT_FALSE(response->was_fetched_via_spdy); EXPECT_TRUE(response->was_npn_negotiated); + EXPECT_FALSE(response->was_alternate_protocol_available); HttpNetworkTransaction::SetNextProtos(""); HttpNetworkTransaction::SetUseAlternateProtocols(false); diff --git a/net/http/http_response_info.cc b/net/http/http_response_info.cc index d1e7911..0159caf 100644 --- a/net/http/http_response_info.cc +++ b/net/http/http_response_info.cc @@ -47,6 +47,10 @@ enum { // This bit is set if the request was fetched via an explicit proxy. RESPONSE_INFO_WAS_PROXY = 1 << 15, + // This bit is set if response could use alternate protocol. However, browser + // will ingore the alternate protocol if spdy is not enabled. + RESPONSE_INFO_WAS_ALTERNATE_PROTOCOL_AVAILABLE = 1 << 16, + // TODO(darin): Add other bits to indicate alternate request methods. // For now, we don't support storing those. }; @@ -55,6 +59,7 @@ HttpResponseInfo::HttpResponseInfo() : was_cached(false), was_fetched_via_spdy(false), was_npn_negotiated(false), + was_alternate_protocol_available(false), was_fetched_via_proxy(false) { } @@ -119,6 +124,9 @@ bool HttpResponseInfo::InitFromPickle(const Pickle& pickle, was_npn_negotiated = (flags & RESPONSE_INFO_WAS_NPN) != 0; + was_alternate_protocol_available = + (flags & RESPONSE_INFO_WAS_ALTERNATE_PROTOCOL_AVAILABLE) != 0; + was_fetched_via_proxy = (flags & RESPONSE_INFO_WAS_PROXY) != 0; *response_truncated = (flags & RESPONSE_INFO_TRUNCATED) ? true : false; @@ -144,6 +152,8 @@ void HttpResponseInfo::Persist(Pickle* pickle, flags |= RESPONSE_INFO_WAS_SPDY; if (was_npn_negotiated) flags |= RESPONSE_INFO_WAS_NPN; + if (was_alternate_protocol_available) + flags |= RESPONSE_INFO_WAS_ALTERNATE_PROTOCOL_AVAILABLE; if (was_fetched_via_proxy) flags |= RESPONSE_INFO_WAS_PROXY; diff --git a/net/http/http_response_info.h b/net/http/http_response_info.h index 518d8ac3..52c3a0e 100644 --- a/net/http/http_response_info.h +++ b/net/http/http_response_info.h @@ -38,6 +38,10 @@ class HttpResponseInfo { // True if the npn was negotiated for this request. bool was_npn_negotiated; + // True if response could use alternate protocol. However, browser + // will ingore the alternate protocol if spdy is not enabled. + bool was_alternate_protocol_available; + // True if the request was fetched via an explicit proxy. The proxy could // be any type of proxy, HTTP or SOCKS. Note, we do not know if a // transparent proxy may have been involved. diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h index aff01c7..f770c89 100644 --- a/net/url_request/url_request.h +++ b/net/url_request/url_request.h @@ -371,7 +371,8 @@ class URLRequest { // Indicate if this response was fetched from disk cache. bool was_cached() const { return response_info_.was_cached; } - // Returns true if the URLRequest was delivered with SPDY. + // True if response could use alternate protocol. However, browser will + // ingore the alternate protocol if spdy is not enabled. bool was_fetched_via_spdy() const { return response_info_.was_fetched_via_spdy; } @@ -382,6 +383,12 @@ class URLRequest { return response_info_.was_npn_negotiated; } + // Returns true if the URLRequest was delivered when the alertnate protocol + // is available. + bool was_alternate_protocol_available() const { + return response_info_.was_alternate_protocol_available; + } + // Returns true if the URLRequest was delivered through a proxy. bool was_fetched_via_proxy() const { return response_info_.was_fetched_via_proxy; diff --git a/webkit/glue/resource_loader_bridge.cc b/webkit/glue/resource_loader_bridge.cc index 1e41f22..af77ade 100644 --- a/webkit/glue/resource_loader_bridge.cc +++ b/webkit/glue/resource_loader_bridge.cc @@ -26,6 +26,7 @@ ResourceLoaderBridge::ResponseInfo::ResponseInfo() { appcache_id = appcache::kNoCacheId; was_fetched_via_spdy = false; was_npn_negotiated = false; + was_alternate_protocol_available = false; was_fetched_via_proxy = false; } diff --git a/webkit/glue/resource_loader_bridge.h b/webkit/glue/resource_loader_bridge.h index 7c19af0..1d76625 100644 --- a/webkit/glue/resource_loader_bridge.h +++ b/webkit/glue/resource_loader_bridge.h @@ -128,6 +128,10 @@ class ResourceLoaderBridge { // True if the response was delivered after NPN is negotiated. bool was_npn_negotiated; + // True if response could use alternate protocol. However, browser will + // ignore the alternate protocol when spdy is not enabled on browser side. + bool was_alternate_protocol_available; + // True if the response was fetched via an explicit proxy (as opposed to a // transparent proxy). The proxy could be any type of proxy, HTTP or SOCKS. // Note: we cannot tell if a transparent proxy may have been involved. diff --git a/webkit/glue/weburlloader_impl.cc b/webkit/glue/weburlloader_impl.cc index 8800d6a9..3fb0206 100644 --- a/webkit/glue/weburlloader_impl.cc +++ b/webkit/glue/weburlloader_impl.cc @@ -172,6 +172,8 @@ void PopulateURLResponse( response->setAppCacheManifestURL(info.appcache_manifest_url); response->setWasFetchedViaSPDY(info.was_fetched_via_spdy); response->setWasNpnNegotiated(info.was_npn_negotiated); + response->setWasAlternateProtocolAvailable( + info.was_alternate_protocol_available); response->setWasFetchedViaProxy(info.was_fetched_via_proxy); const net::HttpResponseHeaders* headers = info.headers; |