487402 - test_dynUnsecureRedirect.html intermittently fails, change timeouts to callbacks where possible

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Honza Bambas (:mayhemer)]

Even bug 480713 has been fixed, we are still getting mixed content detection failures. I am enable to reproduce locally, so it will take time to figure out what's going on here.

For now this test has been disabled.

Comment 1

[Honza Bambas (:mayhemer)]

Attachment: v1

It more looks like a test is wrong in this case. I am using timeout instead of onload. This is a fix and where is no other way prolong of timeout.

Comment 2

[Honza Bambas (:mayhemer)]

Attachment: v1

The previous patch was not built on tip... sorry for bloat.

Comment 4

[Honza Bambas (:mayhemer)]

Comment on attachment 372235 [details] [diff] [review]
v1

Bob, could you please take a look? Just test changes.. Thanks.

Comment 5

[Robert Relyea]

Comment on attachment 372235 [details] [diff] [review]
v1

Changing review of PSM java and html to Kai.

bob

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 372235 [details] [diff] [review]
v1

I like the img onload change, but bumping the other timer values feels like it's not addressing problems, just trying to hide them more. In general, we should avoid using setTimeout in tests, as it's a known cause of flaky behavior. Is there some other notification your test can wait for to ensure that whatever it's waiting for is ready?

Comment 7

[Honza Bambas (:mayhemer)]

Agree.

- test_bug329869.html might wait for onload of the new script element
- test_bug472986.html might wait for onload of the img element
- I have no idea what to do for test_cssContent2.html, it adds content with URL and I don't know about any callback for this, but that test has never randomly failed
- test_dynDelayedUnsecurePicture.html might wait for onload of the img
- No idea for test_dynUnsecureBackground.html... is there a callback for background image load? also this test has never failed
- test_dynUnsecurePicture.html might use img onload
- test_dynUnsecureRedirect.html as well
- test_innerHtmlDelayedUnsecurePicture.html might inject the onload code to innerHTML
- test_innerHtmlUnsecurePicture.html as well
- test_unsecureIframeMetaRedirect.html might probably use onload of the iframe

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 372235 [details] [diff] [review]
v1

Ok. If you want to make those changes, feel free to do so without further review. You can push the test_dynDelayedUnsecurePicture.html  change, since that looks good.

Comment 9

[Honza Bambas (:mayhemer)]

Attachment: v2 - mainly fixing bug 553326 [Check-in comment 11]

Fixing just the 4 failing tests from bug 553326 for now to see the effect.

After landing the new redirect API (my strongest suspect) we got over the edge of 500ms timeout to get the security UI updated.  All the failing tests are simply using timeout of 500ms to check for sec UI changes expecting the image to load in that time.

I changed the code to poll every 100ms for the state change, but not longer then 20 seconds.

Comment 10

[Kai Engert (:KaiE:)]

Comment on attachment 466871 [details] [diff] [review]
v2 - mainly fixing bug 553326 [Check-in comment 11]

r=kaie

Comment 11

[Honza Bambas (:mayhemer)]

Comment on attachment 466871 [details] [diff] [review]
v2 - mainly fixing bug 553326 [Check-in comment 11]

http://hg.mozilla.org/mozilla-central/rev/ea40235da826

So far, fixing just 4 mostly failing tests.  Remaining test will be patched later after we check this helped.