Intermittent browser_522545.js | sessionstore got correct userTypedValue - Got undefined, expected example.org

VERIFIED FIXED

Status

()

Firefox
Session Restore
VERIFIED FIXED
8 years ago
5 years ago

People

(Reporter: birtles, Unassigned)

Tracking

({intermittent-failure})

Trunk
intermittent-failure
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
This has popped up a couple of times:

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_522545.js | sessionstore got correct userTypedValue - Got undefined, expected mozilla.org
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_522545.js | sessionstore got correct userTypedClear - Got undefined, expected 0

once on Windows Opt:
Rev3 WINNT 6.1 mozilla-central opt test mochitest-other on 2010/09/17 18:51:50
s: talos-r3-w7-032
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284774710.1284776559.6920.gz

once on OSX Debug:
Rev3 MacOSX Leopard 10.5.8 mozilla-central debug test mochitest-other on 2010/09/17 19:52:41
s: talos-r3-leopard-008
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284778361.1284780471.23182.gz

Suspect this push caused it:
http://hg.mozilla.org/mozilla-central/rev/a73c063e52cb
http://hg.mozilla.org/mozilla-central/rev/957c89c4eac3
http://hg.mozilla.org/mozilla-central/rev/16071f59d247
This started after bug 586068 landed. I had a couple issues with that test locally, but fixed the problem. I ran these tests again before pushing and everything passed, so yay new orange!
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Summary: Intermittent failure: TEST-UNEXPECTED-FAIL | content/browser/browser/components/sessionstore/test/browser/browser_522545.js | sessionstore got correct userTypedValue - Got undefined, expected mozilla.org → Intermittent browser_522545.js | sessionstore got correct userTypedValue - Got undefined, expected mozilla.org
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
That explosion was because you're hitting the network - do you actually need to?
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Though, wtf part of it is hitting the network? Sometimes the input event is actually loading mozilla.org? Typing mozilla.org is triggering awesomebar suggestions that are hitting the network to load favicons? I sure don't see anything other than mozilla.org that wouldn't be local or proxied.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Created attachment 498682 [details] [diff] [review]
[checked in] potential fix

This is clearly stupid, and couldn't possibly fix the problem, but I want to try it anyway.

You can see that setting gURLBar.value and firing an input event doesn't trigger any sort of network activity, doesn't do anything unfortunate like opening the awesomebar's suggestions and trying to load favicons, by just imitating what the test does in the error console.

But, you can also see that there are no other non-proxied hostnames in the test, and that this is the only part of the test that intermittently fails, and that back when the build network was broken, it failed on every run. So, my justification for wanting to change it is to eliminate the obvious, so when someone smarter looks at it they won't have to stop to think about whether that "mozilla.org" could be hitting the network.
Attachment #498682 - Flags: review?(paul)
Comment on attachment 498682 [details] [diff] [review]
[checked in] potential fix

(In reply to comment #274)
> But, you can also see that there are no other non-proxied hostnames in the
> test, and that this is the only part of the test that intermittently fails, and
> that back when the build network was broken, it failed on every run. So, my
> justification for wanting to change it is to eliminate the obvious, so when
> someone smarter looks at it they won't have to stop to think about whether that
> "mozilla.org" could be hitting the network.

I agree that this really should have absolutely no effect on this failure, but I'm willing to try it to eliminate the obvious. Let's give this a shot.
Attachment #498682 - Flags: review?(paul) → review+
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment on attachment 498682 [details] [diff] [review]
[checked in] potential fix

http://hg.mozilla.org/mozilla-central/rev/3149334e551e

Any bets on how many cycles it will cruelly let me think it worked, before failing again?
Attachment #498682 - Attachment description: potential fix → [checked in] potential fix
Comment hidden (Treeherder Robot)
Just two, not too cruel.
Summary: Intermittent browser_522545.js | sessionstore got correct userTypedValue - Got undefined, expected mozilla.org → Intermittent browser_522545.js | sessionstore got correct userTypedValue - Got undefined, expected example.org
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Created attachment 500419 [details] [diff] [review]
For reproducing this issue

This issue can be reproduced.

The problem is that there is no teardown function so preceding test affects following test. In this case, test_getBrowserState_lotsOfTabsOpening made test_getBrowserState_userTypedValue succeed.

I have not investigated the reason why test_getBrowserState_lotsOfTabsOpening makes test_getBrowserState_userTypedValue succeed.
Created attachment 500420 [details] [diff] [review]
Proposed fix with proper teardown.
Attachment #500420 - Flags: review?(paul)
Comment on attachment 500420 [details] [diff] [review]
Proposed fix with proper teardown.

>+      executeSoon(function() {

Do you need the executeSoon wrapping? It seems like it shouldn't be necessary with the addition of waitForBrowserState. (I'm certainly guilty of abusing executeSoon, but I'd like to avoid it if we don't really need it). If it doesn't pass without the executeSoon, then keep it.

>   function runNextTest() {
>     if (tests.length) {
>-      tests.shift()();
>+      gBrowser.stop();

I think if we're using waitForBrowserState, calling stop would be extraneous (not harmful though).

>+      waitForBrowserState(state, function() {
>+        tests.shift()();
>+      });

You can just write that as
> waitForBrowserState(state, tests.shift());

Also, I landed a more complete waitForBrowserState into head.js in bug 615394, so you should be able to delete the definition in this test (while you're there).
Attachment #500420 - Flags: review?(paul) → feedback+
(In reply to comment #338)
> Comment on attachment 500420 [details] [diff] [review]
> Proposed fix with proper teardown.
> 
> >+      executeSoon(function() {
> 
> Do you need the executeSoon wrapping? It seems like it shouldn't be necessary
> with the addition of waitForBrowserState. (I'm certainly guilty of abusing
> executeSoon, but I'd like to avoid it if we don't really need it). If it
> doesn't pass without the executeSoon, then keep it.

waitForBrowserState just waits for being restored session but in this case we need to wait for being updated current session state after dispatching event so I think executeSoon is needed there, isn't it?

> >   function runNextTest() {
> >     if (tests.length) {
> >-      tests.shift()();
> >+      gBrowser.stop();
> 
> I think if we're using waitForBrowserState, calling stop would be extraneous
> (not harmful though).

I think test_getBrowserState_lotsOfTabsOpening might be loading some pages.

Invoking gBrowser.stop() just before runNextTest() in firstLocationChange() is good to you?

> >+      waitForBrowserState(state, function() {
> >+        tests.shift()();
> >+      });
> 
> You can just write that as
> > waitForBrowserState(state, tests.shift());
> 
> Also, I landed a more complete waitForBrowserState into head.js in bug 615394,
> so you should be able to delete the definition in this test (while you're
> there).

I did not notice head.js. I will use it. Thank you for your kindly comments.
(In reply to comment #339)
> waitForBrowserState just waits for being restored session but in this case we
> need to wait for being updated current session state after dispatching event so
> I think executeSoon is needed there, isn't it?

Ah I was looking at the whole test file, not this particular test within. Keep the executeSoon. I'm not 100% sure it's needed but it doesn't hurt.

> I think test_getBrowserState_lotsOfTabsOpening might be loading some pages.
> 
> Invoking gBrowser.stop() just before runNextTest() in firstLocationChange() is
> good to you?

Well, so calling ss.setBrowserState (via waitForBrowserState) should be calling stop() on each browser, which is why I don't think it's necessary. Try it without but if there are issues, put it in like you have it.

> Thank you for your kindly comments.

No problem. Thanks for looking into this!
> > I think test_getBrowserState_lotsOfTabsOpening might be loading some pages.
> > 
> > Invoking gBrowser.stop() just before runNextTest() in firstLocationChange() is
> > good to you?
> 
> Well, so calling ss.setBrowserState (via waitForBrowserState) should be calling
> stop() on each browser, which is why I don't think it's necessary. Try it
> without but if there are issues, put it in like you have it.

You are right. Removal of stop() works fine. I will attach a new patch soon.
Created attachment 500445 [details] [diff] [review]
[checked in] Revised patch

Changes from the previous patch:

* Removed waitForBrowserState
* Removed gBrowser.stop in runNextTest
* waitForBrowserState in runNextTest became one line
Attachment #500419 - Attachment is obsolete: true
Attachment #500420 - Attachment is obsolete: true
Attachment #500445 - Flags: review?(paul)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)