Bug 92824

(In reply to comment #0) > @rniwa: in http://trac.webkit.org/changeset/119194 you added this comment to this expectation: > > // The result of jquery/traversing.html is bleeding into this test. > BUGDPRANKE : media/video-poster-blocked-by-willsendrequest.html = TEXT PASS > > Do you remember what you saw that made you say that, or do you have any other state on this? I looked at the flakiness dashboard results.

(In reply to comment #1) > (In reply to comment #0) > > @rniwa: in http://trac.webkit.org/changeset/119194 you added this comment to this expectation: > > > > // The result of jquery/traversing.html is bleeding into this test. > > BUGDPRANKE : media/video-poster-blocked-by-willsendrequest.html = TEXT PASS > > > > Do you remember what you saw that made you say that, or do you have any other state on this? > > I looked at the flakiness dashboard results. I don't follow; is there something on http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&showLargeExpectations=true&tests=media%2Fvideo-poster-blocked-by-willsendrequest.html%20 that mentions jquery/traversing.html ?

There used to be. Maybe this bug no longer reproduces.

We also found this happening with a console.log from platform/chromium/fast/forms/color/color-suggestion-picker-appearance.html bleeding into platform/chromium/fast/loader/create-view-target-blank.html . The latter test's output would get a spurious "CONSOLE MESSAGE: line 36: [object PagePopupController]" from the former test.

Note: That same test also bleeds into platform/chromium/fast/text/international/complex-text-rectangle.html sometimes.

Context: IRC conversation around 22:40-22:54 July 31 (pacific) included the wisdom that tests' console.log() & alert() sometimes bleed into subsequent tests, and that this has been the case for a very long time, and that nobody knows exactly why this happens, though a suspicion is that these APIs are too async for the way DRT works (test calls console.log() and immediately endTest(), but DRT/WebKit don't flush outstanding async work, which gets executed in the context of the next test). Given that context, I don't think bleeding is still a problem here. The test in question (media/video-poster-blocked-by-willsendrequest.html) is failing almost 100% on the bots, but looking like it indicates a real bug (poster isn't being blocked by willsendrequest).

> the wisdom that tests' console.log() & alert() sometimes bleed into subsequent tests Is that Chromium specific? We have a lot of tests that alert(), and I don't think that it happened with Mac WK1 or WK2 any time recently.

(In reply to comment #7) > > the wisdom that tests' console.log() & alert() sometimes bleed into subsequent tests > > Is that Chromium specific? I don't know, but my impression from IRC was that Eric Seidel was saying it was not port-specific. Eric: anything you want to add to my summary above?

I believe alert() and console.log() are synchronous in Mac WK/DRT. I don't know why they wouldn't be for Chromium, but I don't know much about that part of the code. These are the callbacks from WebCore: http://trac.webkit.org/browser/trunk/Source/WebCore/page/ChromeClient.h#L140 http://trac.webkit.org/browser/trunk/Source/WebCore/page/ChromeClient.h#L133 We'd have to look at the chromium implementation of those to know if they're async.

Figured out the problem with this test on the bots: if content/abe.png is fetched by any other test before the test in question runs, then even though the test blocks the load (I verified that WebViewHost.cpp:blockRequest() is called even when the test fails) the media element finds the resource's dimensions and fails the test. Quick repro of the problem: rm -rf /tmp/x2 && time ./Tools/Scripts/new-run-webkit-tests --child-processes=1 --debug --no-retry --iterations=2 --results-directory=/tmp/x2 media/video-poster-blocked-by-willsendrequest.html media/video-poster.html I'm going to land a patch that appends a random query string param to the URL for this test and filed 93195 to fix the apparent bug in chromium's DRT where it allows caches to persist between tests.

Created attachment 156552 [details] Patch

Created attachment 156553 [details] Patch

Attachment 156553 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/medi..." exit_code: 1 LayoutTests/ChangeLog:13: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] LayoutTests/ChangeLog:14: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 156554 [details] Patch

Comment on attachment 156554 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156554&action=review > LayoutTests/media/video-poster-blocked-by-willsendrequest.html:43 > + video.poster = "content/abe.png?" + Math.random(); Can we append timestamp instead of Math.random() so that it'll be different without doubt? i.e. P(query being same) === 0 instead of arbitrary small epsilon?

Created attachment 156555 [details] Patch

(In reply to comment #15) > (From update of attachment 156554 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=156554&action=review > > > LayoutTests/media/video-poster-blocked-by-willsendrequest.html:43 > > + video.poster = "content/abe.png?" + Math.random(); > > Can we append timestamp instead of Math.random() so that it'll be different without doubt? i.e. P(query being same) === 0 instead of arbitrary small epsilon? Good point. Done.

Comment on attachment 156555 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156555&action=review > LayoutTests/media/video-poster-blocked-by-willsendrequest.html:42 > + // Since this is testing resource-loading being blocked, append a unique query string to avoid cache hits from other tests. > + // FIXME: it's madness that this needs to account for other tests. > + // Why isn't the cache being cleared between tests? Looks like port-specific DRT bugs (chromium: 93195, mac: 82976, gtk: 79760). You may want to make this comment more concise.

Created attachment 156561 [details] Patch

(In reply to comment #18) > (From update of attachment 156555 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=156555&action=review > > > LayoutTests/media/video-poster-blocked-by-willsendrequest.html:42 > > + // Since this is testing resource-loading being blocked, append a unique query string to avoid cache hits from other tests. > > + // FIXME: it's madness that this needs to account for other tests. > > + // Why isn't the cache being cleared between tests? Looks like port-specific DRT bugs (chromium: 93195, mac: 82976, gtk: 79760). > > You may want to make this comment more concise. Done. Thanks for the review.

Comment on attachment 156561 [details] Patch Clearing flags on attachment: 156561 Committed r124717: <http://trac.webkit.org/changeset/124717>

All reviewed patches have been landed. Closing bug.