Closed Bug 1259908 Opened 4 years ago Closed Last year

Intermittent mixedcontentblocker/test_main.html | application timed out after 330 seconds with no output

Categories

(Core :: DOM: Security, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- fixed
firefox63 --- fixed
firefox64 --- fixed

People

(Reporter: aryx, Assigned: baku)

References

Details

(Keywords: intermittent-failure, Whiteboard: [domsecurity-intermittent])

Attachments

(2 files)

https://treeherder.mozilla.org/logviewer.html#?job_id=8260160&repo=fx-team

19:23:58     INFO -  [NPAPI 1060] WARNING: NS_ENSURE_TRUE(InitStaticMembers()) failed: file c:/builds/moz2_slave/fx-team-w32-d-0000000000000000/build/src/modules/libpref/Preferences.cpp, line 1355
19:29:27     INFO -  [3092] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file c:/builds/moz2_slave/fx-team-w32-d-0000000000000000/build/src/toolkit/components/places/Database.cpp, line 510
19:29:27     INFO -  [3092] WARNING: NS_ENSURE_TRUE(mDB) failed: file c:/builds/moz2_slave/fx-team-w32-d-0000000000000000/build/src/toolkit/components/places/nsNavHistory.cpp, line 295
19:29:27     INFO -  JavaScript error: resource://gre/modules/FormHistory.jsm, line 375: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]
19:29:27     INFO -  [3092] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file c:/builds/moz2_slave/fx-team-w32-d-0000000000000000/build/src/toolkit/components/places/Database.cpp, line 510
19:29:27     INFO -  [3092] WARNING: NS_ENSURE_TRUE(mDB) failed: file c:/builds/moz2_slave/fx-team-w32-d-0000000000000000/build/src/toolkit/components/places/nsNavHistory.cpp, line 295
19:29:27     INFO -  JavaScript error: resource://gre/modules/PlacesUtils.jsm, line 1959: NS_ERROR_XPC_GS_RETURNED_FAILURE: Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]
19:29:27     INFO -  [3092] WARNING: 'NS_FAILED(rv)', file c:/builds/moz2_slave/fx-team-w32-d-0000000000000000/build/src/dom/quota/ActorsParent.cpp, line 2121
19:29:27     INFO -  [3092] WARNING: 'NS_FAILED(rv)', file c:/builds/moz2_slave/fx-team-w32-d-0000000000000000/build/src/dom/quota/ActorsParent.cpp, line 2272
19:29:27     INFO -  [3092] WARNING: '!quotaManager', file c:/builds/moz2_slave/fx-team-w32-d-0000000000000000/build/src/dom/quota/ActorsParent.cpp, line 5174
19:29:28     INFO -  502 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | xhr did not follow block_active_content pref
19:29:28     INFO -  503 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | iframe did not follow block_active_content pref
19:29:28     INFO -  504 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | image did not follow block_display_content pref
19:29:28     INFO -  505 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | media did not follow block_display_content pref
19:29:28     INFO -  506 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | imageSrcset did not follow block_active_content pref
19:29:28     INFO -  507 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | imageSrcsetFallback did not follow block_active_content pref
19:29:28     INFO -  508 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | imagePicture did not follow block_active_content pref
19:29:28     INFO -  509 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | imageJoinPicture did not follow block_active_content pref
19:29:28     INFO -  510 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | imageLeavePicture did not follow block_display_content pref
19:29:28     INFO -  511 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | object did not follow block_active_content pref
19:29:28     INFO -  512 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | script did not follow block_active_content pref
19:29:28     INFO -  513 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | stylesheet did not follow block_active_content pref
19:29:28     INFO -  514 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | image did not follow block_display_content pref
19:29:28     INFO -  515 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | media did not follow block_display_content pref
19:29:28     INFO -  516 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | object did not follow block_active_content pref
19:29:28     INFO -  517 INFO TEST-PASS | dom/security/test/mixedcontentblocker/test_main.html | stylesheet did not follow block_active_content pref
19:29:28  WARNING -  TEST-UNEXPECTED-TIMEOUT | dom/security/test/mixedcontentblocker/test_main.html | application timed out after 330 seconds with no output
Component: Security → DOM: Security
Whiteboard: [domsecurity-intermittent]
I see this quite frequently when running tests on windows 7-vm.  Why?  I am not sure.  I suspect it is related to the frequent failures we see on plugins on win7-vm, but I have no proof of that.

here is 60 jobs of mochitest-3 with 8 instances of this failing:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b797b32b88b2ab42b126bcb434155f3216d18a95&selectedJob=25874002&filter-searchStr=debug%20mochitest-3

that is >10% failure rate, ouch!  Looking at the logs, we fail in the same spot which happens to be the same failure as the bug had on initial filing from comment 0 :)

I would rather fix this up to work, but we could disable it for windows debug, or consider moving it to the 'clipboard' job which is more of a catchall for problems on the VM since that job is running on hardware.

:ckerschb, would you be able to help look into why this is a problem and finding a solution or approving some temporary measure (like disabling or moving it)?
Flags: needinfo?(ckerschb)
(In reply to Joel Maher ( :jmaher) from comment #15)
> :ckerschb, would you be able to help look into why this is a problem and
> finding a solution or approving some temporary measure (like disabling or
> moving it)?

I took a quick look (and I remember I have looked into this test before). The problem is that this test relies on SetTimeOut(), which can be really flaky on VMs. I suppose we would have to fundamentally restructure this test to make it work, but from what I remember the problem is related to blocking object, which is hard test without using SetTimeOut().

In general I am not a big supporter of disabling tests bug having a failure rate of > 10% is obviously not that great. I don't know the process of moving things to the 'clipboard'. I am not even completely sure what that means to be honest, but if that means the test would still be run, that would be fine with me.

Ultimately, Tanvi is the de facto owner of mixed content blocker. Tanvi what do you think? How should be proceed?
Flags: needinfo?(ckerschb) → needinfo?(tanvi)
moving this test to the 'clipboard' job is just changing a line in the manifest- eventually we will need to fix it, that is more of a short term solution to get us running tests faster in AWS vs struggling to fix every single issue.
(In reply to Joel Maher ( :jmaher) from comment #17)
> moving this test to the 'clipboard' job is just changing a line in the
> manifest- eventually we will need to fix it, that is more of a short term
> solution to get us running tests faster in AWS vs struggling to fix every
> single issue.

Thanks Joel for the clarification. I would actually be fine with that. Let's wait what Tanvi replies, but that sounds like the best short term solution. At least until we find someone to restructure the test.
(In reply to Joel Maher ( :jmaher) from comment #17)
> moving this test to the 'clipboard' job is just changing a line in the
> manifest- eventually we will need to fix it, that is more of a short term
> solution to get us running tests faster in AWS vs struggling to fix every
> single issue.

I still don't quite get what moving to clipboard means.  is it just moved to "clipboard" for windows vms?  Is the test still run normally on all other platforms?

This is the main mixed content blocker test.  What you could do alternatively is pull out the plugin part temporarily instead of disabling the whole test.  It would be nice if their were todo() tests that were depending on platform, because then you could just make the plugin test todo() for windows vms.
Flags: needinfo?(tanvi)
on windows VMs we have a timing issue with the system clipboard and this has forced us to create a clipboard job that runs on hardware- since then we have moved the dom plugin tests there as well.  The clipboard job is only different on windows 7 as all linux tests are already run on VMs and the other windows platforms are not targets for VMs yet (although windows 10 in the near future will be).  So moving it would just put it in a different job.

A couple of options based on your suggestion:
1) detect in the test if we are running in windows and skip the plugin bits
2) split the test into 2 parts (pull the plugin stuff into its own test) and skip that on windows in a manifest file

both of those above options would not distinguish between windows xp/7/8, so it would be skipped on all instances.  As an interesting point, this only shows up on debug, so possibly we can detect windows and debug.

Please advise, I have a review up for the easy solution of a move, I am happy to cancel that and edit the test.
(In reply to Joel Maher ( :jmaher) from comment #21)
> on windows VMs we have a timing issue with the system clipboard and this has
> forced us to create a clipboard job that runs on hardware- since then we
> have moved the dom plugin tests there as well.  The clipboard job is only
> different on windows 7 as all linux tests are already run on VMs and the
> other windows platforms are not targets for VMs yet (although windows 10 in
> the near future will be).  So moving it would just put it in a different job.
> 
> A couple of options based on your suggestion:
> 1) detect in the test if we are running in windows and skip the plugin bits
> 2) split the test into 2 parts (pull the plugin stuff into its own test) and
> skip that on windows in a manifest file
> 
> both of those above options would not distinguish between windows xp/7/8, so
> it would be skipped on all instances.  As an interesting point, this only
> shows up on debug, so possibly we can detect windows and debug.
> 
> Please advise, I have a review up for the easy solution of a move, I am
> happy to cancel that and edit the test.

Since test_main is supposed to cover all content types, it would be best if we kept the plugin parts in there.  We could skip that portion of the test if windows and debug.  Then in addition, we could add a test_plugins that is marked as clipboard job and only run for windows and debug.  This solution gives us the best coverage, but is also the most work.

The other option is to just pull plugins completely out of test_main.  Have a second test for test_plugins that is run normally in general, and run on the clipboard in windows debug mode.  I prefer the above but would be okay with this too.
in looking at the test, I see:
https://dxr.mozilla.org/mozilla-central/source/dom/security/test/mixedcontentblocker/test_main.html?q=path%3Atest_main.html&redirect_type=single#14

where we set plugin state to enabled, but I don't see specifically where we call plugins.  I really don't understand the options above.  I am not proposing dropping any coverage with the patch, just moving the test to a different job.  If there is a simple way to split the test up so all plugin stuff can be in test_main_plugins.html, etc. I am fine with that as well.  Right now I am not sure where the plugin stuff is other than the single line referenced above.
Flags: needinfo?(tanvi)
(In reply to Joel Maher ( :jmaher) from comment #24)
> in looking at the test, I see:
> https://dxr.mozilla.org/mozilla-central/source/dom/security/test/
> mixedcontentblocker/test_main.html?q=path%3Atest_main.
> html&redirect_type=single#14
> 
> where we set plugin state to enabled, but I don't see specifically where we
> call plugins.  I really don't understand the options above.  I am not
> proposing dropping any coverage with the patch, just moving the test to a
> different job.  If there is a simple way to split the test up so all plugin
> stuff can be in test_main_plugins.html, etc. I am fine with that as well. 
> Right now I am not sure where the plugin stuff is other than the single line
> referenced above.

Spoke to jmaher on irc.  He is going to try and create test_main_plugins (and potentially file_main_plugin) for all the "object" parts of test_main (and file_main).

http://searchfox.org/mozilla-central/source/dom/security/test/mixedcontentblocker/file_main.html
http://searchfox.org/mozilla-central/source/dom/security/test/mixedcontentblocker/test_main.html
Flags: needinfo?(tanvi)
I verified for my primary use case that on windows 7-vm, that removing plugin/object works well:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bda046fa363ab3c1517d6e3be78a21b0a0f67a86

then I verified on all platforms that mochitest-plain works well as well as the new test in the clipboard job:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7ee1119d702a2643e7d0a98144e8457cbdc1fdc
:tanvi, not sure if you have seen this review request- I figured it has been a few days and I would check in with you.
Flags: needinfo?(tanvi)
Comment on attachment 8782548 [details]
Bug 1259908 - split plugin to own test and run in clipboard job temporarily.

https://reviewboard.mozilla.org/r/72670/#review72460

Overall looks good, but I'd like to see another version.  I won't be available next week, so if the second review needs to be done sooner than later, please flag Christoph.  Thanks!

::: dom/security/test/mixedcontentblocker/file_main.html:18
(Diff revision 3)
>  
>  <!-- types the Mixed Content Blocker can block
>       /*
>    switch (aContentType) {
> -  case nsIContentPolicy::TYPE_OBJECT:
>    case nsIContentPolicy::TYPE_SCRIPT:

Can you include TYPE_OBJECT in the comment, and then add text explainign that TYPE_OBJECT tests had to be pulled out into a different test.


It may even be worth commenting out the TYPE_OBJECT parts instead of removing them.  Then later, when plugins are fixed in windows vms, we can reunify the tests easily.

::: dom/security/test/mixedcontentblocker/file_main.html:49
(Diff revision 3)
>    var TIMEOUT_INTERVAL = 100;
>  
>    var testContent = document.getElementById("testContent");
>  
>    /* Part 1: Mixed Script tests */
>  

Leave // Test 1a: insecure object
Then add
// insecure object test was moved to ... etc.

::: dom/security/test/mixedcontentblocker/file_main_plugin.html:15
(Diff revision 3)
> +  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
> +</head>
> +<body>
> +<div id="testContent"></div>
> +
> +<!-- types the Mixed Content Blocker can block

Remove this comment.  Instead add a comment about how this is testing mixed plugins / TYPE_OBJECT

::: dom/security/test/mixedcontentblocker/file_main_plugin.html:32
(Diff revision 3)
> +  var MAX_COUNT = 100;
> +  var TIMEOUT_INTERVAL = 100;
> +
> +  var testContent = document.getElementById("testContent");
> +
> +  /* Part 1: Mixed Script tests */

remove comment

::: dom/security/test/mixedcontentblocker/file_main_plugin.html:34
(Diff revision 3)
> +
> +  var testContent = document.getElementById("testContent");
> +
> +  /* Part 1: Mixed Script tests */
> +
> +  // Test 1a: insecure object

remove comment

::: dom/security/test/mixedcontentblocker/test_main.html
(Diff revision 3)
>    <title>Tests for Bug 62178</title>
>    <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
>    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
>  
>    <script>
> -  SpecialPowers.setTestPluginEnabledState(SpecialPowers.Ci.nsIPluginTag.STATE_ENABLED, "Test Plug-in");

Again, we may just want to comment out all the object parts in this file instead of removing them.

::: dom/security/test/mixedcontentblocker/test_main_plugin.html:82
(Diff revision 3)
> +
> +    log("test: "+event.data.test+", msg: "+event.data.msg + " logging message.");
> +    // test that the load type matches the pref for this type of content
> +    // (i.e. active vs. display)
> +
> +    switch(event.data.test) {

You don't need this switch anymore, since object is the only case.

::: dom/security/test/mixedcontentblocker/test_main_plugin.html:96
(Diff revision 3)
> +    checkTestsCompleted();
> +  }
> +
> +  function startTest() {
> +    //Set the first set of mixed content settings and increment the counter.
> +    //Enable <picture> and <img srcset> for the test.

I don't think you need to touch these image prefs here.
Attachment #8782548 - Flags: review?(tanvi) → review-
Flags: needinfo?(tanvi)
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
Attached patch race.patchSplinter Review
This test is quite complex and it has several bugs:

1. there was not a correlation between the 'lastRequest' check and the real last request. In order to fix this I introduce a unique ID per request. When checking the last request, the unique ID must match.

2. the same 'lastRequest' data was sent over and over again without resetting the internal data. Now 'lastRequest' data is set to "" after the first 'lastRequest' request. If called again, sjs returns an error.

3. There was not error reporting in the sjs file. Now if the sjs is called wrongly, it returns a 400. This was useful when debugging and I would like to keep it.

4. some tests ignored the return value of imgHandlers(). This was probably the main bug.

last point: I introduce some helper functions: report(), is() and ok().
Assignee: nobody → amarchesini
Attachment #9012765 - Flags: review?(ehsan)
Comment on attachment 9012765 [details] [diff] [review]
race.patch

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

Stealing review from Ehsan. Looks sane to me. Thanks for the improvements to the test. r=me
Attachment #9012765 - Flags: review?(ehsan) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/87cd2cf69eb5
Fix intermittent failure for dom/security/test/mixedcontentblocker/test_main.html, r=ckerschb
https://hg.mozilla.org/mozilla-central/rev/87cd2cf69eb5
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.