Remove unused was_fetched_via_proxy members (issue 2356013002 by csharrison@chromium.org)

[cshar...@chromium.org]

Reviewers: nasko (slow)
CL: https://codereview.chromium.org/2356013002/

Message:
If this patch causes runtime errors I will be terrified. So far so good...

Nasko, deleting unused code. yay or nay?

Description:
Remove unused was_fetched_via_proxy members

Affected files (+0, -64 lines):
M content/child/web_url_loader_impl.cc
M content/child/weburlresponse_extradata_impl.h
M content/common/frame_messages.h
M content/public/renderer/document_state.h
M content/public/renderer/document_state.cc
M content/renderer/render_frame_impl.cc
M third_party/WebKit/Source/platform/exported/WebURLResponse.cpp
M third_party/WebKit/Source/platform/network/ResourceResponse.h
M third_party/WebKit/public/platform/WebURLResponse.h


Index: content/child/web_url_loader_impl.cc
diff --git a/content/child/web_url_loader_impl.cc b/content/child/web_url_loader_impl.cc
index c405acb003d48294c58353dd532a02a772818b73..b4032598f54a3e0540133c3a00bca3e6fba21ed8 100644
--- a/content/child/web_url_loader_impl.cc
+++ b/content/child/web_url_loader_impl.cc
@@ -1018,8 +1018,6 @@ void WebURLLoaderImpl::PopulateURLResponse(const GURL& url,
extra_data->set_was_alternate_protocol_available(
info.was_alternate_protocol_available);
extra_data->set_connection_info(info.connection_info);
- extra_data->set_was_fetched_via_proxy(info.was_fetched_via_proxy);
- extra_data->set_proxy_server(info.proxy_server);
extra_data->set_is_using_lofi(info.is_using_lofi);
extra_data->set_effective_connection_type(info.effective_connection_type);

Index: content/child/weburlresponse_extradata_impl.h
diff --git a/content/child/weburlresponse_extradata_impl.h b/content/child/weburlresponse_extradata_impl.h
index 1529138d0a7c20d2e8e86a0fb78a68f1cda97b66..deace323c4f96fcf4e7846127527480d547d40d2 100644
--- a/content/child/weburlresponse_extradata_impl.h
+++ b/content/child/weburlresponse_extradata_impl.h
@@ -27,24 +27,6 @@ class CONTENT_EXPORT WebURLResponseExtraDataImpl :
return npn_negotiated_protocol_;
}

- // Flag whether this request was loaded via an explicit proxy
- // (HTTP, SOCKS, etc).
- bool was_fetched_via_proxy() const {
- return was_fetched_via_proxy_;
- }
- void set_was_fetched_via_proxy(bool was_fetched_via_proxy) {
- was_fetched_via_proxy_ = was_fetched_via_proxy;
- }
-
- // The proxy server used if this request was loaded via an explicit proxy
- // (HTTP, SOCKS, etc).
- net::HostPortPair proxy_server() const {
- return proxy_server_;
- }
- void set_proxy_server(net::HostPortPair proxy_server) {
- proxy_server_ = proxy_server;
- }
-
/// Flag whether this request was loaded via the SPDY protocol or not.
// SPDY is an experimental web protocol, see http://dev.chromium.org/spdy
bool was_fetched_via_spdy() const {
@@ -102,8 +84,6 @@ class CONTENT_EXPORT WebURLResponseExtraDataImpl :
private:
std::string npn_negotiated_protocol_;
bool is_ftp_directory_listing_;
- bool was_fetched_via_proxy_;
- net::HostPortPair proxy_server_;
bool was_fetched_via_spdy_;
bool was_npn_negotiated_;
net::HttpResponseInfo::ConnectionInfo connection_info_;
Index: content/common/frame_messages.h
diff --git a/content/common/frame_messages.h b/content/common/frame_messages.h
index 64ef075a71469f16d0f5a0680117c22299805575..dd4b1e4ccbd9695e48499c5e30bc28ac3c4ea391 100644
--- a/content/common/frame_messages.h
+++ b/content/common/frame_messages.h
@@ -255,10 +255,6 @@ IPC_STRUCT_BEGIN_WITH_PARENT(FrameHostMsg_DidCommitProvisionalLoad_Params,
// so that we can set the appropriate page type.
IPC_STRUCT_MEMBER(bool, url_is_unreachable)

- // True if the connection was proxied. In this case, socket_address
- // will represent the address of the proxy, rather than the remote host.
- IPC_STRUCT_MEMBER(bool, was_fetched_via_proxy)
-
// Serialized history item state to store in the navigation entry.
IPC_STRUCT_MEMBER(content::PageState, page_state)

Index: content/public/renderer/document_state.cc
diff --git a/content/public/renderer/document_state.cc b/content/public/renderer/document_state.cc
index 30d75c756816c80efc8f2ec2363b33fd4f059205..3f61ca52b8774329ceb0b48b0c0d600c857c9d8a 100644
--- a/content/public/renderer/document_state.cc
+++ b/content/public/renderer/document_state.cc
@@ -15,7 +15,6 @@ DocumentState::DocumentState()
was_npn_negotiated_(false),
was_alternate_protocol_available_(false),
connection_info_(net::HttpResponseInfo::CONNECTION_INFO_UNKNOWN),
- was_fetched_via_proxy_(false),
was_load_data_with_base_url_request_(false),
load_type_(UNDEFINED_LOAD),
can_load_local_resources_(false) {
Index: content/public/renderer/document_state.h
diff --git a/content/public/renderer/document_state.h b/content/public/renderer/document_state.h
index 437cec09eef4773e926b92a0486036e6740262b5..c56d841c771f9ccd6fa5d3fe73a247548af9283a 100644
--- a/content/public/renderer/document_state.h
+++ b/content/public/renderer/document_state.h
@@ -157,17 +157,6 @@ class CONTENT_EXPORT DocumentState
connection_info_ = connection_info;
}

- bool was_fetched_via_proxy() const { return was_fetched_via_proxy_; }
- void set_was_fetched_via_proxy(bool value) {
- was_fetched_via_proxy_ = value;
- }
-
- const net::HostPortPair& proxy_server() const { return proxy_server_; }
- void set_proxy_server(const net::HostPortPair& proxy_server) {
- proxy_server_ = proxy_server;
- }
-
-
// For LoadDataWithBaseURL navigations, |was_load_data_with_base_url_request_|
// is set to true and |data_url_| is set to the data URL of the navigation.
// Otherwise, |was_load_data_with_base_url_request_| is false and |data_url_|
@@ -212,8 +201,6 @@ class CONTENT_EXPORT DocumentState
std::string npn_negotiated_protocol_;
bool was_alternate_protocol_available_;
net::HttpResponseInfo::ConnectionInfo connection_info_;
- bool was_fetched_via_proxy_;
- net::HostPortPair proxy_server_;

bool was_load_data_with_base_url_request_;
GURL data_url_;
Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 48f0183ed0ba72e1ae632028c1a41d224b87eec7..48eaf6075b0d37ddca9096e75320a25c37922143 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -4190,10 +4190,6 @@ void RenderFrameImpl::didReceiveResponse(
extra_data->was_alternate_protocol_available());
document_state->set_connection_info(
extra_data->connection_info());
- document_state->set_was_fetched_via_proxy(
- extra_data->was_fetched_via_proxy());
- document_state->set_proxy_server(
- extra_data->proxy_server());
}
InternalDocumentStateData* internal_data =
InternalDocumentStateData::FromDocumentState(document_state);
@@ -4619,9 +4615,6 @@ void RenderFrameImpl::SendDidCommitProvisionalLoad(
params.render_view_routing_id = render_view_->routing_id();
params.socket_address.set_host(response.remoteIPAddress().utf8());
params.socket_address.set_port(response.remotePort());
- WebURLResponseExtraDataImpl* extra_data = GetExtraDataFromResponse(response);
- if (extra_data)
- params.was_fetched_via_proxy = extra_data->was_fetched_via_proxy();
params.was_within_same_page = navigation_state->WasWithinSamePage();

// Set the origin of the frame. This will be replicated to the corresponding
Index: third_party/WebKit/Source/platform/exported/WebURLResponse.cpp
diff --git a/third_party/WebKit/Source/platform/exported/WebURLResponse.cpp b/third_party/WebKit/Source/platform/exported/WebURLResponse.cpp
index 5b5c8c4a03d449fa3ae09795fcf0eb44a10f5c38..540d5c22c3ea1993e0ef162c5665df73307864a3 100644
--- a/third_party/WebKit/Source/platform/exported/WebURLResponse.cpp
+++ b/third_party/WebKit/Source/platform/exported/WebURLResponse.cpp
@@ -396,16 +396,6 @@ void WebURLResponse::setWasAlternateProtocolAvailable(bool value)
m_resourceResponse->setWasAlternateProtocolAvailable(value);
}

-bool WebURLResponse::wasFetchedViaProxy() const
-{
- return m_resourceResponse->wasFetchedViaProxy();
-}
-
-void WebURLResponse::setWasFetchedViaProxy(bool value)
-{
- m_resourceResponse->setWasFetchedViaProxy(value);
-}
-
bool WebURLResponse::wasFetchedViaServiceWorker() const
{
return m_resourceResponse->wasFetchedViaServiceWorker();
Index: third_party/WebKit/Source/platform/network/ResourceResponse.h
diff --git a/third_party/WebKit/Source/platform/network/ResourceResponse.h b/third_party/WebKit/Source/platform/network/ResourceResponse.h
index 2b644b47d9167dd4cf94f852c1c2a772d4357e75..f447511995639cf13d3e9c1d695e2e49a71600d6 100644
--- a/third_party/WebKit/Source/platform/network/ResourceResponse.h
+++ b/third_party/WebKit/Source/platform/network/ResourceResponse.h
@@ -241,9 +241,6 @@ public:
m_wasAlternateProtocolAvailable = value;
}

- bool wasFetchedViaProxy() const { return m_wasFetchedViaProxy; }
- void setWasFetchedViaProxy(bool value) { m_wasFetchedViaProxy = value; }
-
bool wasFetchedViaServiceWorker() const { return m_wasFetchedViaServiceWorker; }
void setWasFetchedViaServiceWorker(bool value) { m_wasFetchedViaServiceWorker = value; }

Index: third_party/WebKit/public/platform/WebURLResponse.h
diff --git a/third_party/WebKit/public/platform/WebURLResponse.h b/third_party/WebKit/public/platform/WebURLResponse.h
index ff22cb9a5ae479391245e7c64d44e3cf7cd7094c..64fbae544772c381577a59d3893d5ea5448794df 100644
--- a/third_party/WebKit/public/platform/WebURLResponse.h
+++ b/third_party/WebKit/public/platform/WebURLResponse.h
@@ -238,10 +238,6 @@ public:
BLINK_PLATFORM_EXPORT bool wasAlternateProtocolAvailable() const;
BLINK_PLATFORM_EXPORT void setWasAlternateProtocolAvailable(bool);

- // Flag whether this request was loaded via an explicit proxy (HTTP, SOCKS, etc).
- BLINK_PLATFORM_EXPORT bool wasFetchedViaProxy() const;
- BLINK_PLATFORM_EXPORT void setWasFetchedViaProxy(bool);
-
// Flag whether this request was loaded via a ServiceWorker.
BLINK_PLATFORM_EXPORT bool wasFetchedViaServiceWorker() const;
BLINK_PLATFORM_EXPORT void setWasFetchedViaServiceWorker(bool);

[na...@chromium.org]

All unused code removal is LGTM in my book :). I'd suggest pinging bengr@ just
in case, since they recently had a need to know whether something was fetched
through a proxy, but IIRC they went with a different design.

https://codereview.chromium.org/2356013002/

[cshar...@chromium.org]

Thanks, +bengr in that case.

https://codereview.chromium.org/2356013002/

[tba...@chromium.org]

drive-by-comment:

Thanks a ton for doing this. DRP definitely does not need proxy server on the
renderer side. As nasko said, we went with a different approach.

Also, it seems that now it is possible to remove the two variables (bool and
HostPortPair) from content/public/common/resource_response_info.h,
content/common/resource_messages.h and resource_loader.cc.

We can do it in this CL or I can do it in the next one. Removing them would
definitely make my related CL (https://codereview.chromium.org/2334623003/) more
palatable :).

https://codereview.chromium.org/2356013002/

[cshar...@chromium.org]

Thanks for the drive-by. Updated the CL per your request.

https://codereview.chromium.org/2356013002/

[tba...@chromium.org]

On 2016/09/23 18:01:49, Charlie Harrison wrote:
> Thanks for the drive-by. Updated the CL per your request.

Great. Thank you.

https://codereview.chromium.org/2356013002/

[be...@chromium.org]

lgtm



https://codereview.chromium.org/2356013002/

[cshar...@chromium.org]

Thanks. +haraken for platform changes.

https://codereview.chromium.org/2356013002/

[cshar...@chromium.org]

-haraken, +jochen for public and platform changes (simple).

https://codereview.chromium.org/2356013002/

[har...@chromium.org]

On 2016/09/23 18:28:39, Charlie Harrison wrote:
> -haraken, +jochen for public and platform changes (simple).

LGTM


https://codereview.chromium.org/2356013002/

[joc...@chromium.org]

lgtm



https://codereview.chromium.org/2356013002/

[commit-bot@chromium.org via codereview.chromium.org]

CQ is trying da patch. Follow status at

https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2356013002/40001


https://codereview.chromium.org/2356013002/

[commit-bot@chromium.org via codereview.chromium.org]

Try jobs failed on following builders:
win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/286132)

https://codereview.chromium.org/2356013002/

[commit-bot@chromium.org via codereview.chromium.org]

[commit-bot@chromium.org via codereview.chromium.org]

Committed patchset #3 (id:40001)

https://codereview.chromium.org/2356013002/

[commit-bot@chromium.org via codereview.chromium.org]

Patchset 3 (id:??) landed as
https://crrev.com/49714cce0664eadd15a9de6291b3b01d127156b4
Cr-Commit-Position: refs/heads/master@{#422123}

https://codereview.chromium.org/2356013002/