Add an initial omnibox to HTML UI content.

Add an initial omnibox to HTML UI content.

- It currently displays only the current URL
- It is not interactive
- It needs better styling

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/95c8077faabeac737d9c18169d2916647cb2cc65
Cr-Commit-Position: refs/heads/master@{#431311}

[cjgrant, 2016-11-01 18:32:22]

Description was changed from

==========
Add an initial omnibox to HTML UI content.

- It currently displays only the current URL
- It is not interactive
- It needs better styling

BUG=641508
==========

to

==========
Add an initial omnibox to HTML UI content.

- It currently displays only the current URL
- It is not interactive
- It needs better styling

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[cjgrant, 2016-11-01 18:36:17]

cjgrant@chromium.org changed reviewers:
+ bshe@chromium.org, mthiesse@chromium.org

[cjgrant, 2016-11-01 18:36:17]

Guys, a follow-on change will shake up the CSS file for all elements, getting
rid of absolute positioning where possible, and letting the locations be
determined at runtime (Javascript queries DOM elements for their position).

Included in this will be a scrub of omnibox styling.

[bshe, 2016-11-01 22:24:13]

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.css (right):

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.css:176: left: 0;
Perhaps use flex box to be i18n friendly?

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:13: let scaleFactor =
uiStyle.getPropertyValue('--scaleFactor');
I am not sure I understand why do you want to get the value from css through
--scaleFactor? why not define it in js directly?

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:47: let style =
window.getComputedStyle(domElement);
huh. Just realize that we call getComputedStyle for each new element. It is not
a very cheap operation:
https://gist.github.com/paulirish/5d52fb081b3570c81e3a#getcomputedstyle
It can be a future TODO, but we probably should try to reduce the number of
calls.

[cjgrant, 2016-11-09 16:33:26]

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:13: let scaleFactor =
uiStyle.getPropertyValue('--scaleFactor');
On 2016/11/01 22:24:13, bshe wrote:
> I am not sure I understand why do you want to get the value from css through
> --scaleFactor? why not define it in js directly?

This scaling factor is required to correct for the "transform: scale()" we do in
CSS.  I want to keep the values together, like this:

#ui {
  --scaleFactor: 4;
  transform: scale(0.25, 0.25);
  ...
}

This way, if we change or remove this scaling, it's easy to locate.

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:47: let style =
window.getComputedStyle(domElement);
On 2016/11/01 22:24:12, bshe wrote:
> huh. Just realize that we call getComputedStyle for each new element. It is
not
> a very cheap operation:
> https://gist.github.com/paulirish/5d52fb081b3570c81e3a#getcomputedstyle
> It can be a future TODO, but we probably should try to reduce the number of
> calls. 

That's interesting.  I'll profile this.  We won't be querying many objects at
this point, but from what you posted, maybe there's a better way.  The
information we want should already be computed, and we don't want to cause any
relayout/recomputation if we can avoid it.

[cjgrant, 2016-11-09 18:03:34]

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:47: let style =
window.getComputedStyle(domElement);
On 2016/11/09 16:33:26, cjgrant wrote:
> On 2016/11/01 22:24:12, bshe wrote:
> > huh. Just realize that we call getComputedStyle for each new element. It is
> not
> > a very cheap operation:
> > https://gist.github.com/paulirish/5d52fb081b3570c81e3a#getcomputedstyle
> > It can be a future TODO, but we probably should try to reduce the number of
> > calls. 
> 
> That's interesting.  I'll profile this.  We won't be querying many objects at
> this point, but from what you posted, maybe there's a better way.  The
> information we want should already be computed, and we don't want to cause any
> relayout/recomputation if we can avoid it.

So FWIW, it looks like the impact of getComputedStyle is small.  We seem to
incur < 1 ms per call, while the creation of the UI takes quite a while 50 to
100 ms.  This excludes time required to actually flush the scene updates to
native.  So something is taking a while, but it's not the style calls, I
believe.

[bshe, 2016-11-10 18:06:49]

lgtm

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2473443002/diff/20001/chrome/browser/resource...
chrome/browser/resources/vr_shell/vr_shell_ui.js:47: let style =
window.getComputedStyle(domElement);
On 2016/11/09 18:03:34, cjgrant wrote:
> On 2016/11/09 16:33:26, cjgrant wrote:
> > On 2016/11/01 22:24:12, bshe wrote:
> > > huh. Just realize that we call getComputedStyle for each new element. It
is
> > not
> > > a very cheap operation:
> > > https://gist.github.com/paulirish/5d52fb081b3570c81e3a#getcomputedstyle
> > > It can be a future TODO, but we probably should try to reduce the number
of
> > > calls. 
> > 
> > That's interesting.  I'll profile this.  We won't be querying many objects
at
> > this point, but from what you posted, maybe there's a better way.  The
> > information we want should already be computed, and we don't want to cause
any
> > relayout/recomputation if we can avoid it.
> 
> So FWIW, it looks like the impact of getComputedStyle is small.  We seem to
> incur < 1 ms per call, while the creation of the UI takes quite a while 50 to
> 100 ms.  This excludes time required to actually flush the scene updates to
> native.  So something is taking a while, but it's not the style calls, I
> believe.

yah. I would imagine for our simple UI, this shouldn't take a long time. But as
our UI scales, we might need to be careful about this calls.

[cjgrant, 2016-11-10 18:15:18]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2016-11-10 18:15:19]

The patchset sent to the CQ was uploaded after l-g-t-m from bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2473443002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-11-10 18:15:59]

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, 2016-11-10 19:10:08]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-10 19:40:23]

Description was changed from

==========
Add an initial omnibox to HTML UI content.

- It currently displays only the current URL
- It is not interactive
- It needs better styling

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Add an initial omnibox to HTML UI content.

- It currently displays only the current URL
- It is not interactive
- It needs better styling

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/95c8077faabeac737d9c18169d2916647cb2cc65
Cr-Commit-Position: refs/heads/master@{#431311}
==========

[commit-bot: I haz the power, 2016-11-10 19:40:24]

Patchset 3 (id:??) landed as
https://crrev.com/95c8077faabeac737d9c18169d2916647cb2cc65
Cr-Commit-Position: refs/heads/master@{#431311}