Don't store both content::SSLStatus and net::SSLInfo on a NavigationHandle.
Just store the net::SSLInfo, since we can derive content::SSLStatus from it.
Bug:
Change-Id: I2f989a6d69405463f40954c7c29bf509870d5d43
Reviewed-on: https://chromium-review.googlesource.com/794971
Reviewed-by: Camille Lamy <[email protected]>
Reviewed-by: Emily Stark <[email protected]>
Commit-Queue: John Abd-El-Malek <[email protected]>
Cr-Commit-Position: refs/heads/master@{#520561}diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 4597350d..a5ac096 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -1148,7 +1148,7 @@
new_entry = pending_entry_->Clone();
update_virtual_url = new_entry->update_virtual_url_with_url();
- new_entry->GetSSL() = handle->ssl_status();
+ new_entry->GetSSL() = SSLStatus(handle->GetSSLInfo());
if (params.url.SchemeIs(url::kHttpsScheme) && !rfh->GetParent() &&
handle->GetNetErrorCode() == net::OK) {
@@ -1176,7 +1176,7 @@
// to show chrome://bookmarks/#1 when the bookmarks webui extension changes
// the URL.
update_virtual_url = needs_update;
- new_entry->GetSSL() = handle->ssl_status();
+ new_entry->GetSSL() = SSLStatus(handle->GetSSLInfo());
if (params.url.SchemeIs(url::kHttpsScheme) && !rfh->GetParent() &&
handle->GetNetErrorCode() == net::OK) {
@@ -1254,7 +1254,7 @@
// If this is a same document navigation, then there's no SSLStatus in the
// NavigationHandle so don't overwrite the existing entry's SSLStatus.
if (!is_same_document)
- entry->GetSSL() = handle->ssl_status();
+ entry->GetSSL() = SSLStatus(handle->GetSSLInfo());
if (params.url.SchemeIs(url::kHttpsScheme) && !rfh->GetParent() &&
handle->GetNetErrorCode() == net::OK) {
@@ -1293,7 +1293,7 @@
// Only copy in the restore case since this code path can be taken during
// navigation. See http://crbug.com/727892
if (was_restored)
- entry->GetSSL() = handle->ssl_status();
+ entry->GetSSL() = SSLStatus(handle->GetSSLInfo());
}
if (params.url.SchemeIs(url::kHttpsScheme) && !rfh->GetParent() &&
@@ -1329,7 +1329,7 @@
// If this is a same document navigation, then there's no SSLStatus in the
// NavigationHandle so don't overwrite the existing entry's SSLStatus.
if (!is_same_document)
- entry->GetSSL() = handle->ssl_status();
+ entry->GetSSL() = SSLStatus(handle->GetSSLInfo());
if (params.url.SchemeIs(url::kHttpsScheme) && !rfh->GetParent() &&
handle->GetNetErrorCode() == net::OK) {
@@ -1421,7 +1421,7 @@
// If a user presses enter in the omnibox and the server redirects, the URL
// might change (but it's still considered a SAME_PAGE navigation). So we must
// update the SSL status.
- existing_entry->GetSSL() = handle->ssl_status();
+ existing_entry->GetSSL() = SSLStatus(handle->GetSSLInfo());
if (existing_entry->GetURL().SchemeIs(url::kHttpsScheme) &&
!rfh->GetParent() && handle->GetNetErrorCode() == net::OK) {
diff --git a/content/browser/frame_host/navigation_handle_impl.cc b/content/browser/frame_host/navigation_handle_impl.cc
index 25e23a3..73afa8b3 100644
--- a/content/browser/frame_host/navigation_handle_impl.cc
+++ b/content/browser/frame_host/navigation_handle_impl.cc
@@ -343,7 +343,7 @@
return connection_info_;
}
-const base::Optional<net::SSLInfo>& NavigationHandleImpl::GetSSLInfo() {
+const net::SSLInfo& NavigationHandleImpl::GetSSLInfo() {
return ssl_info_;
}
@@ -474,7 +474,7 @@
NavigationThrottle::ThrottleCheckResult result = NavigationThrottle::DEFER;
WillProcessResponse(static_cast<RenderFrameHostImpl*>(render_frame_host),
headers, net::HttpResponseInfo::CONNECTION_INFO_UNKNOWN,
- net::HostPortPair(), SSLStatus(), GlobalRequestID(),
+ net::HostPortPair(), net::SSLInfo(), GlobalRequestID(),
false, false, false, base::Closure(),
base::Bind(&UpdateThrottleCheckResult, &result));
@@ -718,15 +718,12 @@
const ThrottleChecksFinishedCallback& callback) {
TRACE_EVENT_ASYNC_STEP_INTO0("navigation", "NavigationHandle", this,
"WillFailRequest");
-
- ssl_info_ = ssl_info;
+ if (ssl_info.has_value())
+ ssl_info_ = ssl_info.value();
should_ssl_errors_be_fatal_ = should_ssl_errors_be_fatal;
complete_callback_ = callback;
state_ = WILL_FAIL_REQUEST;
- if (ssl_info.has_value())
- ssl_status_ = SSLStatus(ssl_info.value());
-
// Notify each throttle of the request.
base::Closure on_defer_callback_copy = on_defer_callback_for_testing_;
NavigationThrottle::ThrottleCheckResult result = CheckWillFailRequest();
@@ -748,7 +745,7 @@
scoped_refptr<net::HttpResponseHeaders> response_headers,
net::HttpResponseInfo::ConnectionInfo connection_info,
const net::HostPortPair& socket_address,
- const SSLStatus& ssl_status,
+ const net::SSLInfo& ssl_info,
const GlobalRequestID& request_id,
bool should_replace_current_entry,
bool is_download,
@@ -767,7 +764,7 @@
is_download_ = is_download;
is_stream_ = is_stream;
state_ = WILL_PROCESS_RESPONSE;
- ssl_status_ = ssl_status;
+ ssl_info_ = ssl_info;
socket_address_ = socket_address;
complete_callback_ = callback;
transfer_callback_ = transfer_callback;
diff --git a/content/browser/frame_host/navigation_handle_impl.h b/content/browser/frame_host/navigation_handle_impl.h
index fc07092..fe65b9e9 100644
--- a/content/browser/frame_host/navigation_handle_impl.h
+++ b/content/browser/frame_host/navigation_handle_impl.h
@@ -26,15 +26,10 @@
#include "content/public/browser/navigation_throttle.h"
#include "content/public/browser/navigation_type.h"
#include "content/public/browser/restore_type.h"
-#include "content/public/browser/ssl_status.h"
#include "content/public/common/request_context_type.h"
#include "third_party/WebKit/public/platform/WebMixedContentContextType.h"
#include "url/gurl.h"
-namespace net {
-class SSLInfo;
-} // namespace net
-
struct FrameHostMsg_DidCommitProvisionalLoad_Params;
namespace content {
@@ -148,7 +143,7 @@
net::HostPortPair GetSocketAddress() override;
const net::HttpResponseHeaders* GetResponseHeaders() override;
net::HttpResponseInfo::ConnectionInfo GetConnectionInfo() override;
- const base::Optional<net::SSLInfo>& GetSSLInfo() override;
+ const net::SSLInfo& GetSSLInfo() override;
bool ShouldSSLErrorsBeFatal() override;
void RegisterThrottleForTesting(
std::unique_ptr<NavigationThrottle> navigation_throttle) override;
@@ -335,7 +330,7 @@
scoped_refptr<net::HttpResponseHeaders> response_headers,
net::HttpResponseInfo::ConnectionInfo connection_info,
const net::HostPortPair& socket_address,
- const SSLStatus& ssl_status,
+ const net::SSLInfo& ssl_info,
const GlobalRequestID& request_id,
bool should_replace_current_entry,
bool is_download,
@@ -372,8 +367,6 @@
navigation_data_ = std::move(navigation_data);
}
- SSLStatus ssl_status() { return ssl_status_; }
-
// Called when the navigation is transferred to a different renderer.
void Transfer();
@@ -501,7 +494,7 @@
bool subframe_entry_committed_;
scoped_refptr<net::HttpResponseHeaders> response_headers_;
net::HttpResponseInfo::ConnectionInfo connection_info_;
- base::Optional<net::SSLInfo> ssl_info_;
+ net::SSLInfo ssl_info_;
bool should_ssl_errors_be_fatal_;
// The original url of the navigation. This may differ from |url_| if the
@@ -578,8 +571,6 @@
// Embedder data from the UI thread tied to this navigation.
std::unique_ptr<NavigationUIData> navigation_ui_data_;
- SSLStatus ssl_status_;
-
// The unique id to identify this to navigation with.
int64_t navigation_id_;
diff --git a/content/browser/frame_host/navigation_handle_impl_unittest.cc b/content/browser/frame_host/navigation_handle_impl_unittest.cc
index f56dbf3..5f7e8cb 100644
--- a/content/browser/frame_host/navigation_handle_impl_unittest.cc
+++ b/content/browser/frame_host/navigation_handle_impl_unittest.cc
@@ -172,7 +172,7 @@
test_handle_->WillProcessResponse(
main_test_rfh(), scoped_refptr<net::HttpResponseHeaders>(),
net::HttpResponseInfo::CONNECTION_INFO_QUIC_35, net::HostPortPair(),
- SSLStatus(), GlobalRequestID(), false, false, false, base::Closure(),
+ net::SSLInfo(), GlobalRequestID(), false, false, false, base::Closure(),
base::Bind(&NavigationHandleImplTest::UpdateThrottleCheckResult,
base::Unretained(this)));
}
@@ -1143,8 +1143,8 @@
}
// Checks that data from the SSLInfo passed into SimulateWillStartRequest() is
-// stored on the handle's SSLStatus.
-TEST_F(NavigationHandleImplTest, WillFailRequestSetsSSLStatus) {
+// stored on the handle.
+TEST_F(NavigationHandleImplTest, WillFailRequestSetsSSLInfo) {
uint16_t cipher_suite = 0xc02f; // TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
int connection_status = 0;
net::SSLConnectionStatusSetCipherSuite(cipher_suite, &connection_status);
@@ -1158,8 +1158,8 @@
SimulateWillFailRequest(net::ERR_CERT_DATE_INVALID, ssl_info);
EXPECT_EQ(net::CERT_STATUS_AUTHORITY_INVALID,
- test_handle()->ssl_status().cert_status);
- EXPECT_EQ(connection_status, test_handle()->ssl_status().connection_status);
+ test_handle()->GetSSLInfo().cert_status);
+ EXPECT_EQ(connection_status, test_handle()->GetSSLInfo().connection_status);
}
} // namespace content
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
index 213edb7..44ef0a12 100644
--- a/content/browser/frame_host/navigation_request.cc
+++ b/content/browser/frame_host/navigation_request.cc
@@ -733,7 +733,7 @@
const scoped_refptr<ResourceResponse>& response,
std::unique_ptr<StreamHandle> body,
mojo::ScopedDataPipeConsumerHandle consumer_handle,
- const SSLStatus& ssl_status,
+ const net::SSLInfo& ssl_info,
std::unique_ptr<NavigationData> navigation_data,
const GlobalRequestID& request_id,
bool is_download,
@@ -810,7 +810,7 @@
response_ = response;
body_ = std::move(body);
handle_ = std::move(consumer_handle);
- ssl_status_ = ssl_status;
+ ssl_info_ = ssl_info;
is_download_ = is_download;
subresource_loader_params_ = std::move(subresource_loader_params);
@@ -847,7 +847,7 @@
// Check if the navigation should be allowed to proceed.
navigation_handle_->WillProcessResponse(
render_frame_host, response->head.headers.get(),
- response->head.connection_info, response->head.socket_address, ssl_status,
+ response->head.connection_info, response->head.socket_address, ssl_info_,
request_id, common_params_.should_replace_current_entry, is_download,
is_stream, base::Closure(),
base::Bind(&NavigationRequest::OnWillProcessResponseChecksComplete,
@@ -1135,7 +1135,7 @@
BrowserContext::GetDownloadManager(browser_context));
loader_->InterceptNavigation(
download_manager->GetNavigationInterceptionCB(
- response_, std::move(handle_), ssl_status_,
+ response_, std::move(handle_), ssl_info_.cert_status,
frame_tree_node_->frame_tree_node_id()));
OnRequestFailed(false, net::ERR_ABORTED, base::nullopt, false);
return;
diff --git a/content/browser/frame_host/navigation_request.h b/content/browser/frame_host/navigation_request.h
index ee448ef7..2cddf27 100644
--- a/content/browser/frame_host/navigation_request.h
+++ b/content/browser/frame_host/navigation_request.h
@@ -222,7 +222,7 @@
void OnResponseStarted(const scoped_refptr<ResourceResponse>& response,
std::unique_ptr<StreamHandle> body,
mojo::ScopedDataPipeConsumerHandle consumer_handle,
- const SSLStatus& ssl_status,
+ const net::SSLInfo& ssl_info,
std::unique_ptr<NavigationData> navigation_data,
const GlobalRequestID& request_id,
bool is_download,
@@ -352,7 +352,7 @@
scoped_refptr<ResourceResponse> response_;
std::unique_ptr<StreamHandle> body_;
mojo::ScopedDataPipeConsumerHandle handle_;
- SSLStatus ssl_status_;
+ net::SSLInfo ssl_info_;
bool is_download_;
// Holds information for the navigation while the WillFailRequest
