VR: Render the current URL and security level on the URL bar.

VR: Render the current URL and security level on the URL bar.

- Don't refresh the URL bar texture if in WebVR mode.
- Use the appropriate security icon based on level.
- Don't draw any icon if there is no URL.
- Hook up the Back button (currently the whole bar) for testing.

BUG=715713
Review-Url: https://codereview.chromium.org/2872773002
Cr-Commit-Position: refs/heads/master@{#470465}
Committed: https://chromium.googlesource.com/chromium/src/+/ec187eecfd39a18cb94f43132858c2f7b3525cc8

[cjgrant, 2017-05-08 21:19:34]

cjgrant@chromium.org changed reviewers:
+ amp@chromium.org, mthiesse@chromium.org, tiborg@chromium.org

[cjgrant, 2017-05-08 21:19:35]

This breathes some life into the URL bar.  Hit testing on the actual back
button, vs. the whole bar, is next.

[cjgrant, 2017-05-08 21:22:56]

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/BUILD.gn (right):

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/BUILD.gn:90: "//chrome/app/vector_icons",
I'm checking whether this impacts binary size.

[mthiesse, 2017-05-08 21:39:30]

lgtm

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/BUILD.gn (right):

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/BUILD.gn:90: "//chrome/app/vector_icons",
On 2017/05/08 21:22:56, cjgrant wrote:
> I'm checking whether this impacts binary size.

Unused icons should be getting stripped.

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/textures/url_bar_texture.cc (right):

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:136:
PaintVectorIcon(&gfx_canvas, security_icon, kForeground);
Why not just use the 4-arg variant like our other icon rendering code? You can
specify the dip_size so it draws to a constant size.

[amp, 2017-05-08 21:45:25]

lgtm

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:230:
Java_VrShellImpl_navigateBack(env, j_vr_shell_.obj());
I was going to say we should wire up exiting full screen, but then I remembered
that we don't show the ui in full screen... :)

[cjgrant, 2017-05-09 00:57:42]

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/textures/url_bar_texture.cc (right):

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:136:
PaintVectorIcon(&gfx_canvas, security_icon, kForeground);
On 2017/05/08 21:39:30, mthiesse wrote:
> Why not just use the 4-arg variant like our other icon rendering code? You can
> specify the dip_size so it draws to a constant size.

Reason: Since the UX spec gives all dimensions in meters, I scale the canvas up
front so we can draw here in meters. But, the gfx:: functions seem tied to
integer pixel sizes, presumably because their designed for drawing on a 2D UI. 
To completely decouple from this, I just scale the icon myself.  The other
choice is to un-scale the canvas (as is done for the text below), and convert
all meters dimensions to pixels instead.  I don't know yet which approach is
better.

[tiborg, 2017-05-09 15:00:36]

lgtm with suggestions

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/textures/url_bar_texture.cc (right):

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:125: //
TODO(cjgrant): Plug in the correct icons based on security level.
Can this TODO go away now?

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/ui_elements/url_bar.h (right):

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/ui_elements/url_bar.h:30: void
SetBackButtonCallback(const base::Callback<void()>& callback);
What about void SetOnBackButtonClickedHandler(const base::Callback<void()>&
handler);?
Callback sounds so generic. But it's just an idea.

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:230:
Java_VrShellImpl_navigateBack(env, j_vr_shell_.obj());
On 2017/05/08 21:45:24, amp wrote:
> I was going to say we should wire up exiting full screen, but then I
remembered
> that we don't show the ui in full screen... :)

Also, isn't fullscreen automatically exited when you navigate?

[amp, 2017-05-09 16:38:07]

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2872773002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:230:
Java_VrShellImpl_navigateBack(env, j_vr_shell_.obj());
On 2017/05/09 15:00:36, tiborg wrote:
> On 2017/05/08 21:45:24, amp wrote:
> > I was going to say we should wire up exiting full screen, but then I
> remembered
> > that we don't show the ui in full screen... :)
> 
> Also, isn't fullscreen automatically exited when you navigate?

Yes it should be, although I'm not sure where that is wired up.

The main concern was if we are not doing what the user expected.  If the user
pressed back and expected to get back to the page where they started the full
screen, then navigating to the page previous to that would be bad.  That's why
we had the check in there before, to just exit fullscreen instead of navigating
away.

Since we will no longer have navigation ability in VR fullscreen anymore that
shouldn't be an issue.

[cjgrant, 2017-05-09 19:17:52]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2017-05-09 19:17:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, tiborg@chromium.org, amp@chromium.org
Link to the patchset: https://codereview.chromium.org/2872773002/#ps20001
(title: "Fix text mocks; rebase.")

[commit-bot: I haz the power, 2017-05-09 19:18:50]

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-05-09 19:25:20]

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

[commit-bot: I haz the power, 2017-05-09 19:25:23]

Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[cjgrant, 2017-05-09 21:36:10]

The CQ bit was checked by cjgrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 21:37:18]

Dry run: 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-05-09 21:55:41]

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

[commit-bot: I haz the power, 2017-05-09 21:55:44]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[cjgrant, 2017-05-10 01:58:17]

The CQ bit was checked by cjgrant@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 01:59:32]

Dry run: CQ is trying da patch.

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

[cjgrant, 2017-05-10 03:20:55]

The CQ bit was unchecked by cjgrant@chromium.org

[cjgrant, 2017-05-10 03:21:02]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2017-05-10 03:21:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, tiborg@chromium.org, amp@chromium.org
Link to the patchset: https://codereview.chromium.org/2872773002/#ps60001
(title: "Revert the inclusion of the lock icon; will re-add when moved to UI.")

[commit-bot: I haz the power, 2017-05-10 03:22:23]

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-05-10 04:58:09]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494386462720700,
"parent_rev": "f6a80eb8da8fdc08600b99885f38ad912a25331f", "commit_rev":
"ec187eecfd39a18cb94f43132858c2f7b3525cc8"}

[commit-bot: I haz the power, 2017-05-10 04:58:20]

Description was changed from

==========
VR: Render the current URL and security level on the URL bar.

- Don't refresh the URL bar texture if in WebVR mode.
- Use the appropriate security icon based on level.
- Don't draw any icon if there is no URL.
- Hook up the Back button (currently the whole bar) for testing.

BUG=715713
==========

to

==========
VR: Render the current URL and security level on the URL bar.

- Don't refresh the URL bar texture if in WebVR mode.
- Use the appropriate security icon based on level.
- Don't draw any icon if there is no URL.
- Hook up the Back button (currently the whole bar) for testing.

BUG=715713

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

[commit-bot: I haz the power, 2017-05-10 04:58:21]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/ec187eecfd39a18cb94f43132858...