Switch //net functions to use SchemeIsCryptographic() instead of SchemeIsSecure().

net/base/sdch_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/base/sdch_manager.h"
 
 #include "base/base64.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 168 matching lines @@
   if (blacklisted_domains_.end() == blacklisted_domains_.find(domain_lower))
     return 0;
   return blacklisted_domains_[domain_lower].exponential_count;
 }
 
 SdchProblemCode SdchManager::IsInSupportedDomain(const GURL& url) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!g_sdch_enabled_ )
     return SDCH_DISABLED;
 
-  if (!secure_scheme_supported() && url.SchemeIsSecure())
+  if (!secure_scheme_supported() && url.SchemeIsCryptographic())

[asanka, 2015/05/11 22:00:46]

This is a bit confusing, but the intent here appears to be to disable SDCH for
everything other than HTTP. If so, I think we should make that explicit. I.e.:

  - rename secure_scheme_supported() to something like support_http_only()
  - change the condition to url.SchemeIs(url::kHttpScheme)

Here and elsewhere.

+rdsmith to verify.

[lgarron, 2015/05/11 23:23:47]

I started splitting this into a separate CL, but I wasn't sure how to deal with
replacing secure_scheme_supported with its negative.

static void EnableSecureSchemeSupport(bool enabled); [1] is called in several
places. What would you think of preserving EnableSecureSchemeSupport but
replacing secure_scheme_supported with support_http_only.

[1]
https://code.google.com/p/chromium/codesearch#search/&q=EnableSecureSchemeSup...

[Randy Smith (Not in Mondays), 2015/05/12 14:08:04]

Yes, that's the intent.  I'm not sure the conditioning is needed anymore,
though; that condition was in place while we were tentatively rolling SDCH out
first for HTTP then for HTTPS; it might be reasonable to eliminate the ability
to distinguish between the two schemes in the context of SDCH at this point.

[Elly Fong-Jones, 2015/05/12 15:26:31]

I would drop support for disabling SDCH for secure schemes (ie, remove SDCH's
awareness of whether the protocol is secure/cryptographic or not).

[asanka, 2015/05/12 17:00:17]

I'd wait till rdsmith confirms whether the suggested change matches the intent.

Regarding the callers: The only non-test caller is io_thread.cc which calls it
to disable secure scheme support when the SDCH Finch trial is
"EnabledHttpOnly*". I think changing the semantics of the call to enable SDCH
over HTTP only is consistent with the intent of the callsite.

[asanka, 2015/05/12 17:00:17]

ellyjones, rdsmith: Cool. Thanks for confirming!

I feel that a change to make SDCH support unaware of the properties of the
transport is likely a bit involved, and should probably be handled by //net
folks. lgarron is welcome to take a whack at it, but I don't feel right about
pulling him into doing the work.

lgarron: SchemeIsSecure() and SchemeIsCryptographic() are semantically
equivalent in the proposed changes to SDCH. If that unblocks your
SchemeIsSecure() cleanup, then AFAIAC you can land the changes as proposed and
then a follow-up CL (either by you or someone else) can remove the checks
entirely along with associated cleanup.

... this is assuming ellyjones is okay with this plan.

     return SDCH_SECURE_SCHEME_NOT_SUPPORTED;
 
   if (blacklisted_domains_.empty())
     return SDCH_OK;
 
   DomainBlacklistInfo::iterator it =
       blacklisted_domains_.find(base::StringToLowerASCII(url.host()));
   if (blacklisted_domains_.end() == it || it->second.count == 0)
     return SDCH_OK;
 
@@ skipping 61 matching lines @@
 }
 
 scoped_ptr<SdchManager::DictionarySet>
 SdchManager::GetDictionarySet(const GURL& target_url) {
   if (IsInSupportedDomain(target_url) != SDCH_OK)
     return NULL;
 
   int count = 0;
   scoped_ptr<SdchManager::DictionarySet> result(new DictionarySet);
   for (const auto& entry: dictionaries_) {
-    if (!secure_scheme_supported() && target_url.SchemeIsSecure())
+    if (!secure_scheme_supported() && target_url.SchemeIsCryptographic())
       continue;
     if (entry.second->data.CanUse(target_url) != SDCH_OK)
       continue;
     if (entry.second->data.Expired())
       continue;
     ++count;
     result->AddDictionary(entry.first, entry.second);
   }
 
   if (count == 0)
@@ skipping 10 matching lines @@
     const std::string& server_hash,
     SdchProblemCode* problem_code) {
   scoped_ptr<SdchManager::DictionarySet> result;
 
   *problem_code = SDCH_DICTIONARY_HASH_NOT_FOUND;
   const auto& it = dictionaries_.find(server_hash);
   if (it == dictionaries_.end())
     return result.Pass();
 
   if (!SdchManager::secure_scheme_supported() &&
-      target_url.SchemeIsSecure()) {
+      target_url.SchemeIsCryptographic()) {
     *problem_code = SDCH_DICTIONARY_FOUND_HAS_WRONG_SCHEME;
     return result.Pass();
   }
 
   *problem_code = it->second->data.CanUse(target_url);
   if (*problem_code != SDCH_OK)
     return result.Pass();
 
   result.reset(new DictionarySet);
   result->AddDictionary(it->first, it->second);
@@ skipping 204 matching lines @@
       entry_dict->SetInteger("tries", it->second.count);
     entry_dict->SetInteger("reason", it->second.reason);
     entry_list->Append(entry_dict);
   }
   value->Set("blacklisted", entry_list);
 
   return value;
 }
 
 }  // namespace net