Don't show scheme in permission prompts if it is HTTPS

Don't show scheme in permission prompts if it is HTTPS

HTTPS is nice and safe, so doesn't need to be called out in permission prompts. This change should also make origin display for permission prompts consistent across Android, Mac and Views.

BUG=596488
Committed: https://crrev.com/2337b81057c1ac85098834ad75d250e7d87e6a39
Cr-Commit-Position: refs/heads/master@{#388404}

[benwells, 2016-03-30 07:00:24]

benwells@chromium.org changed reviewers:
+ felt@chromium.org, palmer@chromium.org

[benwells, 2016-03-30 07:00:25]

felt / palmer:

This isn't complete (tests will break, android infobars), but thought I'd get
your opinions.

Maybe this is too aggressive. If it is, I can just introduce one more method to
format URLs ...

[palmer, 2016-03-30 20:23:34]

I think this might break valid use cases. It might be better to add a 3rd
function, e.g. FormatUrlForSecurityDisplayOmitCryptographicScheme (or something
slightly less awful, if possible).

https://codereview.chromium.org/1843063002/diff/1/components/url_formatter/el...
File components/url_formatter/elide_url.cc (right):

https://codereview.chromium.org/1843063002/diff/1/components/url_formatter/el...
components/url_formatter/elide_url.cc:143: if (!omit_scheme ||
!url.SchemeIs(url::kHttpsScheme))
Maybe url.SchemeIsCryptographic()?

[benwells, 2016-03-30 23:53:20]

On 2016/03/30 20:23:34, palmer wrote:
> I think this might break valid use cases. 

Yeah, that's what I was worried about too. For my info do you have anything
specific in mind, or is it more of a general worry?

> It might be better to add a 3rd
> function, e.g. FormatUrlForSecurityDisplayOmitCryptographicScheme (or
something
> slightly less awful, if possible).

If we do that I thinkI'd prefer to move to just one function, but give it an
enum parameter saying what to do.

> 
>
https://codereview.chromium.org/1843063002/diff/1/components/url_formatter/el...
> File components/url_formatter/elide_url.cc (right):
> 
>
https://codereview.chromium.org/1843063002/diff/1/components/url_formatter/el...
> components/url_formatter/elide_url.cc:143: if (!omit_scheme ||
> !url.SchemeIs(url::kHttpsScheme))
> Maybe url.SchemeIsCryptographic()?

[palmer, 2016-04-01 18:55:49]

> > I think this might break valid use cases. 
> 
> Yeah, that's what I was worried about too. For my info do you have anything
> specific in mind, or is it more of a general worry?

I can't think of anything specific off the top of my head, but I bet searching
the code for callers will turn up at least 1.

> > It might be better to add a 3rd
> > function, e.g. FormatUrlForSecurityDisplayOmitCryptographicScheme (or
> something
> > slightly less awful, if possible).
> 
> If we do that I thinkI'd prefer to move to just one function, but give it an
> enum parameter saying what to do.

Sure.

[benwells, 2016-04-14 01:26:54]

Description was changed from

==========
When showing origins and omitting scheme, show scheme if it is HTTP.

This highlights HTTP and hides HTTPS, which is secure. This change
affects the following surfaces:
- origin info bubble
- download notifications
- push messaging notifications (which should all be HTTPS)
- javascript dialogs
- passwords ui
- Android single website preferences
- Android web app URL bars (when web apps navigate to another frame)
- Android media controls

Permission prompts now also omit the scheme, as long as the scheme is
HTTPS.

BUG=596488
==========

to

==========
HTTPS is nice and safe, so doesn't need to be called out in permission prompts.
This change should also make origin display for permission prompts consistent
across Android, Mac and Views.

BUG=596488
==========

[benwells, 2016-04-14 01:27:13]

Description was changed from

==========
HTTPS is nice and safe, so doesn't need to be called out in permission prompts.
This change should also make origin display for permission prompts consistent
across Android, Mac and Views.

BUG=596488
==========

to

==========
Don't show scheme in permission prompts if it is HTTPS

HTTPS is nice and safe, so doesn't need to be called out in permission prompts.
This change should also make origin display for permission prompts consistent
across Android, Mac and Views.

BUG=596488
==========

[benwells, 2016-04-14 01:27:58]

OK, done more safely now. This change looks large but it is mostly refactoring
cruft.

[benwells, 2016-04-14 06:15:05]

Description was changed from

==========
Don't show scheme in permission prompts if it is HTTPS

HTTPS is nice and safe, so doesn't need to be called out in permission prompts.
This change should also make origin display for permission prompts consistent
across Android, Mac and Views.

BUG=596488
==========

to

==========
Don't show scheme in permission prompts if it is HTTPS

HTTPS is nice and safe, so doesn't need to be called out in permission prompts.
This change should also make origin display for permission prompts consistent
across Android, Mac and Views.

BUG=596488, 601695
==========

[palmer, 2016-04-15 00:53:29]

lgtm

https://codereview.chromium.org/1843063002/diff/80001/components/url_formatte...
File components/url_formatter/elide_url.h (right):

https://codereview.chromium.org/1843063002/diff/80001/components/url_formatte...
components/url_formatter/elide_url.h:71: // Generally, set prefer
SchemeDisplay::SHOW to omitting the scheme unless there
Typo in my original test: remove "set".

[benwells, 2016-04-18 00:51:32]

Description was changed from

==========
Don't show scheme in permission prompts if it is HTTPS

HTTPS is nice and safe, so doesn't need to be called out in permission prompts.
This change should also make origin display for permission prompts consistent
across Android, Mac and Views.

BUG=596488, 601695
==========

to

==========
Don't show scheme in permission prompts if it is HTTPS

HTTPS is nice and safe, so doesn't need to be called out in permission prompts.
This change should also make origin display for permission prompts consistent
across Android, Mac and Views.

BUG=596488
==========

[benwells, 2016-04-18 00:53:05]

felt - do you wanna review any of this before I send to an uber owner?

https://codereview.chromium.org/1843063002/diff/80001/components/url_formatte...
File components/url_formatter/elide_url.h (right):

https://codereview.chromium.org/1843063002/diff/80001/components/url_formatte...
components/url_formatter/elide_url.h:71: // Generally, set prefer
SchemeDisplay::SHOW to omitting the scheme unless there
On 2016/04/15 00:53:29, palmer wrote:
> Typo in my original test: remove "set".

Done.

[felt, 2016-04-18 01:16:26]

nice refactor, lgtm % comment

https://codereview.chromium.org/1843063002/diff/100001/components/url_formatt...
File components/url_formatter/elide_url.h (right):

https://codereview.chromium.org/1843063002/diff/100001/components/url_formatt...
components/url_formatter/elide_url.h:54: OMIT_CRYPTOGRAPHIC,
nit: The meanings of these names are not obvious to me, especially OMIT_WEB. Can
you either make the names clearer or add comments?

[benwells, 2016-04-18 04:00:05]

benwells@chromium.org changed reviewers:
+ caitkp@chromium.org

[benwells, 2016-04-18 04:00:06]

+thakis for chrome/ and ui/
+caitkp for components/app_modal and components/password_manager

[benwells, 2016-04-18 04:00:35]

benwells@chromium.org changed reviewers:
+ thakis@chromium.org

[benwells, 2016-04-18 04:00:36]

actually +thakis for chrome/ and ui/

[Cait (Slow), 2016-04-19 15:05:55]

components/ lgtm

[Nico, 2016-04-19 15:11:16]

lgtm

[benwells, 2016-04-19 21:53:24]

The CQ bit was checked by benwells@chromium.org

[benwells, 2016-04-19 21:53:25]

The patchset sent to the CQ was uploaded after l-g-t-m from palmer@chromium.org,
felt@chromium.org
Link to the patchset: https://codereview.chromium.org/1843063002/#ps120001
(title: "OMIT_WEB -> OMIT_HTTP_AND_HTTPS, added comment")

[commit-bot: I haz the power, 2016-04-19 21:53:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1843063002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1843063002/120001

[commit-bot: I haz the power, 2016-04-19 21:57:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-19 21:57:29]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[benwells, 2016-04-20 00:45:00]

The CQ bit was checked by benwells@chromium.org

[benwells, 2016-04-20 00:45:00]

The patchset sent to the CQ was uploaded after l-g-t-m from palmer@chromium.org,
thakis@chromium.org, felt@chromium.org, caitkp@chromium.org
Link to the patchset: https://codereview.chromium.org/1843063002/#ps140001
(title: "rebase")

[commit-bot: I haz the power, 2016-04-20 00:45:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1843063002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1843063002/140001

[commit-bot: I haz the power, 2016-04-20 01:54:01]

Description was changed from

==========
Don't show scheme in permission prompts if it is HTTPS

HTTPS is nice and safe, so doesn't need to be called out in permission prompts.
This change should also make origin display for permission prompts consistent
across Android, Mac and Views.

BUG=596488
==========

to

==========
Don't show scheme in permission prompts if it is HTTPS

HTTPS is nice and safe, so doesn't need to be called out in permission prompts.
This change should also make origin display for permission prompts consistent
across Android, Mac and Views.

BUG=596488
==========

[commit-bot: I haz the power, 2016-04-20 01:54:02]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-04-22 19:20:01]

Description was changed from

==========
Don't show scheme in permission prompts if it is HTTPS

HTTPS is nice and safe, so doesn't need to be called out in permission prompts.
This change should also make origin display for permission prompts consistent
across Android, Mac and Views.

BUG=596488
==========

to

==========
Don't show scheme in permission prompts if it is HTTPS

HTTPS is nice and safe, so doesn't need to be called out in permission prompts.
This change should also make origin display for permission prompts consistent
across Android, Mac and Views.

BUG=596488

Committed: https://crrev.com/2337b81057c1ac85098834ad75d250e7d87e6a39
Cr-Commit-Position: refs/heads/master@{#388404}
==========

[commit-bot: I haz the power, 2016-04-22 19:20:03]

Patchset 8 (id:??) landed as
https://crrev.com/2337b81057c1ac85098834ad75d250e7d87e6a39
Cr-Commit-Position: refs/heads/master@{#388404}