Closed Bug 981363 Opened 10 years ago Closed 10 years ago

Intermittent jsat/test_content_integration.html | undefined - got wow heading level 1 such app, expected Phone status bar Traversal Rule test document etc.

Categories

(Core :: Disability Access APIs, defect)

x86
Windows 8
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox29 --- unaffected
firefox30 --- wontfix
firefox31 --- wontfix
firefox32 --- wontfix
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
firefox-esr24 --- unaffected
firefox-esr31 --- fixed

People

(Reporter: philor, Assigned: eeejay)

References

Details

(Keywords: intermittent-failure)

Attachments

(3 files)

https://tbpl.mozilla.org/php/getParsedLog.php?id=35842539&tree=Mozilla-Inbound
WINNT 6.2 mozilla-inbound opt test mochitest-other on 2014-03-08 07:32:19 PST for push 71e286533bd9
slave: t-w864-ix-092

07:53:04     INFO -  568 INFO TEST-KNOWN-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/jsat/test_content_integration.html | undefined - got many option not checked check button, expected Home button
07:53:04     INFO -  569 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/jsat/test_content_integration.html | undefined - got wow heading level 1 such app, expected Phone status bar Traversal Rule test document
07:53:04     INFO -  570 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/jsat/test_content_integration.html | undefined - got Phone status bar Traversal Rule test document, expected wow heading level 1 such app
07:53:04     INFO -  571 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/jsat/test_content_integration.html | undefined - got Phone status bar, expected many option not checked check button
07:53:04     INFO -  572 INFO TEST-INFO | MEMORY STAT vsize after test: 538963968
07:53:04     INFO -  573 INFO TEST-INFO | MEMORY STAT vsizeMaxContiguous after test: 1893662720
07:53:04     INFO -  574 INFO TEST-INFO | MEMORY STAT residentFast after test: 182325248
07:53:04     INFO -  575 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 68915998
07:53:04     INFO -  ************************************************************
07:53:04     INFO -  * Call to xpconnect wrapped JSObject produced this error:  *
07:53:04     INFO -  [Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIPropertyBag2.getPropertyAsAString]"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: chrome://specialpowers/content/SpecialPowersObserverAPI.js :: addDumpIDToMessage :: line 74"  data: no]
07:53:04     INFO -  ************************************************************
07:53:04     INFO -  System JS : ERROR chrome://global/content/BrowserElementPanning.js:721 - NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getIntPref]
07:53:04     INFO -  System JS : ERROR chrome://global/content/BrowserElementPanning.js:721 - NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getIntPref]
07:53:04     INFO -  576 INFO TEST-END | chrome://mochitests/content/a11y/accessible/tests/mochitest/jsat/test_content_integration.html | finished in 1154ms
Eitan, Yura, can we fix these intermittents? They seem pretty recent, so it should hopefully be an easy fix. I don't like the frequency these are occurring.
Comment on attachment 8394946 [details] [diff] [review]
Wait or cursor to be cleared in tests. Some cleanup and improvements.

nit: recieved -> received
Comment on attachment 8394946 [details] [diff] [review]
Wait or cursor to be cleared in tests. Some cleanup and improvements.

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

Great!

::: accessible/tests/mochitest/jsat/jsatcommon.js
@@ +252,5 @@
>      var expected = this.currentPair[1] || {};
> +
> +    // |expected| can simply be a name of a message, no more further testing.
> +    if (aMessage.name === expected) {
> +      ok(true, 'Recieved ' + expected);

nit: spelling, Marco mentioned
Attachment #8394946 - Flags: review?(yzenevich) → review+
https://hg.mozilla.org/mozilla-central/rev/9bc0bd2a2f3f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Could this be safely uplifted to Aurora along with bug 980522 (which it seems to depend heavily on)?
Eitan, can you answer comment #38?
Flags: needinfo?(eitan)
(In reply to Marco Zehe (:MarcoZ) from comment #40)
> Eitan, can you answer comment #38?

Are we convinced that the patch above fixed this issue? I'm not sure..
Flags: needinfo?(eitan)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 990150
OK, I got to the bottom of what is happening here.

There is a race condition where the internal frame's DOCUMENT_LOAD_COMPLETE a11y event triggers an automove with a delay of 500ms, it will then move the iframe's vc to the first item in the frame and mess with the state that the test expects.

For this to happen two conditions need to be met:
1. The test needs to start before the DOCUMENT_LOAD_COMPLETE is emitted. On most runs, it won't.
2. The test needs to take longer than the 500ms so that the automove is inserted somewhere it is not expected.
Depends on: 1028329
Yura, I don't suppose you're up for the challenge in looking at this one next? :)
Flags: needinfo?(yzenevich)
Target Milestone: mozilla31 → ---
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #143)
> Yura, I don't suppose you're up for the challenge in looking at this one
> next? :)

Forwarding this to Eitan as per his request.
Flags: needinfo?(yzenevich) → needinfo?(eitan)
I'm feeling good about this one..
Attachment #8499927 - Flags: review?(yzenevich)
(In reply to Eitan Isaacson [:eeejay] from comment #150)
> Created attachment 8499927 [details] [diff] [review]
> Don't automove when the DOCUMENT_LOAD_COMPLETE is emitted for the actual
> document.
> 
> I'm feeling good about this one..

I'm curious what are the cases that we need this event for. If we don't want to auto move when the current document is loaded and we have no vc.position in yet?
Comment on attachment 8499927 [details] [diff] [review]
Don't automove when the DOCUMENT_LOAD_COMPLETE is emitted for the actual document.

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

Looks good to me and the tests pass just fine.
I'm still curious about the question earlier though.
Attachment #8499927 - Flags: review?(yzenevich) → review+
Attachment #8499927 - Flags: checkin?
Flags: needinfo?(eitan)
(In reply to Yura Zenevich [:yzen] from comment #156)
> (In reply to Eitan Isaacson [:eeejay] from comment #150)
> > Created attachment 8499927 [details] [diff] [review]
> > Don't automove when the DOCUMENT_LOAD_COMPLETE is emitted for the actual
> > document.
> > 
> > I'm feeling good about this one..
> 
> I'm curious what are the cases that we need this event for. If we don't want
> to auto move when the current document is loaded and we have no vc.position
> in yet?

That event is emitted when a dialog is shown. The name is misleading..
Attachment #8499927 - Flags: checkin? → checkin+
https://hg.mozilla.org/mozilla-central/rev/3b1edb52423b
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Please request aurora and esr31 approval for this when you get a chance please :)
Comment on attachment 8499927 [details] [diff] [review]
Don't automove when the DOCUMENT_LOAD_COMPLETE is emitted for the actual document.

Approval Request Comment
[Feature/regressing bug #]: 981363
[User impact if declined]:
[Describe test coverage new/current, TBPL]: This has been on m-c for a few days, and has successfully stopped these intermittent tests.
[Risks and why]: 
[String/UUID change made/needed]:
Attachment #8499927 - Flags: approval-mozilla-aurora?
Flags: needinfo?(eitan)
Attachment #8499927 - Flags: approval-mozilla-esr31?
Comment on attachment 8499927 [details] [diff] [review]
Don't automove when the DOCUMENT_LOAD_COMPLETE is emitted for the actual document.

Looks safe enough for 34 while still on Aurora. Aurora+

I think we can take this on ESR after 31.2.0 releases.
Attachment #8499927 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8499927 - Flags: approval-mozilla-esr31? → approval-mozilla-esr31+
Bleh, this needs some rebasing for esr31.
Flags: needinfo?(eitan)
Rebased for esr31, carrying over r+.

I would throw this at a try branch, but I don't think you could for an ESR release, right?
Flags: needinfo?(eitan)
Attachment #8512349 - Flags: approval-mozilla-esr31?
Comment on attachment 8512349 [details] [diff] [review]
Don't automove when the DOCUMENT_LOAD_COMPLETE is emitted for the actual document. r=yzen

Trivial rebases don't need to go through approval again.

You can push any branch to Try, provided you're aware that some test suites may not run if they aren't intended to run on that branch. In this case, I think a basic |try: -b do -p linux,linux64 -u mochitest-o -t none| sanity check run should be fine.
Attachment #8512349 - Flags: approval-mozilla-esr31?
You need to log in before you can comment on or make changes to this bug.