Closed Bug 751817 Opened 8 years ago Closed 7 years ago

Intermittent browser_popupNotification.js | message matches - Got This is popup notification test-notification-21 from test 21, expected This is popup notification test-notification-20 from test 20 and 70-odd others

Categories

(Firefox :: General, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: philor, Unassigned)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Starting today, the smart money would blame compartment-per-global for starting it failing, though we've dumped several hundred other things into the tree too.

https://tbpl.mozilla.org/php/getParsedLog.php?id=11449769&tree=Mozilla-Inbound
Rev3 WINNT 5.1 mozilla-inbound opt test mochitest-other on 2012-05-03 18:58:26 PDT for push 3b9b9e948dca

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_popupNotification.js | message matches - Got This is popup notification test-notification-21 from test 21, expected This is popup notification test-notification-20 from test 20
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_popupNotification.js | id matches - Got test-notification-21_2-notification, expected test-notification-20-notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_popupNotification.js | message matches - Got This is popup notification test-notification-22 from test 22, expected This is popup notification test-notification-20 from test 20
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_popupNotification.js | id matches - Got test-notification-22-notification, expected test-notification-20-notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_popupNotification.js | message matches - Got This is popup notification test-notification-22 from test 22, expected This is popup notification test-notification-20 from test 20
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_popupNotification.js | id matches - Got test-notification-22-notification, expected test-notification-20-notification
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_geoprompt.js | an unexpected uncaught JS exception reported through window.onerror - ReferenceError: info is not defined at chrome://mochitests/content/browser/browser/base/content/test/browser_popupNotification.js:96
(tons more fallout failures like that)

https://tbpl.mozilla.org/php/getParsedLog.php?id=11454490&tree=Mozilla-Inbound
Rev3 WINNT 5.1 mozilla-inbound opt test mochitest-other on 2012-05-03 22:01:11 PDT for push 4d81667d36dd
Bug 641892 landed on the 3rd, it seems like a more likely culprit.

Ohzeki-san, do you have any idea why this test would be failing intermittently? It appears to be failing only on Windows.
This browser_popupNotification.js is ran by mochitest, it sometimes fails tests by something to happen at random. I sometimes saw this error by something to happen when developing the patch of Bug 641892. So Comment 8 is not caused by Bug 641892. It is not land on mozilla-aurora.

I cannot be predicable, other failing tests may be caused by other reasons.
browser_popupNotification.js uses some functions using the key event of VK_ESCAPE.
But this key event does not work on Windows when the time from 2012-05-03 to 2012-05-05 (This is Bug 166240). The report of failing tests on this bug are just on its time.
If no more report on latest moz-central, I think that we can close this bug.

(And I add CCing Nakano-san who is an owner of Bug 166240)
The regression shouldn't affect any plain mochi-tests. The regression is in the native key message handler of Windows. The handler isn't run by nsIDOMWindowUtils::sendKeyMessage() which is called via EventUtils.synthesizeKey().
oops, I meant nsIDOMWindowUtils::sendKeyEvent()
Thank for your comment & I'm sorry for accusing you of falsely, Nakano-san.

I consider from logs pasted on this bug, this error may be caused by that Test#20's onShown has been lived like zombie. When happening the error, popupshown event is not fired before popuphidden event is fired in Test#20. So Test#20's onShown is not removed from event listener.
Perhaps this bug will be fixed if we call cleanUp() before running each test.

And I seem that this error happens only on Windows NT5.1 environment from logs. Is this bug WinNT 5.1 bug...?
Attached patch patchSplinter Review
This patch clean up event lisnters & observer before running each test.
However, I cannot establish that this patch will be fix the error because the error is randomly. (and I don't have the WinNT 5.1 build enviroment)
Attachment #622384 - Flags: review?(margaret.leibovic)
Comment on attachment 622384 [details] [diff] [review]
patch

I don't understand exactly how this will help fix the error, and it feels a bit too much like a stab in the dark. I think we need to understand these failures better to come up with the right fix. It looks like the problem is just with popups from test #20/#22, so I think we should inspect those more carefully.
Attachment #622384 - Flags: review?(margaret.leibovic) → review-
(In reply to Margaret Leibovic [:margaret] from comment #19)
> I don't understand exactly how this will help fix the error, and it feels a
> bit too much like a stab in the dark. I think we need to understand these
> failures better to come up with the right fix. It looks like the problem is
> just with popups from test #20/#22, so I think we should inspect those more
> carefully.

From logs, test20's onShown() (line 637) is called after Test 20 ended. I think that, the ending Test20 before its popupshown fired have not released test20's onShown(). By design of this test file, some callback handler sometimes cannot be released obviously if no firing event of its callback event. (ref: doOnPopupEvent())

I cannot confirm that this patch fixes this bug perfectly. But this patch will avoid some errors which are caused with holding callback handlers.

Please review again, Margaret.
Attachment #622384 - Flags: review- → review?(margaret.leibovic)
I'm sorry I've been so slow with this review, but it's been more than a month since we've seen this failure, so I think we can just close it WORKSFORME.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
Attachment #622384 - Flags: review?(margaret.leibovic)
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.