VR: Enforce LTR directionality on rendered URL text.

VR: Enforce LTR directionality on rendered URL text.

Do as the Views-based omnibox does, and enforce LTR on URL text.  This
ensures that malicious URLs can't visually spoof a hostname.  Also, add a unit test to ensure that the LTR directionality holds up against such a URL.

TBR=estark@chromium.org
BUG=735960
Review-Url: https://codereview.chromium.org/2945173006
Cr-Commit-Position: refs/heads/master@{#482054}
Committed: https://chromium.googlesource.com/chromium/src/+/c1f64db1a25c84c6583e9ac7dc2e035a6b8ddfec

[cjgrant, 2017-06-22 17:55:07]

cjgrant@chromium.org changed reviewers:
+ estark@chromium.org

[cjgrant, 2017-06-22 17:55:07]

Emily, this CL yields proper rendering (I believe) of the attack.com URL spoof. 
Basically a one-liner, and we can remove the bail-out.

[estark, 2017-06-23 01:34:28]

lgtm. I don't suppose it's possible to test this?

[cjgrant, 2017-06-23 16:02:29]

On 2017/06/23 01:34:28, estark wrote:
> lgtm. I don't suppose it's possible to test this?

I found a way to test and added it.  Basically, RenderText lets us look up
(through a "for test" method) physical positions of logical ranges of its input
text.  I used the attack.com URL in the test case.  Let me know what you think.

[cjgrant, 2017-06-23 16:02:53]

https://codereview.chromium.org/2945173006/diff/40001/chrome/browser/android/...
File chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc
(right):

https://codereview.chromium.org/2945173006/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc:53: static
void TestUrlStyling(const base::string16& formatted_url,
This is unrelated to RTL; just cleanup to the public/protected nature of this
method in the base class.

https://codereview.chromium.org/2945173006/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc:251: auto
minimum_position = real_bounds[0].x() + real_bounds[0].width();
If I remove the new LTR directionality setting over in the rendering code, this
test fails as expected (all parts of "attack.com" appear to the left of
127.0.0.1).

[cjgrant, 2017-06-23 18:28:26]

On 2017/06/23 16:02:53, cjgrant wrote:
>
https://codereview.chromium.org/2945173006/diff/40001/chrome/browser/android/...
> File chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/2945173006/diff/40001/chrome/browser/android/...
> chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc:53:
static
> void TestUrlStyling(const base::string16& formatted_url,
> This is unrelated to RTL; just cleanup to the public/protected nature of this
> method in the base class.
> 
>
https://codereview.chromium.org/2945173006/diff/40001/chrome/browser/android/...
> chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc:251: auto
> minimum_position = real_bounds[0].x() + real_bounds[0].width();
> If I remove the new LTR directionality setting over in the rendering code,
this
> test fails as expected (all parts of "attack.com" appear to the left of
> 127.0.0.1).

Emily, PTAL when you have time.  I'd appreciate a second opinion on the new unit
test.  Thanks!

[cjgrant, 2017-06-23 21:07:51]

Description was changed from

==========
VR: Enforce LTR directionality on rendered URL text.

Do as the Views-based omnibox does, and enforce LTR on URL text.  This
ensures that malicious URLs can't visually spoof a hostname.

BUG=735960
==========

to

==========
VR: Enforce LTR directionality on rendered URL text.

Do as the Views-based omnibox does, and enforce LTR on URL text.  This
ensures that malicious URLs can't visually spoof a hostname.  Also, add a unit
test to ensure that the LTR directionality holds up against such a URL.

TBR=estark@chromium.org
BUG=735960
==========

[cjgrant, 2017-06-23 21:07:59]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2017-06-23 21:07:59]

The patchset sent to the CQ was uploaded after l-g-t-m from estark@chromium.org
Link to the patchset: https://codereview.chromium.org/2945173006/#ps40001
(title: "Add a test to ensure that a malicious RTL URL cannot spoof a
hostname.")

[commit-bot: I haz the power, 2017-06-23 21:08:22]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-23 22:45:02]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1498252079237570,
"parent_rev": "7271d8176c0ad892bab3b2415f6dcace6a87a0b3", "commit_rev":
"c1f64db1a25c84c6583e9ac7dc2e035a6b8ddfec"}

[commit-bot: I haz the power, 2017-06-23 22:45:18]

Description was changed from

==========
VR: Enforce LTR directionality on rendered URL text.

Do as the Views-based omnibox does, and enforce LTR on URL text.  This
ensures that malicious URLs can't visually spoof a hostname.  Also, add a unit
test to ensure that the LTR directionality holds up against such a URL.

TBR=estark@chromium.org
BUG=735960
==========

to

==========
VR: Enforce LTR directionality on rendered URL text.

Do as the Views-based omnibox does, and enforce LTR on URL text.  This
ensures that malicious URLs can't visually spoof a hostname.  Also, add a unit
test to ensure that the LTR directionality holds up against such a URL.

TBR=estark@chromium.org
BUG=735960

Review-Url: https://codereview.chromium.org/2945173006
Cr-Commit-Position: refs/heads/master@{#482054}
Committed:
https://chromium.googlesource.com/chromium/src/+/c1f64db1a25c84c6583e9ac7dc2e...
==========

[commit-bot: I haz the power, 2017-06-23 22:45:20]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/c1f64db1a25c84c6583e9ac7dc2e...