Generalizing architecture for screenshot testing

chrome/browser/chromeos/login/login_ui_browsertest.cc

Index: chrome/browser/chromeos/login/login_ui_browsertest.cc
diff --git a/chrome/browser/chromeos/login/login_ui_browsertest.cc b/chrome/browser/chromeos/login/login_ui_browsertest.cc
index 12c91c489507bb121867a5f862d1821fabe654d3..b22b754ed09a2e3949830c06f01a3b6bda23151b 100644
--- a/chrome/browser/chromeos/login/login_ui_browsertest.cc
+++ b/chrome/browser/chromeos/login/login_ui_browsertest.cc
@@ -4,11 +4,10 @@
 
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
-#include "base/timer/timer.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/login/login_manager_test.h"
-#include "chrome/browser/chromeos/login/screenshot_tester.h"
+#include "chrome/browser/chromeos/login/screenshot_test.h"
 #include "chrome/browser/chromeos/login/startup_utils.h"
 #include "chrome/browser/chromeos/login/test/oobe_screen_waiter.h"
 #include "chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h"
@@ -27,71 +26,41 @@ namespace {
 const char kTestUser1[] = "test-user1@gmail.com";
 const char kTestUser2[] = "test-user2@gmail.com";
 
-// A class that provides a way to wait until all the animation
-// has loaded and is properly shown on the screen.
-class AnimationDelayHandler : public content::NotificationObserver {
+}  // anonymous namespace
+
+class LoginUITest : public chromeos::ScreenshotTest {
  public:
-  AnimationDelayHandler();
+  virtual ~LoginUITest() {}
 
-  // Should be run as early as possible on order not to miss notifications.
-  // It seems though that it can't be moved to constructor(?).
-  void Initialize();
+  // Override from ScreenshotTest
+  virtual void InitializeAnimationHandler() OVERRIDE;
 
-  // Override from content::NotificationObserver.
+  // Override from ScreenshotTest
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
-  // This method checks if animation is loaded, and, if not,
-  // waits until it is loaded and properly shown on the screen.
-  void WaitUntilAnimationLoads();
+  // Override from ScreenshotTest.
+  virtual bool IsAnimationLoaded() OVERRIDE;
 
  private:
-  void InitializeForWaiting(const base::Closure& quitter);
-
-  // It turns out that it takes some more time for the animation
-  // to finish loading even after all the notifications have been sent.
-  // That happens due to some properties of compositor.
-  // This method should be used after getting all the necessary notifications
-  // to wait for the actual load of animation.
-  void SynchronizeAnimationLoadWithCompositor();
-
-  // This method exists only because of the current implementation of
-  // SynchronizeAnimationLoadWithCompositor.
-  void HandleAnimationLoad();
-
-  // Returns true if, according to the notificatons received, animation has
-  // finished loading by now.
-  bool IsAnimationLoaded();
-
-  base::OneShotTimer<AnimationDelayHandler> timer_;
-  bool waiter_loop_is_on_;
   bool login_or_lock_webui_visible_;
-  base::Closure animation_waiter_quitter_;
-  content::NotificationRegistrar registrar_;
 };
 
-}  // anonymous namespace
-
-AnimationDelayHandler::AnimationDelayHandler()
-    : waiter_loop_is_on_(false), login_or_lock_webui_visible_(false) {
-}
-
-void AnimationDelayHandler::Initialize() {
-  waiter_loop_is_on_ = false;
+void LoginUITest::InitializeAnimationHandler() {
+  login_or_lock_webui_visible_ = false;
   registrar_.Add(this,
                  chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,
                  content::NotificationService::AllSources());
 }
 
-bool AnimationDelayHandler::IsAnimationLoaded() {
+bool LoginUITest::IsAnimationLoaded() {
   return login_or_lock_webui_visible_;
 }
 
-void AnimationDelayHandler::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
+void LoginUITest::Observe(int type,
+                          const content::NotificationSource& source,
+                          const content::NotificationDetails& details) {
   if (chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE == type) {
     login_or_lock_webui_visible_ = true;
     registrar_.Remove(this,
@@ -104,58 +73,6 @@ void AnimationDelayHandler::Observe(
   }
 }
 
-void AnimationDelayHandler::InitializeForWaiting(const base::Closure& quitter) {
-  waiter_loop_is_on_ = true;
-  animation_waiter_quitter_ = quitter;
-}
-
-void AnimationDelayHandler::HandleAnimationLoad() {
-  timer_.Stop();
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI, FROM_HERE, animation_waiter_quitter_);
-}
-
-// Current implementation is a mockup.
-// It simply waits for 5 seconds, assuming that this time is enough for
-// animation to load completely.
-// TODO(elizavetai): Replace this temporary hack with getting a
-// valid notification from compositor.
-void AnimationDelayHandler::SynchronizeAnimationLoadWithCompositor() {
-  base::RunLoop waiter;
-  animation_waiter_quitter_ = waiter.QuitClosure();
-  timer_.Start(FROM_HERE,
-               base::TimeDelta::FromSeconds(5),
-               this,
-               &AnimationDelayHandler::HandleAnimationLoad);
-  waiter.Run();
-}
-
-void AnimationDelayHandler::WaitUntilAnimationLoads() {
-  if (!IsAnimationLoaded()) {
-    base::RunLoop animation_waiter;
-    InitializeForWaiting(animation_waiter.QuitClosure());
-    animation_waiter.Run();
-  }
-  SynchronizeAnimationLoadWithCompositor();
-}
-
-class LoginUITest : public chromeos::LoginManagerTest {
- public:
-  bool enable_test_screenshots_;
-  LoginUITest() : LoginManagerTest(false) {}
-  virtual ~LoginUITest() {}
-  virtual void SetUpOnMainThread() OVERRIDE {
-    enable_test_screenshots_ = screenshot_tester.TryInitialize();
-    if (enable_test_screenshots_) {
-      animation_delay_handler.Initialize();
-    }
-    LoginManagerTest::SetUpOnMainThread();
-  }
-
- protected:
-  AnimationDelayHandler animation_delay_handler;
-  ScreenshotTester screenshot_tester;
-};
 
 IN_PROC_BROWSER_TEST_F(LoginUITest, PRE_LoginUIVisible) {
   RegisterUser(kTestUser1);
@@ -174,10 +91,7 @@ IN_PROC_BROWSER_TEST_F(LoginUITest, LoginUIVisible) {
            ".user.emailAddress == '" + std::string(kTestUser1) + "'");
   JSExpect("document.querySelectorAll('.pod:not(#user-pod-template)')[1]"
            ".user.emailAddress == '" + std::string(kTestUser2) + "'");
-  if (enable_test_screenshots_) {
-    animation_delay_handler.WaitUntilAnimationLoads();
-    screenshot_tester.Run("LoginUITest-LoginUIVisible");
-  }
+  RunScreenshotTesting("LoginUITest-LoginUIVisible");

[dzhioev (left Google), 2014/08/07 11:23:55]

Note for future improvements: we can form test name dynamically, see
https://code.google.com/p/googletest/wiki/AdvancedGuide#Getting_the_Current_T...

 }
 
 IN_PROC_BROWSER_TEST_F(LoginUITest, PRE_InterruptedAutoStartEnrollment) {