Skip to content

Commit

Permalink
Rename GURL::SchemeUsesTLS to SchemeIsCryptographic.
Browse files Browse the repository at this point in the history
Since there will increasingly be things like QUIC that offer similar guarantees
but which are not precisely TLS.

Per discussion in https://codereview.chromium.org/1049533002/.

BUG=362214
TBR=brettw,thestig@chromium.org,isherman@chromium.org,mmenke@chromium.org,abarth@chromium.org

Review URL: https://codereview.chromium.org/1049953003

Cr-Commit-Position: refs/heads/master@{#326236}
  • Loading branch information
palmer authored and Commit bot committed Apr 22, 2015
1 parent 1562114 commit 12b415b
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 32 deletions.
6 changes: 3 additions & 3 deletions chrome/browser/captive_portal/captive_portal_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ void CaptivePortalTabHelper::DidStartProvisionalLoadForFrame(
provisional_render_view_host_ = render_view_host;
pending_error_code_ = net::OK;

tab_reloader_->OnLoadStart(validated_url.SchemeUsesTLS());
tab_reloader_->OnLoadStart(validated_url.SchemeIsCryptographic());
}

void CaptivePortalTabHelper::DidCommitProvisionalLoadForFrame(
Expand All @@ -115,7 +115,7 @@ void CaptivePortalTabHelper::DidCommitProvisionalLoadForFrame(
OnLoadAborted();

// Send information about the new load.
tab_reloader_->OnLoadStart(url.SchemeUsesTLS());
tab_reloader_->OnLoadStart(url.SchemeIsCryptographic());
tab_reloader_->OnLoadCommitted(net::OK);
}

Expand Down Expand Up @@ -239,7 +239,7 @@ void CaptivePortalTabHelper::OnRedirect(int child_id,
return;
}

tab_reloader_->OnRedirect(new_url.SchemeUsesTLS());
tab_reloader_->OnRedirect(new_url.SchemeIsCryptographic());
}

void CaptivePortalTabHelper::OnCaptivePortalResults(
Expand Down
39 changes: 23 additions & 16 deletions chrome/browser/captive_portal/captive_portal_tab_helper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ class CaptivePortalTabHelperTest : public ChromeRenderViewHostTestHarness {
// Simulates a successful load of |url|.
void SimulateSuccess(const GURL& url,
content::RenderViewHost* render_view_host) {
EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeUsesTLS())).Times(1);
EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeIsCryptographic()))
.Times(1);
tab_helper().DidStartProvisionalLoadForFrame(
render_view_host->GetMainFrame(), url, false, false);

Expand All @@ -101,7 +102,8 @@ class CaptivePortalTabHelperTest : public ChromeRenderViewHostTestHarness {
// Simulates a connection timeout while requesting |url|.
void SimulateTimeout(const GURL& url,
content::RenderViewHost* render_view_host) {
EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeUsesTLS())).Times(1);
EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeIsCryptographic()))
.Times(1);
tab_helper().DidStartProvisionalLoadForFrame(
render_view_host->GetMainFrame(), url, false, false);

Expand All @@ -125,7 +127,8 @@ class CaptivePortalTabHelperTest : public ChromeRenderViewHostTestHarness {
void SimulateAbort(const GURL& url,
content::RenderViewHost* render_view_host,
NavigationType navigation_type) {
EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeUsesTLS())).Times(1);
EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeIsCryptographic()))
.Times(1);
tab_helper().DidStartProvisionalLoadForFrame(
render_view_host->GetMainFrame(), url, false, false);

Expand All @@ -151,7 +154,8 @@ class CaptivePortalTabHelperTest : public ChromeRenderViewHostTestHarness {
void SimulateAbortTimeout(const GURL& url,
content::RenderViewHost* render_view_host,
NavigationType navigation_type) {
EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeUsesTLS())).Times(1);
EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeIsCryptographic()))
.Times(1);
tab_helper().DidStartProvisionalLoadForFrame(
render_view_host->GetMainFrame(), url, false, false);

Expand Down Expand Up @@ -334,7 +338,7 @@ TEST_F(CaptivePortalTabHelperTest, UnexpectedProvisionalLoad) {

// A same-site load for the original RenderViewHost starts.
EXPECT_CALL(mock_reloader(),
OnLoadStart(same_site_url.SchemeUsesTLS())).Times(1);
OnLoadStart(same_site_url.SchemeIsCryptographic())).Times(1);
tab_helper().DidStartProvisionalLoadForFrame(
main_render_frame1(), same_site_url, false, false);

Expand All @@ -343,7 +347,7 @@ TEST_F(CaptivePortalTabHelperTest, UnexpectedProvisionalLoad) {
// for the old navigation.
EXPECT_CALL(mock_reloader(), OnAbort()).Times(1);
EXPECT_CALL(mock_reloader(),
OnLoadStart(cross_process_url.SchemeUsesTLS())).Times(1);
OnLoadStart(cross_process_url.SchemeIsCryptographic())).Times(1);
tab_helper().DidStartProvisionalLoadForFrame(
main_render_frame2(), cross_process_url, false, false);

Expand Down Expand Up @@ -378,7 +382,7 @@ TEST_F(CaptivePortalTabHelperTest, UnexpectedCommit) {

// A same-site load for the original RenderViewHost starts.
EXPECT_CALL(mock_reloader(),
OnLoadStart(same_site_url.SchemeUsesTLS())).Times(1);
OnLoadStart(same_site_url.SchemeIsCryptographic())).Times(1);
tab_helper().DidStartProvisionalLoadForFrame(
main_render_frame1(), same_site_url, false, false);

Expand All @@ -387,7 +391,7 @@ TEST_F(CaptivePortalTabHelperTest, UnexpectedCommit) {
// for the old navigation.
EXPECT_CALL(mock_reloader(), OnAbort()).Times(1);
EXPECT_CALL(mock_reloader(),
OnLoadStart(cross_process_url.SchemeUsesTLS())).Times(1);
OnLoadStart(cross_process_url.SchemeIsCryptographic())).Times(1);
tab_helper().DidStartProvisionalLoadForFrame(
main_render_frame2(), cross_process_url, false, false);

Expand All @@ -400,7 +404,7 @@ TEST_F(CaptivePortalTabHelperTest, UnexpectedCommit) {
// The same-site navigation succeeds.
EXPECT_CALL(mock_reloader(), OnAbort()).Times(1);
EXPECT_CALL(mock_reloader(),
OnLoadStart(same_site_url.SchemeUsesTLS())).Times(1);
OnLoadStart(same_site_url.SchemeIsCryptographic())).Times(1);
EXPECT_CALL(mock_reloader(), OnLoadCommitted(net::OK)).Times(1);
tab_helper().DidCommitProvisionalLoadForFrame(
main_render_frame1(), same_site_url, ui::PAGE_TRANSITION_LINK);
Expand Down Expand Up @@ -450,7 +454,8 @@ TEST_F(CaptivePortalTabHelperTest, HttpsSubframeParallelError) {
->AppendChild("subframe");

// Loads start.
EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeUsesTLS())).Times(1);
EXPECT_CALL(mock_reloader(), OnLoadStart(url.SchemeIsCryptographic()))
.Times(1);
tab_helper().DidStartProvisionalLoadForFrame(
main_render_frame1(), url, false, false);
tab_helper().DidStartProvisionalLoadForFrame(subframe, url, false, false);
Expand Down Expand Up @@ -505,13 +510,14 @@ TEST_F(CaptivePortalTabHelperTest, HttpToHttpsRedirectTimeout) {
// Simulates an HTTPS to HTTP redirect.
TEST_F(CaptivePortalTabHelperTest, HttpsToHttpRedirect) {
GURL https_url(kHttpsUrl);
EXPECT_CALL(mock_reloader(),
OnLoadStart(https_url.SchemeUsesTLS())).Times(1);
EXPECT_CALL(mock_reloader(), OnLoadStart(https_url.SchemeIsCryptographic()))
.Times(1);
tab_helper().DidStartProvisionalLoadForFrame(
main_render_frame1(), https_url, false, false);

GURL http_url(kHttpUrl);
EXPECT_CALL(mock_reloader(), OnRedirect(http_url.SchemeUsesTLS())).Times(1);
EXPECT_CALL(mock_reloader(), OnRedirect(http_url.SchemeIsCryptographic()))
.Times(1);
OnRedirect(content::RESOURCE_TYPE_MAIN_FRAME, http_url,
render_view_host1()->GetProcess()->GetID());

Expand All @@ -523,12 +529,13 @@ TEST_F(CaptivePortalTabHelperTest, HttpsToHttpRedirect) {
// Simulates an HTTPS to HTTPS redirect.
TEST_F(CaptivePortalTabHelperTest, HttpToHttpRedirect) {
GURL http_url(kHttpUrl);
EXPECT_CALL(mock_reloader(),
OnLoadStart(http_url.SchemeUsesTLS())).Times(1);
EXPECT_CALL(mock_reloader(), OnLoadStart(http_url.SchemeIsCryptographic()))
.Times(1);
tab_helper().DidStartProvisionalLoadForFrame(
main_render_frame1(), http_url, false, false);

EXPECT_CALL(mock_reloader(), OnRedirect(http_url.SchemeUsesTLS())).Times(1);
EXPECT_CALL(mock_reloader(), OnRedirect(http_url.SchemeIsCryptographic()))
.Times(1);
OnRedirect(content::RESOURCE_TYPE_MAIN_FRAME, http_url,
render_view_host1()->GetProcess()->GetID());

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/net/certificate_error_reporter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ void CertificateErrorReporter::SendReport(ReportType type,
break;
case REPORT_TYPE_EXTENDED_REPORTING:
// TODO(estark): Encrypt the report if not sending over HTTPS.
DCHECK(upload_url_.SchemeUsesTLS());
DCHECK(upload_url_.SchemeIsCryptographic());
SendCertLoggerRequest(request);
break;
default:
Expand Down
2 changes: 1 addition & 1 deletion chrome/common/origin_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "url/gurl.h"

bool IsOriginSecure(const GURL& url) {
if (url.SchemeUsesTLS() || url.SchemeIsFile())
if (url.SchemeIsCryptographic() || url.SchemeIsFile())
return true;

if (url.SchemeIsFileSystem() && url.inner_url() &&
Expand Down
2 changes: 1 addition & 1 deletion chrome/utility/importer/ie_importer_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ void IEImporter::ImportPasswordsIE6() {
// import a password from IE whose scheme is https, we give it the benefit
// of the doubt and DON'T auto-fill it unless the form appears under
// valid TLS conditions.
form.ssl_valid = url.SchemeUsesTLS();
form.ssl_valid = url.SchemeIsCryptographic();

// Goes through the list to find out the username field
// of the web page.
Expand Down
4 changes: 2 additions & 2 deletions chrome/utility/importer/nss_decryptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ void NSSDecryptor::ParseSignons(
form.signon_realm = form.origin.GetOrigin().spec();
if (!realm.empty())
form.signon_realm += realm;
form.ssl_valid = form.origin.SchemeUsesTLS();
form.ssl_valid = form.origin.SchemeIsCryptographic();
++begin;

// There may be multiple username/password pairs for this site.
Expand Down Expand Up @@ -296,7 +296,7 @@ bool NSSDecryptor::ReadAndParseSignons(const base::FilePath& sqlite_file,
// digest_auth entry, so let's assume basic_auth.
form.scheme = autofill::PasswordForm::SCHEME_BASIC;
}
form.ssl_valid = form.origin.SchemeUsesTLS();
form.ssl_valid = form.origin.SchemeIsCryptographic();
// The user name, password and action.
form.username_element = s2.ColumnString16(3);
form.username_value = Decrypt(s2.ColumnString(5));
Expand Down
17 changes: 9 additions & 8 deletions url/gurl.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,13 @@ class URL_EXPORT GURL {

// Returns true if the scheme indicates a secure connection.
//
// NOTE: This function is deprecated. You probably want |SchemeUsesTLS| (if
// you just want to know if a scheme uses TLS for network transport) or
// Chromium's |IsOriginSecure| for a higher-level test about an origin's
// security. See those functions' documentation for more detail.
// NOTE: This function is deprecated. You probably want
// |SchemeIsCryptographic| (if you just want to know if a scheme uses TLS for
// network transport) or Chromium's |IsOriginSecure| for a higher-level test
// about an origin's security. See those functions' documentation for more
// detail.
//
// TODO(palmer): Audit callers and change them to |SchemeUsesTLS| or
// TODO(palmer): Audit callers and change them to |SchemeIsCryptographic| or
// |IsOriginSecure|, as appropriate. Then remove |SchemeIsSecure|.
// crbug.com/362214
bool SchemeIsSecure() const {
Expand All @@ -239,14 +240,14 @@ class URL_EXPORT GURL {
inner_url()->SchemeIsSecure());
}

// Returns true if the scheme indicates a network connection that uses TLS for
// security.
// Returns true if the scheme indicates a network connection that uses TLS or
// some other cryptographic protocol (e.g. QUIC) for security.
//
// This function is a not a complete test of whether or not an origin's code
// is minimally trustworthy. For that, see Chromium's |IsOriginSecure| for a
// higher-level and more complete semantics. See that function's documentation
// for more detail.
bool SchemeUsesTLS() const {
bool SchemeIsCryptographic() const {
return SchemeIs(url::kHttpsScheme) || SchemeIs(url::kWssScheme);
}

Expand Down

0 comments on commit 12b415b

Please sign in to comment.