Bug 179488

Summary: Fix preload/picture-type.html to address race conditions with preloader and necessary DPR test setup
Product: WebKit Reporter: Colin Bendell <colin>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: achristensen, commit-queue, darin, lforschler, ryanhaddad
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 179545    
Bug Blocks:    
Description Flags
Patch none

Description Colin Bendell 2017-11-09 09:23:01 PST
Current test http/preload/picture-type.html can fail because of race conditions from the test setup. In order to test <source sizes srcset> tags, the browser DPR must be fixed. There is a pre-existing deficiency in testRunner.setBackingScaleFactor which requires the browser to reload for the target DPR to be effective in `srcset` evaluation. This unfortunately creates test delays and a race condition with the preloader and the page reload.

A less elegant solution is to adjust the `srcset` targets to account for a wide range of browser DPRs in the tests. This is adequate because this test is not testing the `srcset` evaluation logic but rather that `srcset` continues to work with the additional `type` attribute test.
Comment 1 Colin Bendell 2017-11-09 10:06:57 PST
Created attachment 326463 [details]
Comment 2 Ryan Haddad 2017-11-10 09:07:09 PST
Alex, any chance that you can review this change today?
Comment 3 WebKit Commit Bot 2017-11-10 11:01:42 PST
Comment on attachment 326463 [details]

Clearing flags on attachment: 326463

Committed r224697: <https://trac.webkit.org/changeset/224697>
Comment 4 WebKit Commit Bot 2017-11-10 11:01:44 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Ryan Haddad 2017-11-10 14:00:27 PST
The test is still a flaky failure after this change:

--- /Volumes/Data/slave/sierra-release-tests-wk2/build/layout-test-results/http/tests/preload/picture-type-expected.txt
+++ /Volumes/Data/slave/sierra-release-tests-wk2/build/layout-test-results/http/tests/preload/picture-type-actual.txt
@@ -1,40 +1,41 @@
-PASS internals.isPreloaded('resources/base-image1.png?0'); is true
+FAIL internals.isPreloaded('resources/base-image1.png?0'); should be true. Was false.
 PASS internals.isPreloaded('resources/preload-test.jpg?0'); is false
-PASS internals.isPreloaded('resources/base-image1.png?1'); is true
+FAIL internals.isPreloaded('resources/base-image1.png?1'); should be true. Was false.
 PASS internals.isPreloaded('resources/preload-test.jpg?1'); is false
-PASS internals.isPreloaded('resources/base-image1.png?2'); is true
+FAIL internals.isPreloaded('resources/base-image1.png?2'); should be true. Was false.
 PASS internals.isPreloaded('resources/preload-test.jpg?2'); is false
 PASS internals.isPreloaded('resources/base-image1.png?3'); is false
-PASS internals.isPreloaded('resources/preload-test.jpg?3'); is true
+FAIL internals.isPreloaded('resources/preload-test.jpg?3'); should be true. Was false.
 PASS internals.isPreloaded('resources/base-image1.png?4'); is false
 PASS internals.isPreloaded('resources/base-image2.png?4'); is false
-PASS internals.isPreloaded('resources/base-image3.png?4'); is true
+FAIL internals.isPreloaded('resources/base-image3.png?4'); should be true. Was false.
 PASS internals.isPreloaded('resources/preload-test.jpg?4'); is false
 PASS internals.isPreloaded('resources/base-image1.png?5'); is false
 PASS internals.isPreloaded('resources/base-image2.png?5'); is false
 PASS internals.isPreloaded('resources/base-image3.png?5'); is false
-PASS internals.isPreloaded('resources/preload-test.jpg?5'); is true
-PASS internals.isPreloaded('resources/base-image1.png?6'); is true
+FAIL internals.isPreloaded('resources/preload-test.jpg?5'); should be true. Was false.
+FAIL internals.isPreloaded('resources/base-image1.png?6'); should be true. Was false.
 PASS internals.isPreloaded('resources/preload-test.jpg?6'); is false
 PASS internals.isPreloaded('resources/base-image1.png?7'); is false
-PASS internals.isPreloaded('resources/preload-test.jpg?7'); is true
-PASS internals.isPreloaded('resources/base-image1.png?8'); is true
+FAIL internals.isPreloaded('resources/preload-test.jpg?7'); should be true. Was false.
+FAIL internals.isPreloaded('resources/base-image1.png?8'); should be true. Was false.
 PASS internals.isPreloaded('resources/preload-test.jpg?8'); is false
 PASS internals.isPreloaded('resources/base-image1.png?9'); is false
-PASS internals.isPreloaded('resources/preload-test.jpg?9'); is true
-PASS internals.isPreloaded('resources/base-image1.png?10'); is true
+FAIL internals.isPreloaded('resources/preload-test.jpg?9'); should be true. Was false.
+FAIL internals.isPreloaded('resources/base-image1.png?10'); should be true. Was false.
 PASS internals.isPreloaded('resources/base-image2.png?10'); is false
 PASS internals.isPreloaded('resources/preload-test.jpg?10'); is false
 PASS internals.isPreloaded('resources/base-image1.png?11'); is false
-PASS internals.isPreloaded('resources/base-image2.png?11'); is true
+FAIL internals.isPreloaded('resources/base-image2.png?11'); should be true. Was false.
 PASS internals.isPreloaded('resources/preload-test.jpg?11'); is false
-PASS internals.isPreloaded('resources/base-image1.png?12'); is true
+FAIL internals.isPreloaded('resources/base-image1.png?12'); should be true. Was false.
 PASS internals.isPreloaded('resources/base-image2.png?12'); is false
 PASS internals.isPreloaded('resources/preload-test.jpg?12'); is false
 PASS internals.isPreloaded('resources/base-image1.png?13'); is false
 PASS internals.isPreloaded('resources/base-image2.png?13'); is false
-PASS internals.isPreloaded('resources/preload-test.jpg?13'); is true
+FAIL internals.isPreloaded('resources/preload-test.jpg?13'); should be true. Was false.
 PASS successfullyParsed is true
+Some tests failed.
Comment 6 Ryan Haddad 2017-11-10 14:00:41 PST
I'm going to roll it out again.
Comment 7 WebKit Commit Bot 2017-11-10 14:01:47 PST
Re-opened since this is blocked by bug 179545
Comment 8 Ryan Haddad 2017-11-10 14:05:02 PST

*** This bug has been marked as a duplicate of bug 179231 ***