Last Comment Bug 759703 - Intermittent browser_pluginnotification.js | Test 16b, Should have a click-to-play notification | Test 16c, Should not have a click-to-play notification | Test 16c, Plugin should be activated
: Intermittent browser_pluginnotification.js | Test 16b, Should have a click-to...
Status: RESOLVED FIXED
: intermittent-failure
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla16
Assigned To: David Keeler [:keeler] (use needinfo?)
:
Mentors:
Depends on:
Blocks: 438871
  Show dependency treegraph
 
Reported: 2012-05-30 03:55 PDT by Ed Morley [:emorley]
Modified: 2012-11-25 19:31 PST (History)
3 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix idea (14.45 KB, patch)
2012-06-05 10:00 PDT, David Keeler [:keeler] (use needinfo?)
jaws: review+
Details | Diff | Splinter Review
patch (14.27 KB, patch)
2012-06-07 10:16 PDT, David Keeler [:keeler] (use needinfo?)
jaws: review+
Details | Diff | Splinter Review
patch (14.21 KB, patch)
2012-06-07 14:27 PDT, David Keeler [:keeler] (use needinfo?)
dkeeler: review+
Details | Diff | Splinter Review

Description Ed Morley [:emorley] 2012-05-30 03:55:55 PDT
Rev3 Fedora 12x64 mozilla-inbound debug test mochitest-other on 2012-05-30 02:45:07 PDT for push 8f8639307984

slave: talos-r3-fed64-016

https://tbpl.mozilla.org/php/getParsedLog.php?id=12190816&tree=Mozilla-Inbound

{
TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16a, Should not have a click-to-play notification
--DOMWINDOW == 42 (0x77acb50) [serial = 1036] [outer = 0x7761f50] [url = chrome://mochitests/content/browser/browser/base/content/test/plugin_clickToPlayAllow.html]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16b, Should have a click-to-play notification
Stack trace:
    JS frame :: chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js :: test16b :: line 477
    native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16b, Plugin should not be activated
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16c, Should not have a click-to-play notification
Stack trace:
    JS frame :: chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js :: test16c :: line 488
    native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16c, Plugin should be activated
Stack trace:
    JS frame :: chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js :: test16c :: line 491
    native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16d, Should have a click-to-play notification
TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16d, Plugin should not be activated
WARNING: NS_ENSURE_TRUE(mMutable) failed: file ../../../../netwerk/base/src/nsSimpleURI.cpp, line 258
INFO TEST-END | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | finished in 7056ms
}
Comment 1 Treeherder Robot 2012-06-01 14:49:27 PDT
philor
https://tbpl.mozilla.org/php/getParsedLog.php?id=12287501&tree=Mozilla-Inbound
Rev3 Fedora 12x64 mozilla-inbound debug test mochitest-other on 2012-06-01 13:40:46
slave: talos-r3-fed64-044

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16b, Should have a click-to-play notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16c, Should not have a click-to-play notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16c, Plugin should be activated
Comment 2 Treeherder Robot 2012-06-02 00:47:54 PDT
bz
https://tbpl.mozilla.org/php/getParsedLog.php?id=12301203&tree=Try
Rev3 Fedora 12x64 try debug test mochitest-other on 2012-06-01 23:39:33
slave: talos-r3-fed64-066

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16b, Should have a click-to-play notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16c, Should not have a click-to-play notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16c, Plugin should be activated
Comment 3 David Keeler [:keeler] (use needinfo?) 2012-06-05 10:00:05 PDT
Created attachment 630199 [details] [diff] [review]
fix idea

Ideally, we'd want some kind of event when the plugin is actually in the page and ready to be clicked on. The only event I could find that actually fired was "focus" ("load" doesn't seem to fire for objects or embeds (?!)). So, I set up an interval that would send tab key events. When the plugin shows up in the page, it will eventually be focused by the tabbing, and the listener will go on to the next test. If the plugin doesn't show up after 5 seconds' worth of tabbing, the test moves on.

Also, I thought it would be a good idea to get rid of all other timeouts, because the same thing could happen with them. Most of the time we're waiting for a plugin to be activated, so I added other intervals that basically do the same thing.
Comment 4 Jared Wein [:jaws] (please needinfo? me) 2012-06-05 16:12:15 PDT
Comment on attachment 630199 [details] [diff] [review]
fix idea

Review of attachment 630199 [details] [diff] [review]:
-----------------------------------------------------------------

Interesting solution. Maybe this will work, but the failures are very infrequent so I'm not sure how useful this change is. What do you think?

::: browser/base/content/test/browser_pluginnotification.js
@@ +476,5 @@
>  
>    prepareTest(test16a, gTestRoot + "plugin_bug743421.html");
>  }
>  
> +function sendTab(aWindow) {

please rename this to sendTabKey to disambiguate with browser tabs.

@@ +532,5 @@
> +  var interval = setInterval(function() { 
> +    if (tries < 50) {
> +      sendTab(gTestBrowser.contentWindow); 
> +    } else {
> +      callback();

We shouldn't be hitting these |else| blocks here nor in the other test functions within this file. Can you fail a test if it reaches one of these |else| blocks or if |tries| goes over its limit?
Comment 5 David Keeler [:keeler] (use needinfo?) 2012-06-06 10:54:38 PDT
Thanks for the feedback. I'll make the changes when I get a chance.
I think it's worth it to fix for two reasons: 1) it makes the test faster, because it only waits as long as it has to (within 100ms or whatever we want to set that number to) and 2) fewer intermittent oranges means a better test suite means better development, so it's good for the project.
Comment 6 Treeherder Robot 2012-06-06 15:47:08 PDT
mounir
https://tbpl.mozilla.org/php/getParsedLog.php?id=12429759&tree=Try
Rev3 WINNT 5.1 try debug test mochitest-other on 2012-06-06 12:31:06
slave: talos-r3-xp-072

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16b, Should have a click-to-play notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16c, Should not have a click-to-play notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16c, Plugin should be activated
Comment 7 David Keeler [:keeler] (use needinfo?) 2012-06-07 10:16:59 PDT
Created attachment 631027 [details] [diff] [review]
patch

I refactored things, and I think this is a much better solution. We no longer need that tab-sending business. I wasn't quite sure what you meant about the else blocks, but I made sure the test failed if it waited too long (tries hit the limit). I also sped up the interval, so the test goes even faster.
Comment 8 Jared Wein [:jaws] (please needinfo? me) 2012-06-07 13:16:54 PDT
Comment on attachment 631027 [details] [diff] [review]
patch

Review of attachment 631027 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/test/browser_pluginnotification.js
@@ +252,5 @@
>    ok(!objLoadingContent.activated, "Test 9a, Plugin with id=" + plugin2.id + " should not be activated");
>  
>    EventUtils.synthesizeMouse(plugin1, 100, 100, { });
> +  var objLoadingContent = plugin1.QueryInterface(Ci.nsIObjectLoadingContent);
> +  var condition = function() { return objLoadingContent.activated; };

This could use an expression closure here if you wanted. E.g. |var condition = function() objLoadingContent.activated;| See https://developer.mozilla.org/en/New_in_JavaScript_1.8#Expression_closures_%28Merge_into_own_page.2Fsection%29 for more information. No added benefit, but I just thought I could mention this since it's cool and is less typing :)
Comment 9 David Keeler [:keeler] (use needinfo?) 2012-06-07 14:27:23 PDT
Created attachment 631138 [details] [diff] [review]
patch

Decided to use expression closures - carrying over r+
Try run: https://tbpl.mozilla.org/?tree=Try&rev=115fa4447f2b
Comment 10 Treeherder Robot 2012-06-09 02:58:21 PDT
neil%parkwaycc.co.uk
https://tbpl.mozilla.org/php/getParsedLog.php?id=12504607&tree=Try
Rev4 MacOSX Lion 10.7 try debug test mochitest-other on 2012-06-08 16:02:26
slave: talos-r4-lion-027

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16b, Should have a click-to-play notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16c, Should not have a click-to-play notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_pluginnotification.js | Test 16c, Plugin should be activated
Comment 11 Ryan VanderMeulen [:RyanVM] 2012-06-09 09:27:18 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/700a03cd53d4
Comment 12 Ryan VanderMeulen [:RyanVM] 2012-06-09 19:38:59 PDT
https://hg.mozilla.org/mozilla-central/rev/700a03cd53d4

Note You need to log in before you can comment on or make changes to this bug.