Expected result images are sometimes blank in WKTR

https://bugs.webkit.org/show_bug.cgi?id=120715

Reviewed by Tim Horton.

In WebKitTestRunner, snapshots obtained via windowSnapshotImage() were
sometimes blank if a previous test triggered compositing mode, and the
current test or reference did not require compositing. This happened
because the UI process didn't wait for the web process to complete
its compositing mode switch before snapshotting. Fix by calling
WKPageForceRepaint() before we take the snapshot; this is async,
so we have to spin the runloop for a while.

Remove the Qt/EFL code that does the same thing.

* WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::forceRepaintDoneCallback):
(WTR::TestInvocation::dumpResults):
* WebKitTestRunner/TestInvocation.h:
* WebKitTestRunner/cairo/TestInvocationCairo.cpp:
(WTR::TestInvocation::dumpPixelsAndCompareWithExpected):
* WebKitTestRunner/qt/TestInvocationQt.cpp:
(WTR::TestInvocation::dumpPixelsAndCompareWithExpected):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@155140 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

Tools/ChangeLog

@@ -1,3 +1,29 @@
+2013-09-05  Simon Fraser  <[email protected]>
+
+        Expected result images are sometimes blank in WKTR
+        https://bugs.webkit.org/show_bug.cgi?id=120715
+
+        Reviewed by Tim Horton.
+        
+        In WebKitTestRunner, snapshots obtained via windowSnapshotImage() were
+        sometimes blank if a previous test triggered compositing mode, and the
+        current test or reference did not require compositing. This happened
+        because the UI process didn't wait for the web process to complete
+        its compositing mode switch before snapshotting. Fix by calling
+        WKPageForceRepaint() before we take the snapshot; this is async,
+        so we have to spin the runloop for a while.
+        
+        Remove the Qt/EFL code that does the same thing.
+
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::forceRepaintDoneCallback):
+        (WTR::TestInvocation::dumpResults):
+        * WebKitTestRunner/TestInvocation.h:
+        * WebKitTestRunner/cairo/TestInvocationCairo.cpp:
+        (WTR::TestInvocation::dumpPixelsAndCompareWithExpected):
+        * WebKitTestRunner/qt/TestInvocationQt.cpp:
+        (WTR::TestInvocation::dumpPixelsAndCompareWithExpected):
+
 2013-09-05  Csaba Osztrogonác  <[email protected]>
 
         Make build.webkit.org report the number of failing run-fast-jsc tests

Tools/WebKitTestRunner/TestInvocation.cpp

@@ -310,15 +310,30 @@ void TestInvocation::dump(const char* textToStdout, const char* textToStderr, bo
     fflush(stderr);
 }
 
+void TestInvocation::forceRepaintDoneCallback(WKErrorRef, void* context)
+{
+    TestInvocation* testInvocation = static_cast<TestInvocation*>(context);
+    testInvocation->m_gotRepaint = true;
+}
+
 void TestInvocation::dumpResults()
 {
     if (m_textOutput.length() || !m_audioResult)
         dump(m_textOutput.toString().utf8().data());
     else
         dumpAudio(m_audioResult.get());
 
-    if (m_dumpPixels && m_pixelResult)
+    if (m_dumpPixels && m_pixelResult) {
+        m_gotRepaint = false;
+        WKPageForceRepaint(TestController::shared().mainWebView()->page(), this, TestInvocation::forceRepaintDoneCallback);
+        TestController::shared().runUntil(m_gotRepaint, TestController::ShortTimeout);
+        if (!m_gotRepaint) {
+            m_errorMessage = "Timed out waiting for pre-pixel dump repaint\n";
+            m_webProcessIsUnresponsive = true;
+            return;
+        }
         dumpPixelsAndCompareWithExpected(m_pixelResult.get(), m_repaintRects.get());
+    }
 
     fputs("#EOF\n", stdout);
     fflush(stdout);

Tools/WebKitTestRunner/TestInvocation.h

@@ -58,10 +58,8 @@ class TestInvocation {
     void dumpAudio(WKDataRef);
     bool compareActualHashToExpectedAndDumpResults(const char[33]);
 
-#if PLATFORM(QT) || PLATFORM(EFL)
     static void forceRepaintDoneCallback(WKErrorRef, void* context);
-#endif
-    
+
     WKRetainPtr<WKURLRef> m_url;
     std::string m_pathOrURL;
 

Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp

@@ -106,34 +106,11 @@ static void paintRepaintRectOverlay(cairo_surface_t* surface, WKArrayRef repaint
     cairo_destroy(context);
 }
 
-#if PLATFORM(EFL)
-void TestInvocation::forceRepaintDoneCallback(WKErrorRef, void *context)
-{
-    static_cast<TestInvocation*>(context)->m_gotRepaint = true;
-    TestController::shared().notifyDone();
-}
-#endif
-
 void TestInvocation::dumpPixelsAndCompareWithExpected(WKImageRef wkImage, WKArrayRef repaintRects)
 {
 #if USE(ACCELERATED_COMPOSITING) && PLATFORM(EFL)
     UNUSED_PARAM(wkImage);
-
-    cairo_surface_t* surface;
-
-    WKPageRef page = TestController::shared().mainWebView()->page();
-    WKPageForceRepaint(page, this, &forceRepaintDoneCallback);
-
-    TestController::shared().runUntil(m_gotRepaint, TestController::ShortTimeout);
-
-    if (!m_gotRepaint) {
-        m_error = true;
-        m_errorMessage = "Timed out waiting for repaint\n";
-        m_webProcessIsUnresponsive = true;
-        return;
-    }
-
-    surface = WKImageCreateCairoSurface(TestController::shared().mainWebView()->windowSnapshotImage().get());
+    cairo_surface_t* surface = WKImageCreateCairoSurface(TestController::shared().mainWebView()->windowSnapshotImage().get());
 #else
     cairo_surface_t* surface = WKImageCreateCairoSurface(wkImage);
 #endif

Tools/WebKitTestRunner/qt/TestInvocationQt.cpp

@@ -64,29 +64,11 @@ static void dumpImage(const QImage& image)
     fflush(stdout);
 }
 
-void TestInvocation::forceRepaintDoneCallback(WKErrorRef, void *context)
-{
-    static_cast<TestInvocation*>(context)->m_gotRepaint = true;
-    TestController::shared().notifyDone();
-}
-
 void TestInvocation::dumpPixelsAndCompareWithExpected(WKImageRef imageRef, WKArrayRef repaintRects)
 {
     QImage image;
     if (PlatformWebView::windowShapshotEnabled()) {
-        WKPageRef page = TestController::shared().mainWebView()->page();
-        WKPageForceRepaint(page, this, &forceRepaintDoneCallback);
-
-        TestController::shared().runUntil(m_gotRepaint, TestController::ShortTimeout);
-
-        if (m_gotRepaint)
-            image = WKImageCreateQImage(TestController::shared().mainWebView()->windowSnapshotImage().get());
-        else {
-            m_error = true;
-            m_errorMessage = "Timed out waiting for repaint\n";
-            m_webProcessIsUnresponsive = true;
-            return;
-        }
+        image = WKImageCreateQImage(TestController::shared().mainWebView()->windowSnapshotImage().get());
     } else
         image = WKImageCreateQImage(imageRef);