New issue
Advanced search Search tips

Issue 736756 link

Starred by 1 user
Status: Fixed
Owner:
cjgrant@chromium.org
Closed: Jun 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR

Blocking:
issue 735683
issue 735737



Sign in to add a comment

Use ToolbarModel to generate state for VR URL bar

Project Member Reported by cjgrant@chromium.org, Jun 26 2017 
Currently, the VR URL bar pulls state directly from WebContentsObserver callbacks.  For consistency, a ToolbarModel instance should be created and used instead.

This should cut down on inconsistent visuals on page transitions, and reduce the likelihood of security bugs in corner cases.

Use of ToolbarModel has bee prototyped and looks quite possible.  Note that the trigger to poll the ToolbarModel may still be WebContentsObserver based (TBD), as the model does not provide a callback indicating when it's reported state changes.

Comment 1 by ddorwin@chromium.org, Jun 26 2017

Labels: VR-BBB

Comment 2 by cjgrant@chromium.org, Jun 26 2017 

This bug might track the source of a few others.  I'm going to finish this work, then see which other issues it resolves.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad4f36ac5736301cbb7c9c47e8810039f6e256e9

commit ad4f36ac5736301cbb7c9c47e8810039f6e256e9
Author: cjgrant <cjgrant@chromium.org>
Date: Thu Jun 29 16:30:31 2017

VR: Use ToolbarModel to drive VR URL bar state.

This brings VR more in line with how desktop's omnibox works, and
reduces the potential for VR to exhibit inconsistencies.

Note that ToolbarModel supplies state, but not an indication of when that state
changes. For this reason, we'll still use a WebContentsObserver to
trigger updates.

BUG= 736756 

Review-Url: https://codereview.chromium.org/2960903002
Cr-Commit-Position: refs/heads/master@{#483383}

[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/BUILD.gn
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/textures/url_bar_texture.cc
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/textures/url_bar_texture.h
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc
[add] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/toolbar_helper.cc
[add] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/toolbar_helper.h
[add] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/toolbar_state.cc
[add] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/toolbar_state.h
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/ui_elements/transient_url_bar.cc
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/ui_elements/transient_url_bar.h
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/ui_elements/url_bar.cc
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/ui_elements/url_bar.h
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/ui_interface.h
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/ui_scene_manager.cc
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/ui_scene_manager.h
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/vr_gl_thread.cc
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/vr_gl_thread.h
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/vr_shell.h
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/vr_web_contents_observer.cc
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/chrome/browser/android/vr_shell/vr_web_contents_observer.h
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/components/toolbar/BUILD.gn
[modify] https://crrev.com/ad4f36ac5736301cbb7c9c47e8810039f6e256e9/components/toolbar/toolbar_model_impl.cc

Comment 4 by cjgrant@chromium.org, Jun 29 2017

Status: Fixed (was: Started)

Comment 5 by cjgrant@chromium.org, Jun 29 2017

Blocking: 735683

Comment 6 by cjgrant@chromium.org, Jun 29 2017

Blocking: 735737
►

Sign in to add a comment