Closed
Bug 487402
Opened 16 years ago
Closed 12 years ago
test_dynUnsecureRedirect.html intermittently fails, change timeouts to callbacks where possible
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
People
(Reporter: mayhemer, Assigned: mayhemer)
References
Details
(Keywords: intermittent-failure)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
KaiE
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•16 years ago
|
||
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.
Attachment #372231 -
Flags: review?(kaie)
Assignee | ||
Comment 2•16 years ago
|
||
The previous patch was not built on tip... sorry for bloat.
Attachment #372231 -
Attachment is obsolete: true
Attachment #372235 -
Flags: review?(kaie)
Attachment #372231 -
Flags: review?(kaie)
Assignee | ||
Comment 4•15 years ago
|
||
Comment on attachment 372235 [details] [diff] [review]
v1
Bob, could you please take a look? Just test changes.. Thanks.
Attachment #372235 -
Flags: review?(kaie) → review?(rrelyea)
Comment 5•15 years ago
|
||
Comment on attachment 372235 [details] [diff] [review]
v1
Changing review of PSM java and html to Kai.
bob
Attachment #372235 -
Flags: review?(rrelyea) → review?(kaie)
Assignee | ||
Updated•15 years ago
|
Whiteboard: [orange]
Assignee | ||
Updated•15 years ago
|
Attachment #372235 -
Flags: review?(kaie) → review?(ted.mielczarek)
Comment 6•15 years ago
|
||
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?
Assignee | ||
Comment 7•15 years ago
|
||
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
Status: NEW → ASSIGNED
Comment 8•15 years ago
|
||
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.
Attachment #372235 -
Flags: review?(ted.mielczarek) → review-
Assignee | ||
Updated•15 years ago
|
Summary: test_dynUnsecureRedirect.html intermittently fails, security UI issue → test_dynUnsecureRedirect.html intermittently fails, change timeouts to callbacks where possible
Assignee | ||
Comment 9•14 years ago
|
||
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.
Attachment #372235 -
Attachment is obsolete: true
Attachment #466871 -
Flags: review?
Comment 10•14 years ago
|
||
Comment on attachment 466871 [details] [diff] [review]
v2 - mainly fixing bug 553326 [Check-in comment 11]
r=kaie
Attachment #466871 -
Flags: review? → review+
Assignee | ||
Comment 11•14 years ago
|
||
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.
Attachment #466871 -
Attachment description: v2 - mainly fixing bug 553326 → v2 - mainly fixing bug 553326 [Check-in comment 11]
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Keywords: intermittent-failure
Updated•12 years ago
|
Whiteboard: [orange]
You need to log in
before you can comment on or make changes to this bug.
Description
•