VR: Factor transient timing out of UiSceneManager.

chrome/browser/android/vr_shell/ui_scene_manager.cc

 // Copyright 2017 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.
 
 #include "chrome/browser/android/vr_shell/ui_scene_manager.h"
 
 #include "base/callback.h"
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/android/vr_shell/textures/close_button_texture.h"
 #include "chrome/browser/android/vr_shell/textures/ui_texture.h"
@@ skipping 388 matching lines @@
   element->set_fill(vr_shell::Fill::NONE);
   element->set_size({kExitPromptBackplaneSize, kExitPromptBackplaneSize, 1.0});
   element->set_translation({0.0, 0.0, -kTextureOffset});
   element->set_parent_id(exit_prompt_->id());
   exit_prompt_backplane_ = element.get();
   content_elements_.push_back(element.get());
   scene_->AddUiElement(std::move(element));
 }
 
 void UiSceneManager::CreateToasts() {
-  std::unique_ptr<UiElement> element = base::MakeUnique<PresentationToast>(512);
+  auto element = base::MakeUnique<PresentationToast>(
+      512, base::TimeDelta::FromSeconds(kToastTimeoutSeconds));
   element->set_debug_id(kPresentationToast);
   element->set_id(AllocateId());
   element->set_fill(vr_shell::Fill::NONE);
   element->set_size({kToastWidth, kToastHeight, 1});
   element->set_translation({0, 0, -kToastDistance});
   element->set_visible(false);
   element->set_hit_testable(false);
   element->set_lock_to_fov(true);
   presentation_toast_ = element.get();
   scene_->AddUiElement(std::move(element));
 }
 
 base::WeakPtr<UiSceneManager> UiSceneManager::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
 void UiSceneManager::SetWebVrMode(bool web_vr,
                                   bool auto_presented,
                                   bool show_toast) {
   if (web_vr_mode_ == web_vr && web_vr_autopresented_ == auto_presented &&
       web_vr_show_toast_ == show_toast) {
     return;
   }
+  // If leaving WebVR, disable the toast. ConfigureScene will re-enable it if
+  // necessary.
+  if (!web_vr)
+    presentation_toast_->SetEnabled(false);

[bshe, 2017/06/27 14:57:09]

Do you need this? Isn't ConfigureScene set enable to correct state?

[cjgrant, 2017/06/28 19:00:55]

See the reworked version, where we need the few lines below.  Let me know if the
comment is not helpful.

+
   web_vr_mode_ = web_vr;
   web_vr_autopresented_ = auto_presented;
   web_vr_show_toast_ = show_toast;
-  toast_state_ = SET_FOR_WEB_VR;
   ConfigureScene();
   ConfigureSecurityWarnings();
   ConfigureTransientUrlBar();
-  ConfigurePresentationToast();
+
+  if (web_vr)

[bshe, 2017/06/27 14:57:09]

Should this be if (web_vr && show_toast)? It still works as we only enable the
toast when the above is true. But it is less readable compare to (web_vr &&
show_toast)

[cjgrant, 2017/06/28 19:00:55]

I think, as it is, this is better.  It's more clear with the IfEnabled() suffix
you suggested.

+    presentation_toast_->transience()->KickVisibility();
 }
 
 void UiSceneManager::ConfigureScene() {
   exit_warning_->SetEnabled(scene_->is_exiting());
   screen_dimmer_->SetEnabled(scene_->is_exiting());
 
   // Controls (URL bar, loading progress, etc).
   bool controls_visible = !web_vr_mode_ && !fullscreen_;
   for (UiElement* element : control_elements_) {
     element->SetEnabled(controls_visible && !scene_->is_prompting_to_exit());
@@ skipping 41 matching lines @@
                        -kCloseButtonDistance));
     close_button_->set_size(
         gfx::Vector3dF(kCloseButtonWidth, kCloseButtonHeight, 1));
   }
 
   scene_->SetMode(mode());
   scene_->SetBackgroundDistance(main_content_->translation().z() *
                                 -kBackgroundDistanceMultiplier);
   UpdateBackgroundColor();
 
+  presentation_toast_->SetEnabled((fullscreen_ && !web_vr_mode_) ||
+                                  (web_vr_mode_ && web_vr_show_toast_));
+
   // Configure other subsystems here as well. Ultimately, it would be nice if we
   // could configure all elements through ConfigureScene(), as the exact
   // conditions that control each element are getting complicated. More systems
   // should move in here, such that a single method call can update the entire
   // scene. The drawback is slightly more overhead for individual scene
   // reconfigurations.
   ConfigureIndicators();
 }
 
 void UiSceneManager::UpdateBackgroundColor() {
@@ skipping 50 matching lines @@
   browser_->ExitFullscreen();
 }
 
 void UiSceneManager::OnAppButtonGesturePerformed(
     UiInterface::Direction direction) {}
 
 void UiSceneManager::SetFullscreen(bool fullscreen) {
   if (fullscreen_ == fullscreen)
     return;
   fullscreen_ = fullscreen;
-  toast_state_ = SET_FOR_FULLSCREEN;
   ConfigureScene();
-  ConfigurePresentationToast();
+  presentation_toast_->transience()->KickVisibility();
 }
 
 void UiSceneManager::ConfigureSecurityWarnings() {
   bool enabled = web_vr_mode_ && !secure_origin_;
   permanent_security_warning_->set_visible(enabled);
   transient_security_warning_->set_visible(enabled);
   if (enabled) {
+    security_warning_timer_.Stop();

[bshe, 2017/06/27 14:57:09]

Stop shouldn't be necessary I believe. Start would reset previous timer.

[cjgrant, 2017/06/28 19:00:55]

Done.

     security_warning_timer_.Start(
         FROM_HERE, base::TimeDelta::FromSeconds(kWarningTimeoutSeconds), this,
         &UiSceneManager::OnSecurityWarningTimer);
   } else {
     security_warning_timer_.Stop();
   }
 }
 
 void UiSceneManager::ConfigureIndicators() {
   bool allowed = !web_vr_mode_ && !fullscreen_;
   audio_capture_indicator_->set_visible(allowed && audio_capturing_);
   video_capture_indicator_->set_visible(allowed && video_capturing_);
   screen_capture_indicator_->set_visible(allowed && screen_capturing_);
   location_access_indicator_->set_visible(allowed && location_access_);
 
   if (!allowed)
     return;
 
   // Position elements dynamically relative to each other, based on which
   // indicators are showing, and how big each one is.
   float total_width = 0;
   for (const UiElement* indicator : system_indicators_) {
     if (indicator->visible()) {
       if (total_width > 0)
         total_width += kIndicatorGap;
       total_width += indicator->size().x();
     }
   }
+
   float x_position = -total_width / 2;
   for (UiElement* indicator : system_indicators_) {
     if (!indicator->visible())
       continue;
     float width = indicator->size().x();
     indicator->set_translation({x_position + width / 2,
                                 kIndicatorVerticalOffset,
                                 kIndicatorDistanceOffset});
     x_position += width + kIndicatorGap;
   }
 }
 
-void UiSceneManager::ConfigurePresentationToast() {
-  bool toast_visible = false;
-  switch (toast_state_) {
-    case SET_FOR_WEB_VR:
-      toast_visible = web_vr_show_toast_;
-      break;
-    case SET_FOR_FULLSCREEN:
-      toast_visible = fullscreen_;
-      break;
-    case UNCHANGED:
-      return;
-  }
-  presentation_toast_->set_visible(toast_visible);
-  if (toast_visible) {
-    presentation_toast_timer_.Start(
-        FROM_HERE, base::TimeDelta::FromSeconds(kToastTimeoutSeconds), this,
-        &UiSceneManager::OnPresentationToastTimer);
-  } else {
-    presentation_toast_timer_.Stop();
-  }
-  toast_state_ = UNCHANGED;
-}
-
 void UiSceneManager::OnSecurityWarningTimer() {
+  // TODO: Also do this one!

[bshe, 2017/06/27 14:57:09]

Add ldap or create a crbug?

[cjgrant, 2017/06/27 15:15:55]

I'm planning to do this before submitting.  I posted the CL to make sure you
approve of the method before changing all the other elements to use it.  There's
a follow-up patch coming.  And sorry, this wasn't as polished as it should have
been.  :)

[cjgrant, 2017/06/28 19:00:55]

Done.

   transient_security_warning_->set_visible(false);
 }
 
 void UiSceneManager::ConfigureTransientUrlBar() {
   bool enabled = web_vr_mode_ && web_vr_autopresented_;
   transient_url_bar_->set_visible(enabled);
   if (enabled) {
     transient_url_bar_timer_.Start(
         FROM_HERE, base::TimeDelta::FromSeconds(kTransientUrlBarTimeoutSeconds),
         this, &UiSceneManager::OnTransientUrlBarTimer);
   } else {
     transient_url_bar_timer_.Stop();
   }
 }
 
 void UiSceneManager::OnTransientUrlBarTimer() {
   transient_url_bar_->set_visible(false);
 }
 
-void UiSceneManager::OnPresentationToastTimer() {
-  presentation_toast_->set_visible(false);
-}
-
 void UiSceneManager::OnBackButtonClicked() {
   browser_->NavigateBack();
 }
 
 void UiSceneManager::OnSecurityIconClickedForTesting() {
   OnSecurityIconClicked();
 }
 
 void UiSceneManager::OnExitPromptPrimaryButtonClickedForTesting() {
   OnExitPromptPrimaryButtonClicked();
@@ skipping 79 matching lines @@
   if (fullscreen_)
     return ColorScheme::kModeFullscreen;
   return ColorScheme::kModeNormal;
 }
 
 const ColorScheme& UiSceneManager::color_scheme() const {
   return ColorScheme::GetColorScheme(mode());
 }
 
 }  // namespace vr_shell