[WTR] Pixel tests generate the snapshots twice in Web and UI processes

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

Reviewed by Tim Horton.

All ports except IOS implement
PlatformWebView::windowSnapshotImage() to generate the snapshot
for the pixel tests in the UI process. But we are still generating
a snapshot for pixel tests in the Web process too, that is passed
to the UI process but ignored.
Whether a pixel result is needed or not, is only known by the web
process depending on whether the test called dumpAsText with
dumpPixels == true or not. Since the pixels are now dump in the UI
process, we need to pass that information to the UI process when
the test is done. For that we set a PixelResultIsPending bool
parameter to the Done message, and we only add the PixelResult
when UI process doesn't need to generate the pixels dump.

* WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
(WTR::InjectedBundle::didReceiveMessageToPage): Set
m_pixelResultIsPending to false on reset.
(WTR::InjectedBundle::done): Add PixelResultIsPending parameter to the
Done message, and set the PixelResult if m_pixelResultIsPending is false.
* WebKitTestRunner/InjectedBundle/InjectedBundle.h:
(WTR::InjectedBundle::setPixelResult): Set m_pixelResultIsPending to false.
(WTR::InjectedBundle::setNeedsPixelResult): Set m_pixelResultIsPending.
* WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:
(WTR::InjectedBundlePage::dump): Only create a snapshot for IOS
port.
* WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::dumpResults): Use either the pixel result
from the web process or generate a pixel result from the web view
if need it.
(WTR::TestInvocation::didReceiveMessageFromInjectedBundle):
* WebKitTestRunner/TestInvocation.h: Add SnapshotResultType enum
parameter to dumpPixelsAndCompareWithExpected, since the snapshot
is created by the caller now, but the CG implementation needs to
know if it's a Web or UI process snapshot.
* WebKitTestRunner/cairo/TestInvocationCairo.cpp:
(WTR::TestInvocation::dumpPixelsAndCompareWithExpected): Create a
cairo surface for the given image.
* WebKitTestRunner/cg/TestInvocationCG.cpp:
(WTR::TestInvocation::dumpPixelsAndCompareWithExpected): Create a
CGContext for the given image.

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

Author: [email protected]

Tools/ChangeLog

@@ -1,3 +1,50 @@
+2015-09-28  Carlos Garcia Campos  <[email protected]>
+
+        [WTR] Pixel tests generate the snapshots twice in Web and UI processes
+        https://bugs.webkit.org/show_bug.cgi?id=149595
+
+        Reviewed by Tim Horton.
+
+        All ports except IOS implement
+        PlatformWebView::windowSnapshotImage() to generate the snapshot
+        for the pixel tests in the UI process. But we are still generating
+        a snapshot for pixel tests in the Web process too, that is passed
+        to the UI process but ignored.
+        Whether a pixel result is needed or not, is only known by the web
+        process depending on whether the test called dumpAsText with
+        dumpPixels == true or not. Since the pixels are now dump in the UI
+        process, we need to pass that information to the UI process when
+        the test is done. For that we set a PixelResultIsPending bool
+        parameter to the Done message, and we only add the PixelResult
+        when UI process doesn't need to generate the pixels dump.
+
+        * WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
+        (WTR::InjectedBundle::didReceiveMessageToPage): Set
+        m_pixelResultIsPending to false on reset.
+        (WTR::InjectedBundle::done): Add PixelResultIsPending parameter to the
+        Done message, and set the PixelResult if m_pixelResultIsPending is false.
+        * WebKitTestRunner/InjectedBundle/InjectedBundle.h:
+        (WTR::InjectedBundle::setPixelResult): Set m_pixelResultIsPending to false.
+        (WTR::InjectedBundle::setNeedsPixelResult): Set m_pixelResultIsPending.
+        * WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:
+        (WTR::InjectedBundlePage::dump): Only create a snapshot for IOS
+        port.
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::dumpResults): Use either the pixel result
+        from the web process or generate a pixel result from the web view
+        if need it.
+        (WTR::TestInvocation::didReceiveMessageFromInjectedBundle):
+        * WebKitTestRunner/TestInvocation.h: Add SnapshotResultType enum
+        parameter to dumpPixelsAndCompareWithExpected, since the snapshot
+        is created by the caller now, but the CG implementation needs to
+        know if it's a Web or UI process snapshot.
+        * WebKitTestRunner/cairo/TestInvocationCairo.cpp:
+        (WTR::TestInvocation::dumpPixelsAndCompareWithExpected): Create a
+        cairo surface for the given image.
+        * WebKitTestRunner/cg/TestInvocationCG.cpp:
+        (WTR::TestInvocation::dumpPixelsAndCompareWithExpected): Create a
+        CGContext for the given image.
+
 2015-09-28  Dean Johnson  <[email protected]>
 
         Fix JS errors on dashboard metrics page

Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp

@@ -186,6 +186,7 @@ void InjectedBundle::didReceiveMessageToPage(WKBundlePageRef page, WKStringRef m
 
         m_state = Idle;
         m_dumpPixels = false;
+        m_pixelResultIsPending = false;
 
         resetLocalSettings();
         m_testRunner->removeAllWebNotificationPermissions();
@@ -338,8 +339,14 @@ void InjectedBundle::done()
     WKRetainPtr<WKStringRef> doneMessageName(AdoptWK, WKStringCreateWithUTF8CString("Done"));
     WKRetainPtr<WKMutableDictionaryRef> doneMessageBody(AdoptWK, WKMutableDictionaryCreate());
 
-    WKRetainPtr<WKStringRef> pixelResultKey = adoptWK(WKStringCreateWithUTF8CString("PixelResult"));
-    WKDictionarySetItem(doneMessageBody.get(), pixelResultKey.get(), m_pixelResult.get());
+    WKRetainPtr<WKStringRef> pixelResultIsPendingKey = adoptWK(WKStringCreateWithUTF8CString("PixelResultIsPending"));
+    WKRetainPtr<WKBooleanRef> pixelResultIsPending(AdoptWK, WKBooleanCreate(m_pixelResultIsPending));
+    WKDictionarySetItem(doneMessageBody.get(), pixelResultIsPendingKey.get(), pixelResultIsPending.get());
+
+    if (!m_pixelResultIsPending) {
+        WKRetainPtr<WKStringRef> pixelResultKey = adoptWK(WKStringCreateWithUTF8CString("PixelResult"));
+        WKDictionarySetItem(doneMessageBody.get(), pixelResultKey.get(), m_pixelResult.get());
+    }
 
     WKRetainPtr<WKStringRef> repaintRectsKey = adoptWK(WKStringCreateWithUTF8CString("RepaintRects"));
     WKDictionarySetItem(doneMessageBody.get(), repaintRectsKey.get(), m_repaintRects.get());

Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.h

@@ -66,7 +66,8 @@ class InjectedBundle {
 
     void done();
     void setAudioResult(WKDataRef audioData) { m_audioResult = audioData; }
-    void setPixelResult(WKImageRef image) { m_pixelResult = image; }
+    void setPixelResult(WKImageRef image) { m_pixelResult = image; m_pixelResultIsPending = false; }
+    void setPixelResultIsPending(bool isPending) { m_pixelResultIsPending = isPending; }
     void setRepaintRects(WKArrayRef rects) { m_repaintRects = rects; }
 
     bool isTestRunning() { return m_state == Testing; }
@@ -164,6 +165,7 @@ class InjectedBundle {
     bool m_useWaitToDumpWatchdogTimer;
     bool m_useWorkQueue;
     int m_timeout;
+    bool m_pixelResultIsPending { false };
 
     WKRetainPtr<WKDataRef> m_audioResult;
     WKRetainPtr<WKImageRef> m_pixelResult;

Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp

@@ -892,11 +892,16 @@ void InjectedBundlePage::dump()
         injectedBundle.dumpBackForwardListsForAllPages(stringBuilder);
 
     if (injectedBundle.shouldDumpPixels() && injectedBundle.testRunner()->shouldDumpPixels()) {
+#if PLATFORM(IOS)
+        // IOS doesn't implement PlatformWebView::windowSnapshotImage() yet, so we need to generate the snapshot in the web process.
         WKSnapshotOptions options = kWKSnapshotOptionsShareable | kWKSnapshotOptionsInViewCoordinates;
         if (injectedBundle.testRunner()->shouldDumpSelectionRect())
             options |= kWKSnapshotOptionsPaintSelectionRectangle;
 
         injectedBundle.setPixelResult(adoptWK(WKBundlePageCreateSnapshotWithOptions(m_page, WKBundleFrameGetVisibleContentBounds(WKBundlePageGetMainFrame(m_page)), options)).get());
+#else
+        injectedBundle.setPixelResultIsPending(true);
+#endif
         if (WKBundlePageIsTrackingRepaints(m_page))
             injectedBundle.setRepaintRects(adoptWK(WKBundlePageCopyTrackedRepaintRects(m_page)).get());
     }

Tools/WebKitTestRunner/TestInvocation.cpp

@@ -288,16 +288,22 @@ void TestInvocation::dumpResults()
     else
         dumpAudio(m_audioResult.get());
 
-    if (m_dumpPixels && m_pixelResult) {
-        m_gotRepaint = false;
-        WKPageForceRepaint(TestController::singleton().mainWebView()->page(), this, TestInvocation::forceRepaintDoneCallback);
-        TestController::singleton().runUntil(m_gotRepaint, TestController::shortTimeout);
-        if (!m_gotRepaint) {
-            m_errorMessage = "Timed out waiting for pre-pixel dump repaint\n";
-            m_webProcessIsUnresponsive = true;
-            return;
+    if (m_dumpPixels) {
+        if (m_pixelResult)
+            dumpPixelsAndCompareWithExpected(m_pixelResult.get(), m_repaintRects.get(), TestInvocation::SnapshotResultType::WebContents);
+        else if (m_pixelResultIsPending) {
+            m_gotRepaint = false;
+            WKPageForceRepaint(TestController::singleton().mainWebView()->page(), this, TestInvocation::forceRepaintDoneCallback);
+            TestController::singleton().runUntil(m_gotRepaint, TestController::shortTimeout);
+            if (!m_gotRepaint) {
+                m_errorMessage = "Timed out waiting for pre-pixel dump repaint\n";
+                m_webProcessIsUnresponsive = true;
+                return;
+            }
+            WKRetainPtr<WKImageRef> windowSnapshot = TestController::singleton().mainWebView()->windowSnapshotImage();
+            ASSERT(windowSnapshot);
+            dumpPixelsAndCompareWithExpected(windowSnapshot.get(), m_repaintRects.get(), TestInvocation::SnapshotResultType::WebView);
         }
-        dumpPixelsAndCompareWithExpected(m_pixelResult.get(), m_repaintRects.get());
     }
 
     fputs("#EOF\n", stdout);
@@ -364,9 +370,15 @@ void TestInvocation::didReceiveMessageFromInjectedBundle(WKStringRef messageName
         ASSERT(WKGetTypeID(messageBody) == WKDictionaryGetTypeID());
         WKDictionaryRef messageBodyDictionary = static_cast<WKDictionaryRef>(messageBody);
 
-        WKRetainPtr<WKStringRef> pixelResultKey = adoptWK(WKStringCreateWithUTF8CString("PixelResult"));
-        m_pixelResult = static_cast<WKImageRef>(WKDictionaryGetItemForKey(messageBodyDictionary, pixelResultKey.get()));
-        ASSERT(!m_pixelResult || m_dumpPixels);
+        WKRetainPtr<WKStringRef> pixelResultIsPendingKey = adoptWK(WKStringCreateWithUTF8CString("PixelResultIsPending"));
+        WKBooleanRef pixelResultIsPending = static_cast<WKBooleanRef>(WKDictionaryGetItemForKey(messageBodyDictionary, pixelResultIsPendingKey.get()));
+        m_pixelResultIsPending = WKBooleanGetValue(pixelResultIsPending);
+
+        if (!m_pixelResultIsPending) {
+            WKRetainPtr<WKStringRef> pixelResultKey = adoptWK(WKStringCreateWithUTF8CString("PixelResult"));
+            m_pixelResult = static_cast<WKImageRef>(WKDictionaryGetItemForKey(messageBodyDictionary, pixelResultKey.get()));
+            ASSERT(!m_pixelResult || m_dumpPixels);
+        }
 
         WKRetainPtr<WKStringRef> repaintRectsKey = adoptWK(WKStringCreateWithUTF8CString("RepaintRects"));
         m_repaintRects = static_cast<WKArrayRef>(WKDictionaryGetItemForKey(messageBodyDictionary, repaintRectsKey.get()));

Tools/WebKitTestRunner/TestInvocation.h

@@ -69,7 +69,8 @@ class TestInvocation : public UIScriptContextDelegate {
 private:
     void dumpResults();
     static void dump(const char* textToStdout, const char* textToStderr = 0, bool seenError = false);
-    void dumpPixelsAndCompareWithExpected(WKImageRef, WKArrayRef repaintRects);
+    enum class SnapshotResultType { WebView, WebContents };
+    void dumpPixelsAndCompareWithExpected(WKImageRef, WKArrayRef repaintRects, SnapshotResultType);
     void dumpAudio(WKDataRef);
     bool compareActualHashToExpectedAndDumpResults(const char[33]);
 
@@ -93,6 +94,7 @@ class TestInvocation : public UIScriptContextDelegate {
     WTF::String m_urlString;
 
     bool m_dumpPixels;
+    bool m_pixelResultIsPending { false };
     std::string m_expectedPixelHash;
 
     int m_timeout;

Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp

@@ -106,9 +106,9 @@ static void paintRepaintRectOverlay(cairo_surface_t* surface, WKArrayRef repaint
     cairo_destroy(context);
 }
 
-void TestInvocation::dumpPixelsAndCompareWithExpected(WKImageRef, WKArrayRef repaintRects)
+void TestInvocation::dumpPixelsAndCompareWithExpected(WKImageRef image, WKArrayRef repaintRects, SnapshotResultType)
 {
-    cairo_surface_t* surface = WKImageCreateCairoSurface(TestController::singleton().mainWebView()->windowSnapshotImage().get());
+    cairo_surface_t* surface = WKImageCreateCairoSurface(image);
 
     if (repaintRects)
         paintRepaintRectOverlay(surface, repaintRects);

Tools/WebKitTestRunner/cg/TestInvocationCG.cpp

@@ -154,16 +154,9 @@ static void paintRepaintRectOverlay(CGContextRef context, WKImageRef image, WKAr
     CGContextRestoreGState(context);
 }
 
-void TestInvocation::dumpPixelsAndCompareWithExpected(WKImageRef image, WKArrayRef repaintRects)
+void TestInvocation::dumpPixelsAndCompareWithExpected(WKImageRef image, WKArrayRef repaintRects, SnapshotResultType snapshotType)
 {
-    PlatformWebView* webView = TestController::singleton().mainWebView();
-    WKRetainPtr<WKImageRef> windowSnapshot = webView->windowSnapshotImage();
-
-    RetainPtr<CGContextRef> context;
-    if (windowSnapshot)
-        context = adoptCF(createCGContextFromImage(windowSnapshot.get(), DontFlipGraphicsContext));
-    else
-        context = adoptCF(createCGContextFromImage(image));
+    RetainPtr<CGContextRef> context = adoptCF(createCGContextFromImage(image, snapshotType == SnapshotResultType::WebView ? DontFlipGraphicsContext : FlipGraphicsContext));
 
     // A non-null repaintRects array means we're doing a repaint test.
     if (repaintRects)