Bug 141364

Summary: fullscreen/full-screen-plugin.html is very flaky on Yosemite WK2
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: REOPENED ---    
Severity: Normal CC: commit-queue, koivisto, zalan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=134489
Description Flags
speculative fix
second try sam: review+, commit-queue: commit-queue-

Description Alexey Proskuryakov 2015-02-07 15:49:39 PST
fullscreen/full-screen-plugin.html fails on Yosemite WK2 about 50% of the time: <http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&showExpectations=true&revision=179781&tests=fullscreen%2Ffull-screen-plugin.html>.

This test was added in <http://trac.webkit.org/r170717>.

@@ -1,3 +1,3 @@
 Test that plugin doesn't restart when taking it to full screen and back.
Comment 1 Alexey Proskuryakov 2015-02-07 19:41:14 PST
Created attachment 246230 [details]
speculative fix
Comment 2 WebKit Commit Bot 2015-02-07 21:24:31 PST
Comment on attachment 246230 [details]
speculative fix

Clearing flags on attachment: 246230

Committed r179794: <http://trac.webkit.org/changeset/179794>
Comment 3 WebKit Commit Bot 2015-02-07 21:24:36 PST
All reviewed patches have been landed.  Closing bug.
Comment 4 Alexey Proskuryakov 2015-02-08 11:22:44 PST
This made the test fail differently, shedding more light on why it failed before:

-Test that plugin doesn't restart when taking it to full screen and back.
+CONSOLE MESSAGE: line 26: TypeError: undefined is not an object (evaluating 'testObject.property = 'foo'')
+FAIL: Timed out waiting for notifyDone to be called

Apparently, testObject is not necessarily available even by the time the load event fires.
Comment 5 Alexey Proskuryakov 2015-02-08 11:28:49 PST
Created attachment 246244 [details]
second try
Comment 6 Sam Weinig 2015-02-08 13:37:57 PST
Comment on attachment 246244 [details]
second try

View in context: https://bugs.webkit.org/attachment.cgi?id=246244&action=review

> LayoutTests/fullscreen/full-screen-plugin.html:65
> +function checkForPlugin()
> +{
> +    var plugin = document.getElementById('plugin');
> +    if (plugin.testObject)
> +        test();
> +    else
> +        setTimeout(checkForPlugin, 100);
> +}

It seems like it might make sense for the test plugin to have a way to indicate it is ready without the need to poll. Maybe the plugin could look for a predefined function name (e.g. function pluginIsAvailable()) and call it when it has been instantiated. I can't imagine this is the only place where this mistake is made.
Comment 7 Alexey Proskuryakov 2015-02-08 17:19:14 PST
I thought that plug-ins were supposed to be available in onload, however maybe some of the async layout patches broke that. Or maybe it never worked correctly in WebKit.
Comment 8 WebKit Commit Bot 2015-02-08 17:20:42 PST
Comment on attachment 246244 [details]
second try

Rejecting attachment 246244 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 246244, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.appspot.com/results/5013007134359552
Comment 9 Alexey Proskuryakov 2015-02-08 17:25:31 PST
Committed <http://trac.webkit.org/r179811>.
Comment 10 Alexey Proskuryakov 2015-02-10 18:12:43 PST
Hmm, it still fails.

@@ -1,3 +1,2 @@
-Test that plugin doesn't restart when taking it to full screen and back.
+FAIL: Timed out waiting for notifyDone to be called
Comment 11 Alexey Proskuryakov 2015-02-16 13:47:48 PST
Marked as flaky in <http://trac.webkit.org/changeset/180170>.

Antti, this is a test for your fix, please make it work correctly if you can.