VR: Disable system indicators in full-screen mode.

chrome/browser/android/vr_shell/ui_scene_manager_unittest.cc

Index: chrome/browser/android/vr_shell/ui_scene_manager_unittest.cc
diff --git a/chrome/browser/android/vr_shell/ui_scene_manager_unittest.cc b/chrome/browser/android/vr_shell/ui_scene_manager_unittest.cc
index d3af85909b8341be5a62b49e297b08709da30a64..d4d90680b7ab9d114f12d2d6797bfb3c2c11e0d1 100644
--- a/chrome/browser/android/vr_shell/ui_scene_manager_unittest.cc
+++ b/chrome/browser/android/vr_shell/ui_scene_manager_unittest.cc
@@ -77,17 +77,31 @@ class UiSceneManagerTest : public testing::Test {
     return element ? element->visible() : false;
   }
 
+  // Verify that only the elements in the set are visible.
   void VerifyElementsVisible(const std::string& debug_name,
-                             const std::set<UiElementDebugId>& elements) {
+                             const std::set<UiElementDebugId>& debug_ids) {
     SCOPED_TRACE(debug_name);
     for (const auto& element : scene_->GetUiElements()) {
       SCOPED_TRACE(element->debug_id());
       bool should_be_visible =
-          elements.find(element->debug_id()) != elements.end();
+          debug_ids.find(element->debug_id()) != debug_ids.end();
       EXPECT_EQ(should_be_visible, element->visible());
     }
   }
 
+  // Return false if not all elements in the set match the specified visibility
+  // state. Other elements are ignored.
+  bool VerifyVisibility(const std::set<UiElementDebugId>& debug_ids,
+                        bool visible) {
+    for (const auto& element : scene_->GetUiElements()) {
+      if (debug_ids.find(element->debug_id()) != debug_ids.end() &&
+          element->visible() != visible) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   std::unique_ptr<MockBrowserInterface> browser_;
   std::unique_ptr<UiScene> scene_;
@@ -358,42 +372,36 @@ TEST_F(UiSceneManagerTest, UiUpdateTransitionToWebVR) {
 }
 
 TEST_F(UiSceneManagerTest, CaptureIndicatorsVisibility) {
+  const std::set<UiElementDebugId> indicators = {
+      kAudioCaptureIndicator, kVideoCaptureIndicator, kScreenCaptureIndicator,
+      kLocationAccessIndicator,
+  };
+
   MakeManager(kNotInCct, kNotInWebVr);
-  EXPECT_FALSE(IsVisible(kAudioCaptureIndicator));
-  EXPECT_FALSE(IsVisible(kVideoCaptureIndicator));
-  EXPECT_FALSE(IsVisible(kScreenCaptureIndicator));
-  EXPECT_FALSE(IsVisible(kLocationAccessIndicator));
+  EXPECT_TRUE(VerifyVisibility(indicators, false));
 
   manager_->SetAudioCapturingIndicator(true);

[amp, 2017/06/23 18:36:59]

Does anything different happen if some of the indicators are enabled and some
are disabled?

[cjgrant, 2017/06/23 19:38:26]

Nope, but, Amir and I talked about additional testing to make sure that enabling
one at a time shows one at a time.  Basically to make sure the plumbing from API
to individual indicator is correct, but that seems slightly too trivial to be
valuable.  A la "1 + 2 = 3.  Unit test that 1 + 2 really is 3".

   manager_->SetVideoCapturingIndicator(true);
   manager_->SetScreenCapturingIndicator(true);
   manager_->SetLocationAccessIndicator(true);
-
-  EXPECT_TRUE(IsVisible(kAudioCaptureIndicator));
-  EXPECT_TRUE(IsVisible(kVideoCaptureIndicator));
-  EXPECT_TRUE(IsVisible(kScreenCaptureIndicator));
-  EXPECT_TRUE(IsVisible(kLocationAccessIndicator));
+  EXPECT_TRUE(VerifyVisibility(indicators, true));
 
   manager_->SetWebVrMode(true, false, false);
-  EXPECT_FALSE(IsVisible(kAudioCaptureIndicator));
-  EXPECT_FALSE(IsVisible(kVideoCaptureIndicator));
-  EXPECT_FALSE(IsVisible(kScreenCaptureIndicator));
-  EXPECT_FALSE(IsVisible(kLocationAccessIndicator));
+  EXPECT_TRUE(VerifyVisibility(indicators, false));
 
   manager_->SetWebVrMode(false, false, false);
-  EXPECT_TRUE(IsVisible(kAudioCaptureIndicator));
-  EXPECT_TRUE(IsVisible(kVideoCaptureIndicator));
-  EXPECT_TRUE(IsVisible(kScreenCaptureIndicator));
-  EXPECT_TRUE(IsVisible(kLocationAccessIndicator));
+  EXPECT_TRUE(VerifyVisibility(indicators, true));
+
+  manager_->SetFullscreen(true);

[asimjour1, 2017/06/23 13:28:41]

We miss the transition from Fullscreen to WebVr.

[cjgrant, 2017/06/23 19:38:26]

Discussed offline.  The intent isn't really to cover all transitions, as much as
transitions that should cause shows/hides.

+  EXPECT_TRUE(VerifyVisibility(indicators, false));
+  manager_->SetFullscreen(false);
+  EXPECT_TRUE(VerifyVisibility(indicators, true));
 
   manager_->SetAudioCapturingIndicator(false);
   manager_->SetVideoCapturingIndicator(false);
   manager_->SetScreenCapturingIndicator(false);
   manager_->SetLocationAccessIndicator(false);
-
-  EXPECT_FALSE(IsVisible(kAudioCaptureIndicator));
-  EXPECT_FALSE(IsVisible(kVideoCaptureIndicator));
-  EXPECT_FALSE(IsVisible(kScreenCaptureIndicator));
-  EXPECT_FALSE(IsVisible(kLocationAccessIndicator));
+  EXPECT_TRUE(VerifyVisibility(indicators, false));
 }
+
 }  // namespace vr_shell