[vr] Close exit prompt when clicking on background

chrome/browser/android/vr_shell/ui_scene_manager_unittest.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/macros.h"
 #include "base/test/scoped_task_environment.h"
 #include "chrome/browser/android/vr_shell/ui_elements/ui_element.h"
 #include "chrome/browser/android/vr_shell/ui_elements/ui_element_debug_id.h"
@@ skipping 43 matching lines @@
 
   // Stub this as scoped pointers don't work as mock method parameters.
   void ProcessContentGesture(std::unique_ptr<blink::WebInputEvent>) {}
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockBrowserInterface);
 };
 
 std::set<UiElementDebugId> kElementsVisibleInBrowsing = {
     kContentQuad, kBackplane, kCeiling, kFloor, kUrlBar, kLoadingIndicator};
+std::set<UiElementDebugId> kElementsVisibleWithExitPrompt = {

[cjgrant, 2017/06/06 21:21:56]

If this is only used in one test, it could live there.  Your call.

[ymalik, 2017/06/07 14:55:59]

Its used by two tests. The primary button click and and the backplane.

+    kExitPrompt, kExitPromptBackplane, kCeiling, kFloor};
 
 }  // namespace
 
 class UiSceneManagerTest : public testing::Test {
  public:
   void SetUp() override { browser_ = base::MakeUnique<MockBrowserInterface>(); }
 
  protected:
   enum InCct : bool {
     kNotInCct = false,
     kInCct = true,
   };
 
   enum InWebVr : bool {
     kNotInWebVr = false,
     kInWebVr = true,
   };
 
   void MakeManager(InCct in_cct, InWebVr in_web_vr) {
     scene_ = base::MakeUnique<UiScene>();
     manager_ = base::MakeUnique<UiSceneManager>(browser_.get(), scene_.get(),
                                                 in_cct, in_web_vr);
   }
 
   bool IsVisible(UiElementDebugId debug_id) {
     UiElement* element = scene_->GetUiElementByDebugId(debug_id);
     return element ? element->visible() : false;
   }
 
+  void VerifyElementsVisible(std::set<UiElementDebugId> elements) {

[cjgrant, 2017/06/06 21:21:56]

const&

[ymalik, 2017/06/07 14:55:58]

Done.

+    for (const auto& element : scene_->GetUiElements()) {
+      SCOPED_TRACE(element->debug_id());

[cjgrant, 2017/06/06 21:21:56]

If an element is wrong, does the trace still identify the right point of failure
(ie. the line on which this method is called)?  Do we need nested SCOPED_TRACE()
calls?  Just curious.

[amp, 2017/06/06 21:42:43]

Yes it fails on a specific line number, but then this scoped_trace will also
output with that failure.

It is immensely helpful in debugging loops (to me at least).

The line number is given, but it also gives you the id of the last item
processed (which you have to look up in the enum, but that's not too hard).

It will tell you exactly what element isn't doing what you expected.

[amp, 2017/06/06 21:46:22]

Oh wait, realized the question was if this would show the call site line number.
 No, it won't.

We need to add a SCOPED_TRACE('[test name]') before each call of '
VerifyElementsVisible' and then when something fails here it will tell us which
test failed.

We should also keep this one in the loop as well (you can have multiple in a
test).

[ymalik, 2017/06/07 14:55:58]

Ah good catch.

I don't think SCOPED_TRACE('[test name]') adds much value since we'll already
know which test failed. I think passing in a debug string is better because it
saves us an extra line before the call.

[cjgrant, 2017/06/07 16:03:06]

I played with this a bit, and I think what you have is optimal.  Docs seem to
suggest adding SCOPED_TRACE calls before every helper function call, but that's
cumbersome too.  This seems better.

[amp, 2017/06/07 17:18:41]

Cumbersome, but it's not really useful if you don't call it before the helper
function call (it won't be able to tell which test it was that failed).  See my
comment on latest patch set.

+      bool should_be_visible =
+          elements.find(element->debug_id()) != elements.end();
+      EXPECT_EQ(should_be_visible, element->visible());
+    }
+  }
+
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   std::unique_ptr<MockBrowserInterface> browser_;
   std::unique_ptr<UiScene> scene_;
   std::unique_ptr<UiSceneManager> manager_;
 };
 
 TEST_F(UiSceneManagerTest, ExitPresentAndFullscreenOnAppButtonClick) {
   MakeManager(kNotInCct, kInWebVr);
 
   // Clicking app button should trigger to exit presentation.
@@ skipping 102 matching lines @@
 }
 
 TEST_F(UiSceneManagerTest, UiUpdatesForFullscreenChanges) {
   std::set<UiElementDebugId> visible_in_fullscreen = {kContentQuad, kBackplane,
                                                       kCeiling, kFloor};
 
   MakeManager(kNotInCct, kNotInWebVr);
 
   // Hold onto the background color to make sure it changes.
   SkColor initial_background = scene_->GetBackgroundColor();
+  VerifyElementsVisible(kElementsVisibleInBrowsing);
 
-  for (const auto& element : scene_->GetUiElements()) {
-    SCOPED_TRACE(element->debug_id());
-    bool should_be_visible =
-        kElementsVisibleInBrowsing.find(element->debug_id()) !=
-        kElementsVisibleInBrowsing.end();
-    EXPECT_EQ(should_be_visible, element->visible());
-  }
-
-  // Transistion to fullscreen.
+  // In fullscreen mode, content elements should be visible, control elements

[amp, 2017/06/06 21:42:43]

My latest change (about to go in) changes this again, just an FYI.

[ymalik, 2017/06/07 14:55:59]

Acknowledged.

+  // should be hidden.
   manager_->SetFullscreen(true);
-
-  // Content elements should be visible, control elements should be hidden.
-  for (const auto& element : scene_->GetUiElements()) {
-    SCOPED_TRACE(element->debug_id());
-    bool should_be_visible = visible_in_fullscreen.find(element->debug_id()) !=
-                             visible_in_fullscreen.end();
-    EXPECT_EQ(should_be_visible, element->visible());
-  }
-
+  VerifyElementsVisible(visible_in_fullscreen);
   {
     SCOPED_TRACE("Entered Fullsceen");
     // Make sure background has changed for fullscreen.
     EXPECT_NE(initial_background, scene_->GetBackgroundColor());
   }
 
-  // Exit fullscreen.
+  // Everything should return to original state after leaving fullscreen.
   manager_->SetFullscreen(false);
-
-  // Everything should return to original state after leaving fullscreen.
-  for (const auto& element : scene_->GetUiElements()) {
-    SCOPED_TRACE(element->debug_id());
-    bool should_be_visible =
-        kElementsVisibleInBrowsing.find(element->debug_id()) !=
-        kElementsVisibleInBrowsing.end();
-    EXPECT_EQ(should_be_visible, element->visible());
-  }
+  VerifyElementsVisible(kElementsVisibleInBrowsing);
   {
     SCOPED_TRACE("Exited Fullsceen");
     EXPECT_EQ(initial_background, scene_->GetBackgroundColor());
   }
 }
 
 TEST_F(UiSceneManagerTest, UiUpdatesExitPrompt) {
-  std::set<UiElementDebugId> visible_when_prompting = {kExitPrompt, kBackplane,
-                                                       kCeiling, kFloor};
   MakeManager(kNotInCct, kNotInWebVr);
 
   manager_->SetWebVrSecureOrigin(true);
 
   // Initial state.
-  for (const auto& element : scene_->GetUiElements()) {
-    SCOPED_TRACE(element->debug_id());
-    bool should_be_visible =
-        kElementsVisibleInBrowsing.find(element->debug_id()) !=
-        kElementsVisibleInBrowsing.end();
-    EXPECT_EQ(should_be_visible, element->visible());
-  }
+  VerifyElementsVisible(kElementsVisibleInBrowsing);
 
   // Exit prompt visible state.
   manager_->OnSecurityIconClicked();
-  for (const auto& element : scene_->GetUiElements()) {
-    SCOPED_TRACE(element->debug_id());
-    bool should_be_visible = visible_when_prompting.find(element->debug_id()) !=
-                             visible_when_prompting.end();
-    EXPECT_EQ(should_be_visible, element->visible());
-  }
+  VerifyElementsVisible(kElementsVisibleWithExitPrompt);
 
   // Back to initial state.
   manager_->OnExitPromptPrimaryButtonClicked();
-  for (const auto& element : scene_->GetUiElements()) {
-    SCOPED_TRACE(element->debug_id());
-    bool should_be_visible =
-        kElementsVisibleInBrowsing.find(element->debug_id()) !=
-        kElementsVisibleInBrowsing.end();
-    EXPECT_EQ(should_be_visible, element->visible());
-  }
+  VerifyElementsVisible(kElementsVisibleInBrowsing);
+}
+
+TEST_F(UiSceneManagerTest, BackplaneClickClosesExitPrompt) {
+  MakeManager(kNotInCct, kNotInWebVr);
+
+  manager_->SetWebVrSecureOrigin(true);
+
+  // Initial state.
+  VerifyElementsVisible(kElementsVisibleInBrowsing);
+
+  // Exit prompt visible state.
+  manager_->OnSecurityIconClicked();
+  VerifyElementsVisible(kElementsVisibleWithExitPrompt);
+
+  // Back to initial state.
+  manager_->OnExitPromptBackplaneClicked();
+  VerifyElementsVisible(kElementsVisibleInBrowsing);
 }
 
 TEST_F(UiSceneManagerTest, UiUpdatesForWebVR) {
   MakeManager(kNotInCct, kInWebVr);
 
   manager_->SetWebVrSecureOrigin(true);
   manager_->SetAudioCapturingIndicator(true);
   manager_->SetVideoCapturingIndicator(true);
   manager_->SetScreenCapturingIndicator(true);
 
   // All elements should be hidden.
-  for (const auto& element : scene_->GetUiElements()) {
-    SCOPED_TRACE(element->debug_id());
-    EXPECT_FALSE(element->visible());
-  }
+  VerifyElementsVisible({});
 }
 
 TEST_F(UiSceneManagerTest, UiUpdateTransitionToWebVR) {
   MakeManager(kNotInCct, kNotInWebVr);
   manager_->SetAudioCapturingIndicator(true);
   manager_->SetVideoCapturingIndicator(true);
   manager_->SetScreenCapturingIndicator(true);
 
   // Transition to WebVR mode
   manager_->SetWebVrMode(true);
   manager_->SetWebVrSecureOrigin(true);
 
   // All elements should be hidden.
-  for (const auto& element : scene_->GetUiElements()) {
-    SCOPED_TRACE(element->debug_id());
-    EXPECT_FALSE(element->visible());
-  }
+  VerifyElementsVisible({});

[cjgrant, 2017/06/06 21:21:56]

Thanks for adjusting all these call sites!  code_size--;

[ymalik, 2017/06/07 14:55:59]

Acknowledged.

 }
 
 TEST_F(UiSceneManagerTest, CaptureIndicatorsVisibility) {
   MakeManager(kNotInCct, kNotInWebVr);
   EXPECT_FALSE(IsVisible(kAudioCaptureIndicator));
   EXPECT_FALSE(IsVisible(kVideoCaptureIndicator));
   EXPECT_FALSE(IsVisible(kScreenCaptureIndicator));
 
   manager_->SetAudioCapturingIndicator(true);
   manager_->SetVideoCapturingIndicator(true);
@@ skipping 15 matching lines @@
 
   manager_->SetAudioCapturingIndicator(false);
   manager_->SetVideoCapturingIndicator(false);
   manager_->SetScreenCapturingIndicator(false);
 
   EXPECT_FALSE(IsVisible(kAudioCaptureIndicator));
   EXPECT_FALSE(IsVisible(kVideoCaptureIndicator));
   EXPECT_FALSE(IsVisible(kScreenCaptureIndicator));
 }
 }  // namespace vr_shell