Allow the VR omnibox to be transient.

chrome/browser/resources/vr_shell/vr_shell_ui.js

Index: chrome/browser/resources/vr_shell/vr_shell_ui.js
diff --git a/chrome/browser/resources/vr_shell/vr_shell_ui.js b/chrome/browser/resources/vr_shell/vr_shell_ui.js
index 1d363bf4f1a3a2326baa0090b98e60a1de821e9b..91b5309061834df6f761171107492d3edefe9793 100644
--- a/chrome/browser/resources/vr_shell/vr_shell_ui.js
+++ b/chrome/browser/resources/vr_shell/vr_shell_ui.js
@@ -251,34 +251,107 @@ var vrShellUi = (function() {
 
   class Omnibox {
     constructor(contentQuadId) {
-      this.setSecureOrigin(false);
-      this.domUiElement = new DomUiElement('#omni');
+      /** @const */ var VISIBILITY_TIMEOUT_MS = 3000;
+
+      this.domUiElement = new DomUiElement('#omni-container');
+      this.enabled = false;
+      this.secure = false;
+      this.visibilityTimeout = VISIBILITY_TIMEOUT_MS;

[bshe, 2016/11/17 15:18:19]

did you change the value of visibilityTimeout? if not, perhaps use
VISIBILITY_TIMEOUT_MS directly.

[cjgrant, 2016/11/17 20:17:43]

Changing the timeout to 0 disables the transient behaviour.  I didn't add a
setter for this yet, but I'd like to leave it build this way so it's easy to
tweak later.

+      this.visibilityTimer = null;
+      this.nativeState = {};
+
+      // Initially invisible.
       let update = new api.UiElementUpdate();
       update.setVisible(false);
       scene.updateElement(this.domUiElement.uiElementId, update);
+      this.nativeState.visible = false;
+
+      // Listen to the end of transitions, so that the box can be natively
+      // hidden after it finishes hiding itself.
+      document.querySelector('#omni').addEventListener('transitionend',
+          this.onAnimationDone.bind(this));
     }
 
-    show(visible) {
-      let update = new api.UiElementUpdate();
-      update.setVisible(visible);
-      scene.updateElement(this.domUiElement.uiElementId, update);
+    show(enabled) {

[bshe, 2016/11/17 15:18:19]

nit: It is possibly easier to understand if you change this to show/hide pair

[cjgrant, 2016/11/17 20:17:43]

I think show/hide will generate more duplicate code, but for consistency with
other members (and the other classes in this module), I'll change show() to
setEnabled().

+      this.enabled = enabled;
+      this.resetVisibilityTimer();
+      this.updateState();
     }
 
     setLoading(loading) {
-      this.domUiElement.domElement.className = loading ? 'loading' : 'idle';
+      this.loading = loading;

[bshe, 2016/11/17 15:18:19]

Should all the functions that might change the state of ominibox also enable
omnibox? Maybe call this.show(true)?

[cjgrant, 2016/11/17 20:17:43]

I don't want that to happen, no.  If the omnibox is disabled, the only thing
that should enable it is an explicit call to do so.  The reason I still handle
setLoading(), setURL(), etc., is to keep the state current so that if enabled,
the omnibox has current information to display.  Let me know if you don't like
this approach.

+      this.resetVisibilityTimer();
+      this.updateState();
     }
 
     setURL(host, path) {
       let omnibox = this.domUiElement.domElement;
       omnibox.querySelector('#domain').innerHTML = host;
       omnibox.querySelector('#path').innerHTML = path;
+      this.resetVisibilityTimer();
+      this.updateState();
     }
 
     setSecureOrigin(secure) {
+      this.secure = secure;
+      this.resetVisibilityTimer();
+      this.updateState();
+    }
+
+    resetVisibilityTimer() {
+      if (this.visibilityTimer) {
+        clearInterval(this.visibilityTimer);
+        this.visibilityTimer = null;
+      }
+      if (this.enabled && this.visibilityTimeout > 0) {
+        this.visibilityTimer = setTimeout(
+          this.onVisibilityTimer.bind(this), this.visibilityTimeout);
+      }
+    }
+
+    onVisibilityTimer() {
+      this.visibilityTimer = null;
+      this.updateState();
+    }
+
+    onAnimationDone(e) {
+      if (e.propertyName == 'opacity' && !this.visibleAfterTransition) {
+        this.setNativeVisibility(false);
+      }
+    }
+
+    updateState() {
+      if (!this.enabled) {
+        this.setNativeVisibility(false);
+        return;
+      }
+
       document.querySelector('#omni-secure-icon').style.display =
-          (secure ? 'block' : 'none');
+          (this.secure ? 'block' : 'none');
       document.querySelector('#omni-insecure-icon').style.display =
-          (!secure ? 'block' : 'none');
+          (this.secure ? 'none' : 'block');
+
+      let state = 'idle';
+      this.visibleAfterTransition = true;
+      if (this.visibilityTimeout > 0 && !this.visibilityTimer) {

[bshe, 2016/11/17 15:18:19]

Doesn't look like you changed the value of visibilityTimeout, so
"this.visibilityTimeout > 0" always return true?

[cjgrant, 2016/11/17 20:17:43]

As above, setting a non-zero timeout enables the transient behaviour.  I'll just
add the setter now for clarity.

+        state = 'hide';
+        this.visibleAfterTransition = false;
+      } else if (this.loading) {
+        state = 'loading';
+      }
+      document.querySelector('#omni').className = state;
+
+      this.setNativeVisibility(true);
+    }
+
+    setNativeVisibility(visible) {
+      if (visible != this.nativeState.visible) {

[bshe, 2016/11/17 15:18:19]

nit: prefer early return. 

+        this.nativeState.visible = visible;
+        let update = new api.UiElementUpdate();
+        update.setVisible(visible);
+        scene.updateElement(this.domUiElement.uiElementId, update);
+        scene.flush();
+      }
     }
   };