[iOS] REGRESSION (r174642): DumpRenderTree.app test may dump result twice
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Jan 2015 03:29:13 +0000 (03:29 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Jan 2015 03:29:13 +0000 (03:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=139685
<rdar://problem/19281317>

Reviewed by Simon Fraser.

Fixes an issue where a test in DumpRenderTree.app may dump its result twice. In particular,
the test LayoutTests/fast/dom/gc-10.html may dump its result twice.

Following <http://trac.webkit.org/changeset/174642>, we dump the test result asynchronously
as opposed to synchronously. So, the WebThread or the main thread may perform other tasks
before DRT dumps the output of a test. In particular, the WebThread may start a new page
load (say, as a result of continued JavaScript execution), which will ultimately lead to
dumping the test result again. Instead we want DRT to dump the test result synchronously
such that we capture the state of the web page either when the page is loaded or when
window.testRunner.notifyDone() is called.

* DumpRenderTree/ios/PixelDumpSupportIOS.mm:
(createBitmapContextFromWebView): Moved logic to re-enable tile painting and update the
state of the display from here to updateDisplay(). Added call to -[CATransaction flush] to
flush CA transactions to the window.
* DumpRenderTree/mac/DumpRenderTree.mm:
(updateDisplay): Added iOS-specific code to update tiles and tell WebKit to flush
compositing changes to ensure that we have up-to-date tile content for a pixel/ref-test.
(dump): Renamed; formerly named dumpTestResults().
(displayWebView): Removed outdated FIXME comment. Added FIXME comment to investigate
enabling repaint support on iOS.
(-[DumpRenderTree _deferDumpToMainThread]): Deleted.
(-[DumpRenderTree _waitForWebThreadThenDump]): Deleted.
(dumpTestResults): Deleted.

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

Tools/ChangeLog
Tools/DumpRenderTree/ios/PixelDumpSupportIOS.mm
Tools/DumpRenderTree/mac/DumpRenderTree.mm

index 604796c5397b720eed170068f9a4ffc53af7a4ce..c9231fdca1437f4c37e1f4dc11d6285f6d091ceb 100644 (file)
@@ -1,3 +1,36 @@
+2015-01-15  Daniel Bates  <dabates@apple.com>
+
+        [iOS] REGRESSION (r174642): DumpRenderTree.app test may dump result twice
+        https://bugs.webkit.org/show_bug.cgi?id=139685
+        <rdar://problem/19281317>
+
+        Reviewed by Simon Fraser.
+
+        Fixes an issue where a test in DumpRenderTree.app may dump its result twice. In particular,
+        the test LayoutTests/fast/dom/gc-10.html may dump its result twice.
+
+        Following <http://trac.webkit.org/changeset/174642>, we dump the test result asynchronously
+        as opposed to synchronously. So, the WebThread or the main thread may perform other tasks
+        before DRT dumps the output of a test. In particular, the WebThread may start a new page
+        load (say, as a result of continued JavaScript execution), which will ultimately lead to
+        dumping the test result again. Instead we want DRT to dump the test result synchronously
+        such that we capture the state of the web page either when the page is loaded or when
+        window.testRunner.notifyDone() is called.
+
+        * DumpRenderTree/ios/PixelDumpSupportIOS.mm:
+        (createBitmapContextFromWebView): Moved logic to re-enable tile painting and update the
+        state of the display from here to updateDisplay(). Added call to -[CATransaction flush] to
+        flush CA transactions to the window.
+        * DumpRenderTree/mac/DumpRenderTree.mm:
+        (updateDisplay): Added iOS-specific code to update tiles and tell WebKit to flush
+        compositing changes to ensure that we have up-to-date tile content for a pixel/ref-test.
+        (dump): Renamed; formerly named dumpTestResults().
+        (displayWebView): Removed outdated FIXME comment. Added FIXME comment to investigate
+        enabling repaint support on iOS.
+        (-[DumpRenderTree _deferDumpToMainThread]): Deleted.
+        (-[DumpRenderTree _waitForWebThreadThenDump]): Deleted.
+        (dumpTestResults): Deleted.
+
 2015-01-15  Brent Fulgham  <bfulgham@apple.com>
 
         [Win] Miscellaneous DRT fixes
 2015-01-15  Brent Fulgham  <bfulgham@apple.com>
 
         [Win] Miscellaneous DRT fixes
index 399141a69eaa07773e70f2ece6cad8c5b5a3aaa4..278d3477f453049c4c36db04c9dd6f8bfed9ea0d 100644 (file)
@@ -80,9 +80,9 @@ void dumpBitmap(BitmapContext* context, const char* checksum)
 PassRefPtr<BitmapContext> createBitmapContextFromWebView(bool onscreen, bool incrementalRepaint, bool sweepHorizontally, bool drawSelectionRect)
 {
     // TODO: <rdar://problem/6558366> DumpRenderTree: Investigate testRepaintSweepHorizontally and dumpSelectionRect
 PassRefPtr<BitmapContext> createBitmapContextFromWebView(bool onscreen, bool incrementalRepaint, bool sweepHorizontally, bool drawSelectionRect)
 {
     // TODO: <rdar://problem/6558366> DumpRenderTree: Investigate testRepaintSweepHorizontally and dumpSelectionRect
+
     WebThreadLock();
     WebThreadLock();
-    [gWebBrowserView layoutIfNeeded]; // Re-enables tile painting, which was disabled when committing the frame load.
-    [gWebBrowserView setNeedsDisplay];
+    [CATransaction flush];
 
     UIGraphicsBeginImageContextWithOptions([[mainFrame webView] frame].size, YES /* opaque */, [gDrtWindow screenScale]);
     [[gWebBrowserView layer] renderInContext:UIGraphicsGetCurrentContext()];
 
     UIGraphicsBeginImageContextWithOptions([[mainFrame webView] frame].size, YES /* opaque */, [gDrtWindow screenScale]);
     [[gWebBrowserView layer] renderInContext:UIGraphicsGetCurrentContext()];
index df161f7d506c2b0e72b741aaf41ca52d0397ef7e..d25e07884de5ba3a2df5450e7583a89cce76301a 100644 (file)
@@ -151,8 +151,11 @@ using namespace std;
 @end
 #endif
 
 @end
 #endif
 
+@interface WebView (Details)
+- (BOOL)_flushCompositingChanges;
+@end
+
 static void runTest(const string& testURL);
 static void runTest(const string& testURL);
-static void dumpTestResults();
 
 // Deciding when it's OK to dump out the state is a bit tricky.  All these must be true:
 // - There is no load in progress
 
 // Deciding when it's OK to dump out the state is a bit tricky.  All these must be true:
 // - There is no load in progress
@@ -1220,15 +1223,6 @@ static const char **_argv;
     [self performSelectorOnMainThread:@selector(_runDumpRenderTree) withObject:nil waitUntilDone:NO];
 }
 
     [self performSelectorOnMainThread:@selector(_runDumpRenderTree) withObject:nil waitUntilDone:NO];
 }
 
-- (void)_deferDumpToMainThread
-{
-    ASSERT(WebThreadIsCurrent());
-    
-    dispatch_async(dispatch_get_main_queue(), ^{
-        dump();
-    });
-}
-
 - (void)applicationDidEnterBackground:(UIApplication *)application
 {
     /* Apps will get suspended or killed some time after entering the background state but we want to be able to run multiple copies of DumpRenderTree. Periodically check to see if our remaining background time dips below a threshold and create a new background task.
 - (void)applicationDidEnterBackground:(UIApplication *)application
 {
     /* Apps will get suspended or killed some time after entering the background state but we want to be able to run multiple copies of DumpRenderTree. Periodically check to see if our remaining background time dips below a threshold and create a new background task.
@@ -1284,12 +1278,6 @@ static const char **_argv;
     }
 }
 
     }
 }
 
-- (void)_waitForWebThreadThenDump
-{
-    [self _waitForWebThread];
-    dumpTestResults();
-}
-
 @end
 #endif
 
 @end
 #endif
 
@@ -1565,32 +1553,19 @@ bool shouldSetWaitToDumpWatchdog()
 static void updateDisplay()
 {
     WebView *webView = [mainFrame webView];
 static void updateDisplay()
 {
     WebView *webView = [mainFrame webView];
-
+#if PLATFORM(IOS)
+    [gWebBrowserView layoutIfNeeded]; // Re-enables tile painting, which was disabled when committing the frame load.
+    [gDrtWindow layoutTilesNow];
+    [webView _flushCompositingChanges];
+#else
     if ([webView _isUsingAcceleratedCompositing])
         [webView display];
     else
         [webView displayIfNeeded];
     if ([webView _isUsingAcceleratedCompositing])
         [webView display];
     else
         [webView displayIfNeeded];
-}
-
-void dump()
-{
-#if PLATFORM(IOS)
-    // This can get called on the web thread if from a JavaScript notifyDone().
-    if (WebThreadIsCurrent()) {
-        [(DumpRenderTree *)UIApp _deferDumpToMainThread];
-        return;
-    }
-
-    // Allow the web thread to run before dumping. We can't call -_waitForWebThread directly since we may
-    // be inside a delegate callback.
-    [UIApp performSelectorOnMainThread:@selector(_waitForWebThreadThenDump) withObject:nil waitUntilDone:NO];
-    return;
 #endif
 #endif
-
-    dumpTestResults();
 }
 
 }
 
-void dumpTestResults()
+void dump()
 {
 #if PLATFORM(IOS)
     WebThreadLock();
 {
 #if PLATFORM(IOS)
     WebThreadLock();
@@ -1980,11 +1955,11 @@ void displayWebView()
 #if !PLATFORM(IOS)
     WebView *webView = [mainFrame webView];
     [webView display];
 #if !PLATFORM(IOS)
     WebView *webView = [mainFrame webView];
     [webView display];
-    
+
+    // FIXME: Tracking repaints is not specific to Mac. We should enable such support on iOS.
     [webView setTracksRepaints:YES];
     [webView resetTrackedRepaints];
 #else
     [webView setTracksRepaints:YES];
     [webView resetTrackedRepaints];
 #else
-    // FIXME: <rdar://problem/5106253> DumpRenderTree: fix DRT and ImageDiff to re-enable pixel tests
     [gDrtWindow layoutTilesNow];
     [gDrtWindow setNeedsDisplayInRect:[gDrtWindow frame]];
     [CATransaction flush];
     [gDrtWindow layoutTilesNow];
     [gDrtWindow setNeedsDisplayInRect:[gDrtWindow frame]];
     [CATransaction flush];