Switch //components to use SchemeIsCryptographic() instead of SchemeIsSecure().

Switch //components to use SchemeIsCryptographic() instead of SchemeIsSecure().

We recently introduced SchemeIsCryptographic() and IsOriginSecure(),
which are meant to replace SchemeIsSecure().

IsOriginSecure() roughly means "do we trust this content not to be
tampered with before it reaches the user?" [1] This is a higher-level
definition that corresponds to the new "privileged contexts" spec. [2]

SchemeIsCryptographic() [3] is close to the old definition of
SchemeIsSecure(), and literally just checks if the scheme is a
cryptographic scheme (HTTPS or WSS as of right now). The difference is
that SchemeIsCryptographic() will not consider filesystem URLs secure.

[1] https://code.google.com/p/chromium/codesearch#chromium/src/content/public/common/origin_util.h&sq=package:chromium&type=cs&l=19&rcl=143099866
[2] https://www.chromium.org/Home/chromium-security/prefer-secure-origins-for-powerful-new-features and https://w3c.github.io/webappsec/specs/powerfulfeatures/
[3] https://code.google.com/p/chromium/codesearch#chromium/src/url/gurl.h&sq=package:chromium&type=cs&l=250&rcl=1430998666

BUG=362214
Committed: https://crrev.com/a2da0cfdd55a53963a5298f301b896bdb909750e
Cr-Commit-Position: refs/heads/master@{#329481}

[lgarron, 2015-05-08 02:10:41]

lgarron@chromium.org changed reviewers:
+ mmenke@chromium.org, sclittle@chromium.org

[lgarron, 2015-05-08 02:10:42]

sclittle@chromium.org: Could you review the data reduction proxy code?
mmenke@chromium.org: Could you review
components/error_page/renderer/net_error_helper_core.cc?

See the description for the semantics of the change.

https://codereview.chromium.org/1135693002/diff/1/components/error_page/rende...
File components/error_page/renderer/net_error_helper_core.cc (right):

https://codereview.chromium.org/1135693002/diff/1/components/error_page/rende...
components/error_page/renderer/net_error_helper_core.cc:153: if
(GURL(unreachable_url).SchemeIsCryptographic())
mmenke@: I don't think we specifically need to block this for, say, localhost,
extensions and file URLs. Do you agree, or would you prefer to fail safer?

[mmenke, 2015-05-08 02:49:36]

error_page LGTM

https://codereview.chromium.org/1135693002/diff/1/components/error_page/rende...
File components/error_page/renderer/net_error_helper_core.cc (right):

https://codereview.chromium.org/1135693002/diff/1/components/error_page/rende...
components/error_page/renderer/net_error_helper_core.cc:153: if
(GURL(unreachable_url).SchemeIsCryptographic())
On 2015/05/08 02:10:42, lgarron wrote:
> mmenke@: I don't think we specifically need to block this for, say, localhost,
> extensions and file URLs. Do you agree, or would you prefer to fail safer?

Extensions and file URLs will return false because of the checks on error.reason
below, anyways.

Does seem weird to call home for anything other than HTTP though - maybe this
should really just be !GURL(unreachable_url).SchemeIs(url::kHttpScheme).  I
don't see any issues with SchemeIsCryptographic(), but I think it's best to be
paranoid.  Anyhow, that change seems beyond the scope of this CL, regardless.

[lgarron, 2015-05-12 02:16:10]

On 2015/05/08 02:49:36, mmenke wrote:
> error_page LGTM
> 
>
https://codereview.chromium.org/1135693002/diff/1/components/error_page/rende...
> File components/error_page/renderer/net_error_helper_core.cc (right):
> 
>
https://codereview.chromium.org/1135693002/diff/1/components/error_page/rende...
> components/error_page/renderer/net_error_helper_core.cc:153: if
> (GURL(unreachable_url).SchemeIsCryptographic())
> On 2015/05/08 02:10:42, lgarron wrote:
> > mmenke@: I don't think we specifically need to block this for, say,
localhost,
> > extensions and file URLs. Do you agree, or would you prefer to fail safer?
> 
> Extensions and file URLs will return false because of the checks on
error.reason
> below, anyways.
> 
> Does seem weird to call home for anything other than HTTP though - maybe this
> should really just be !GURL(unreachable_url).SchemeIs(url::kHttpScheme).  I
> don't see any issues with SchemeIsCryptographic(), but I think it's best to be
> paranoid.  Anyhow, that change seems beyond the scope of this CL, regardless.

sclittle@: Poke. :-)

[sclittle, 2015-05-12 17:21:31]

https://codereview.chromium.org/1135693002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc
(right):

https://codereview.chromium.org/1135693002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc:588:
if (url.SchemeIsCryptographic()) {
There's actually one more place that SchemeIsSecure is used in the drp
component, I actually just added it yesterday (sorry (-:):

https://code.google.com/p/chromium/codesearch#chromium/src/components/data_re...

[lgarron, 2015-05-12 18:30:35]

https://codereview.chromium.org/1135693002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc
(right):

https://codereview.chromium.org/1135693002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc:588:
if (url.SchemeIsCryptographic()) {
On 2015/05/12 17:21:31, sclittle wrote:
> There's actually one more place that SchemeIsSecure is used in the drp
> component, I actually just added it yesterday (sorry (-:):
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/components/data_re...

No problem. :-)

(Done. Thanks for letting me know.)

[sclittle, 2015-05-12 18:31:17]

LGTM

[lgarron, 2015-05-12 18:34:37]

The CQ bit was checked by lgarron@chromium.org

[lgarron, 2015-05-12 18:34:37]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/1135693002/#ps20001
(title: "Include new call.")

[commit-bot: I haz the power, 2015-05-12 18:36:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1135693002/20001

[commit-bot: I haz the power, 2015-05-12 20:52:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-12 20:52:51]

Patchset 2 (id:??) landed as
https://crrev.com/a2da0cfdd55a53963a5298f301b896bdb909750e
Cr-Commit-Position: refs/heads/master@{#329481}