Allow the VR omnibox to be transient.

chrome/browser/resources/vr_shell/vr_shell_ui.js

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 var vrShellUi = (function() {
   'use strict';
 
   let scene = new ui.Scene();
   let sceneManager;
 
@@ skipping 233 matching lines @@
       let update = new api.UiElementUpdate();
       update.setVisible(false);
       scene.updateElement(this.transientWarning.uiElementId, update);
       this.secureOriginTimer = null;
       scene.flush();
     }
   };
 
   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();
+      }
     }
   };
 
   class SceneManager {
     constructor() {
       this.mode = api.Mode.UNKNOWN;
 
       this.contentQuad = new ContentQuad();
       let contentId = this.contentQuad.getElementId();
 
@@ skipping 47 matching lines @@
     scene.flush();
   }
 
   return {
     initialize: initialize,
     command: command,
   };
 })();
 
 document.addEventListener('DOMContentLoaded', vrShellUi.initialize);