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..32e38073ba9736bf7c5061007e791b50cb3f3f6c 100644
--- a/chrome/browser/chromeos/login/login_ui_browsertest.cc
+++ b/chrome/browser/chromeos/login/login_ui_browsertest.cc
@@ -8,7 +8,7 @@
 #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_comparing_admixture.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"
@@ -26,69 +26,26 @@ namespace {
 
 const char kTestUser1[] = "test-user1@gmail.com";
 const char kTestUser2[] = "test-user2@gmail.com";
+}

[dzhioev (left Google), 2014/08/25 17:01:19]

Either add new line here or remove empty line above.

[Lisa Ignatyeva, 2014/08/25 17:57:54]

Done.

 
-// 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 {
+// Abstract class ScreenshotComparingTest is overriden to
+// handle properly all the animation specific for LoginUITest.
+class ScreenshotComparingAdmixtureForLoginUIVisible
+    : public ScreenshotComparingAdmixture {
  public:
-  AnimationDelayHandler();
-
-  // 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 content::NotificationObserver.
   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();
+ protected:
+  virtual void InitializeAnimationHandler() OVERRIDE;
+  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;
-  registrar_.Add(this,
-                 chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,
-                 content::NotificationService::AllSources());
-}
-
-bool AnimationDelayHandler::IsAnimationLoaded() {
-  return login_or_lock_webui_visible_;
-}
-
-void AnimationDelayHandler::Observe(
+void ScreenshotComparingAdmixtureForLoginUIVisible::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
@@ -104,57 +61,30 @@ 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
+ScreenshotComparingAdmixtureForLoginUIVisible::InitializeAnimationHandler() {
+  waiter_loop_is_on_ = false;
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,
+                 content::NotificationService::AllSources());
 }
 
-void AnimationDelayHandler::WaitUntilAnimationLoads() {
-  if (!IsAnimationLoaded()) {
-    base::RunLoop animation_waiter;
-    InitializeForWaiting(animation_waiter.QuitClosure());
-    animation_waiter.Run();
-  }
-  SynchronizeAnimationLoadWithCompositor();
+bool ScreenshotComparingAdmixtureForLoginUIVisible::IsAnimationLoaded() {
+  return login_or_lock_webui_visible_;
 }
 
 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();
+  LoginUITest() : LoginManagerTest(false) {
+    screenshot_comparing = new ScreenshotComparingAdmixtureForLoginUIVisible;
+    AddAdmixture(screenshot_comparing);
   }
 
+  virtual ~LoginUITest() {}
+
  protected:
-  AnimationDelayHandler animation_delay_handler;
-  ScreenshotTester screenshot_tester;
+  ScreenshotComparingAdmixtureForLoginUIVisible* screenshot_comparing;

[dzhioev (left Google), 2014/08/25 17:01:19]

screenshot_comparing_;

[Lisa Ignatyeva, 2014/08/25 17:57:54]

Done.

 };
 
 IN_PROC_BROWSER_TEST_F(LoginUITest, PRE_LoginUIVisible) {
@@ -174,10 +104,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");
-  }
+  screenshot_comparing->RunScreenshotTesting("LoginUITest-LoginUIVisible");
 }
 
 IN_PROC_BROWSER_TEST_F(LoginUITest, PRE_InterruptedAutoStartEnrollment) {