VrShell: Add test for fullscreen transitions.

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 "chrome/browser/android/vr_shell/ui_elements/ui_element.h"
 #include "chrome/browser/android/vr_shell/ui_scene.h"
 #include "chrome/browser/android/vr_shell/vr_browser_interface.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using testing::InSequence;
 
 namespace vr_shell {
 
 namespace {
@@ skipping 53 matching lines @@
   EXPECT_CALL(*browser_, OnContentPaused(true)).Times(1);
   manager_->OnAppButtonClicked();
   EXPECT_FALSE(scene_->GetWebVrRenderingEnabled());
 
   // Clicking it again should resume content rendering.
   EXPECT_CALL(*browser_, OnContentPaused(false)).Times(1);
   manager_->OnAppButtonClicked();
   EXPECT_TRUE(scene_->GetWebVrRenderingEnabled());
 }
 
+TEST_F(UiSceneManagerTest, UiUpdatesForFullscreen) {
+  InSequence s;
+  // Set mode to not WebVR, everything should be visible.
+  manager_->SetWebVRMode(false);
+
+  // Hold onto the background color to make sure it changes.
+  vr::Colorf initialBackground = scene_->GetBackgroundColor();
+
+  for (UiElement* element : manager_->browser_ui_elements_) {
+    EXPECT_TRUE(element->visible());
+  }
+
+  manager_->SetFullscreen(true);
+
+  // All elements should be hidden except for main content.
+  for (UiElement* element : manager_->browser_ui_elements_) {
+    if (element == manager_->main_content_) {
+      EXPECT_TRUE(element->visible());
+      continue;
+    }
+    EXPECT_FALSE(element->visible());
+  }
+  // Make sure background has changed for fullscreen.
+  // TODO(amp): Check more than just the red value.

[bsheedy, 2017/05/09 21:27:09]

This seems like a simple thing to add in this CL - do now and get rid of TODO?

[amp, 2017/05/09 21:36:24]

The reason for the TODO is that I really just want to write:
EXPECT_NE(initialBackground, scene_->GetBackgroundColor());

but that doesn't work because it's a struct without an overridden equality
operator.

So instead I would have to write:
EXPECT_NE(initialBackground.r, scene_->GetBackgroundColor().r);
EXPECT_NE(initialBackground.g, scene_->GetBackgroundColor().g);
EXPECT_NE(initialBackground.b, scene_->GetBackgroundColor().b);
EXPECT_NE(initialBackground.a, scene_->GetBackgroundColor().a);


Not a huge difference, but it's kind of ugly (especially to do multiple times)
and I was hoping someone might point out a better way.  Any ideas?

[amp, 2017/05/09 22:47:37]

Decided to go ahead and just add it.  One thing is that the alpha is the same
for both variants, so it doesn't really work as straightforwardly as it could.

[cjgrant, 2017/05/10 14:08:36]

I feel your pain on this one. :) Wrapper functions around multiple expects
aren't good here because a test failure won't print the exact location.  I've
used macros successfully before (see the scene unit test), but in that case,
you'd lose the readout of exactly which value was wrong.

Another option here would be to add a pair of helpers to this module that
convert the 4 floats to a 32-bit ARGV value, and the 3 floats to an RGV value,
and run your EXPECTs on the results of those.  That could cause a false pass if
the expected color change is tiny, but that's unlikely.

[amp, 2017/05/10 16:05:55]

I'll leave it as is for now since we only care that the values changed at this
point and they are not small differences, but setting up some helpers sounds
like it could be useful if we want to get more specific about color variations.

+  EXPECT_NE(initialBackground.r, scene_->GetBackgroundColor().r);
+
+  manager_->SetFullscreen(false);
+
+  // Everything should return to original state after leaving fullscreen.
+  for (UiElement* element : manager_->browser_ui_elements_) {
+    EXPECT_TRUE(element->visible());
+  }
+  // TODO(amp): Check more than just the red value.

[bsheedy, 2017/05/09 21:27:09]

Ditto.

+  EXPECT_EQ(initialBackground.r, scene_->GetBackgroundColor().r);
+}
+
 }  // namespace vr_shell