>Running chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_459906.js... >TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_459906.js | rich textarea's content correctly duplicated - Got <br>, expected <b>Unique:</b> 1255343414797 >TEST-PASS | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_459906.js | XSS exploit prevented! http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1255341728.1255344367.28032.gz

i can reproduce the failure, if you have ideas let me know, i can test them temporary assigning to avoid losing track

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1255481576.1255483456.385.gz WINNT 5.2 mozilla-central test everythingelse on 2009/10/13 17:52:56 TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_459906.js | rich textarea's content correctly duplicated - Got <br>, expected <b>Unique:</b> 1255482645673

pushed experimental change http://hg.mozilla.org/mozilla-central/rev/af0ade2a9a1d will backout if nothing changes.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1255737333.1255739317.23060.gz WINNT 5.2 mozilla-central test opt everythingelse [testfailed] Started 16:55, finished 17:29

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1255736768.1255738991.19557.gz WINNT 5.2 mozilla-central test everythingelse [testfailed] Started 16:46, finished 17:24

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1255958493.1255960207.21224.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/19 06:21:33

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1255977364.1255979929.19235.gz WINNT 5.2 mozilla-central test everythingelse on 2009/10/19 11:36:04

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256032063.1256033970.21652.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/20 02:47:43 TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_459906.js | rich textarea's content correctly duplicated - Got <br>, expected <b>Unique:</b> 1256033048045 http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256030441.1256032142.1841.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/20 02:20:41 TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_459906.js | rich textarea's content correctly duplicated - Got <br>, expected <b>Unique:</b> 1256031295594

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256136558.1256138473.28335.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/21 07:49:18

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256132025.1256133765.5949.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/21 06:33:45 http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256147650.1256149482.25498.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/21 10:54:10

WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/23 13:05:00 http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256328300.1256330362.10084.gz

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256476073.1256477808.6271.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/25 06:07:53

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256489317.1256491091.29575.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/25 09:48:37

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256508347.1256510107.11498.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/25 15:05:47

changed the polling along with http://hg.mozilla.org/mozilla-central/rev/fd21464eec80 date.now() is not reliable on VMs, plus increased timeout to 3s, let's see.

Note that in comment 16, this test is now timing out (instead of just failing, as it was in comment 14's log). I'm guessing this change was due to the change in comment 15. Another timeout, a bit earlier: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256778964.1256781467.8426.gz

Note also that the log from comment 16 is for an *OS X* cycle (whereas until then, this was a Windows-only bug, AFAICT). Did comment 15 convert this Windows-only test-failure into a cross-platform test-timeout?

no, is just that was late and i confused setTimeout with do_timeout, inverting the arguments. i'll push a fix.

fixed the setTimeout with http://hg.mozilla.org/mozilla-central/rev/40b3d440c5a9

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256923246.1256925115.26702.gz#err0 WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/30 10:20:46

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256955881.1256957990.10583.gz WINNT 5.2 mozilla-central test everythingelse on 2009/10/30 19:24:41 browser_459906.js | rich textarea's content correctly duplicated - Got , expected <b>Unique:</b> 1256957097036

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1256957503.1256959318.24814.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/10/30 19:51:43 TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_459906.js | rich textarea's content correctly duplicated - Got <br>, expected <b>Unique:</b> 1256958445767

i increased the wait time on this test, and the fact it's still failing means we could wait forever, it would never finish loading and setting the value for innerHTML.

i've been able to reproduce this once in an optimized build, but to do so i had to run it in a VM, with 2 virtual CPUs and low ram. this is probably a situation more similar to what out tinderbox VMs have, i'll try to reduce memory size even more to see if i can reproduce more reliably.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257124052.1257126167.11968.gz#err2 WINNT 5.2 mozilla-central test opt everythingelse on 2009/11/01 17:07:32

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257172432.1257174742.4582.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/11/02 06:33:52

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257165136.1257167521.13389.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/11/02 04:32:16

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257254643.1257256368.19312.gz#err1 WINNT 5.2 mozilla-central test opt everythingelse on 2009/11/03 05:24:03

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257297326.1257299427.27860.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/11/03 17:15:26

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257290610.1257293304.24447.gz WINNT 5.2 mozilla-central test opt everythingelse on 2009/11/03 15:23:30

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257548618.1257551067.15003.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/06 15:03:38

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257572422.1257574246.25797.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/06 21:40:22

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257564592.1257567050.29715.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/06 19:29:52 http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257568272.1257570408.16590.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/06 20:31:12

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257726683.1257728630.2110.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/08 16:31:23

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257816977.1257819535.21776.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/09 17:36:17

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257838127.1257840466.31196.gz WINNT 5.2 mozilla-central test everythingelse on 2009/11/09 23:28:47

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257932535.1257934416.30268.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/11 01:42:15 "s: moz2-win32-slave32"

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257942880.1257945155.24102.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/11 04:34:40 "s: moz2-win32-slave36"

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257974529.1257977232.11312.gz WINNT 5.2 mozilla-central test everythingelse on 2009/11/11 13:22:09 "s: moz2-win32-slave38"

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258038550.1258040319.9508.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/12 07:09:10

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258052214.1258054628.13650.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/12 10:56:54 "s: moz2-win32-slave13"

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258119572.1258121842.30435.gz Linux mozilla-central debug test everythingelse on 2009/11/13 02:53:49 "s: moz2-win32-slave02"

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258189492.1258191613.4675.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/14 01:04:52

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258193847.1258196201.25828.gz WINNT 5.2 mozilla-central test everythingelse on 2009/11/14 02:17:27

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258243527.1258245870.7275.gz WINNT 5.2 mozilla-central test everythingelse on 2009/11/14 16:05:27 "s: moz2-win32-slave16"

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258254597.1258257623.5121.gz#err0 WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/14 19:09:57 "s: moz2-win32-slave40"

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258313534.1258315668.28481.gz WINNT 5.2 mozilla-central opt test everythingelse

i think this is actually due to the order in which frames are loaded, the DOMContentLoad sets designMode = on too early on the frame where we try to set innerHTML, instead it should wait for both frames so that the test has time to setup things. I've already demonstrated that the timeout that was added to wait for innerHTML was useless, since increasing the time by 3 times did not brought any useful gain. while i was usually able to reproduce the random failure at least once in 10 runs, i can't anymore with this patch, could be i'm just lucky but could also be it helps.

Comment on attachment 412626 [details] [diff] [review] patch v1.0 browser_459906_sample.html is an actual exploit that worked against Firefox 2.0. Since it depends on a timing issue itself, I'd prefer it not to be modified (else you might accidentally turn the exploit harmless which isn't the way we want the test to pass). Having too much of a headache to figure out something better, though. BTW: Please try to keep unrelated changes down to a minimum or better make them in separate patches if we can't do without them.

unrelated changes is just trailing whitespaces cleanup, that should not hurt nobody unless the test depends on them (i doubt it's the case). unfortunatly i can't see a way to fix this without touching the test file, i asked review to you exactly because you're the best person to evaluate if i could have touched any timers needed by the test. I can actually try to remove the code added to fix the exploit, and see if the test fails.

(In reply to comment #57) > unrelated changes is just trailing whitespaces cleanup, that should not hurt It hurts my brain trying to figure out which changes are actually relevant for what this specific patch is trying to accomplish (ideally they should all be). > I can actually try to remove the code added to fix the exploit, and see if the > test fails. Alright, please do so.

i reverted all tests files to the original state (prior of any change, revision 5e8579e1057c, the one you pushed), and reverted the exploit fix, and the test does not fail still. So either the original test was uneffective or something other fixed the exploit in another place. i could try on 1.9.1.

i'm referring to: http://hg.mozilla.org/mozilla-central/rev/5e8579e1057c

i'm bringing back my tree to that revision, and rebuilding to check tests are failing. if you have any ideas about why they are not failing with current code using the original test and sessionstore code, please share them. Anything that would allow me to reproduce the failure with unchanged test.

You might have to go back to one revision prior to http://hg.mozilla.org/mozilla-central/rev/8b81d681d1ad (when the original fix landed). See bug 464620 for more details and an older revision of the tests - in case that helps.

this is what i get with that revision :( chrome://mochikit/content/browser/../browser/browser/components/sessionstore/test/browser/browser_459906.js TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/../browser/browser/components/sessionstore/test/browser/browser_459906.js | rich textarea's content correctly duplicated - Got , expected <b>Unique:</b> 1258469341649 TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/../browser/browser/components/sessionstore/test/browser/browser_459906.js | XSS exploit prevented! - Got , expected localhost chrome://mochikit/content/browser/../browser/browser/components/sessionstore/test/browser/browser_461743.js TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/../browser/browser/components/sessionstore/test/browser/browser_461743.js | Timed out

and these results don't change with or without the code changes in bug 464620

So, i'm completely unable to make any version of the test fail for the test purpose, old versions are not restoring pages correctly, while new versions seem to have another layer of protection that is preventing them from failing. But i looked deep enough to think that these changes won't hurt the test. What we try to test happens in sessionstore at load event, while the modified part happens at DOMContentLoaded, making it happen at the second call rather then at the first will just ensure both frames are ready to move on, without actually changing the order of the events we are interested in, load won't fire till DOMContentLoaded has finished. If frames[1] has not yet loaded dom content probably frames[1].document.designMode = "on"; will fail, and ss cannot restore innerHTML, thus the failure in seeing the duped data. no timeout can save us from that.

Comment on attachment 412626 [details] [diff] [review] patch v1.0 Alright then, let's do this. Thanks for investigating!

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258595715.1258598646.7206.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/18 17:55:15 "s: moz2-win32-slave12"

crossing fingers http://hg.mozilla.org/mozilla-central/rev/49ed45ddf2e1

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258643868.1258646173.29249.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/11/19 07:17:48 "s: moz2-win32-slave02" TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_459906.js | rich textarea's content correctly duplicated - Got <br>, expected <b>Unique:</b> 1258644733883 (that's prior to your push)

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1260741382.1260743677.22229.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/12/13 13:56:22 s: win32-slave02 TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_459906.js | rich textarea's content correctly duplicated - Got <br>, expected <b>Unique:</b> 1260742359641 Dropped the frequency quite a bit, though :)

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261673502.1261675379.18673.gz WINNT 5.2 mozilla-central opt test everythingelse on 2009/12/24 08:51:42 s: win32-slave32 TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_459906.js | rich textarea's content correctly duplicated - Got <br>, expected <b>Unique:</b> 1261674281110

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263578721.1263579451.6326.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/01/15 10:05:21 s: win32-slave40

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1264215618.1264216244.4151.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/01/22 19:00:18 s: win32-slave25

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1264468887.1264469963.28109.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/01/25 17:21:27 s: win32-slave41

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1264565856.1264567279.2806.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/01/26 20:17:36 s: win32-slave38

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1264619293.1264620076.29382.gz s: win32-slave39 TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_459906.js | rich textarea's content correctly duplicated - Got <br>, expected <b>Unique:</b> 1264619822339

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1264625668.1264626274.14965.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/01/27 12:54:28 s: win32-slave29

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1265093091.1265094144.12986.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/02/01 22:44:51 s: win32-slave42

I've got this in a VMWare recorded session. What information do you need?

(In reply to comment #83) > I've got this in a VMWare recorded session. What information do you need? honestly i've not looked at this recently, the last fix dropped the frequency of the orange, and was related to the time designMode=on was set in the first frame of the test. Now the frequency has increased again, not sure why, i think i've seen some fix to designMode in the last days? The point seems to be that designMode locks the frame before innerHTML is set, but i can't guess much more. This is also a security test, so changes have to be safe regarding not changing what the test is trying to ensure.

What do you mean "locks the frame"? Can you tell me what information would help you understand what's going on? Do you want me to check whether frames[0].document.designMode = "on" runs before iframes[1].document.body.innerHTML = uniqueValue, would that be helpful?

(In reply to comment #84) > This is also a security test, so changes have to be safe regarding not changing > what the test is trying to ensure. If we can't fix this test to be reliable, we may as well disable it (since people will learn to ignore failures anyway), which is worse than a risk of accidental breakage.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1266522557.1266523324.21530.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/02/18 11:49:17

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1266539386.1266540909.24570.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/02/18 16:29:46 s: win32-slave36

(In reply to comment #85) > What do you mean "locks the frame"? i think that designMode does not allow us to set innerHTML. > Can you tell me what information would help you understand what's going on? Do > you want me to check whether frames[0].document.designMode = "on" runs before > iframes[1].document.body.innerHTML = uniqueValue, would that be helpful? Yes, i think that could be useful to know in which order these runs: frames[0].document.designMode = "on" frames[1].document.designMode = "on"; iframes[1].document.body.innerHTML = uniqueValue (In reply to comment #86) > If we can't fix this test to be reliable, we may as well disable it (since > people will learn to ignore failures anyway) This is a point, but i'm still unsure that disabling a security test is a good thing. I mean that if the fail should start failing permanently, that would mean something broke it, while a random failure is not a sign of that.

(In reply to comment #89) > (In reply to comment #85) > > Can you tell me what information would help you understand what's going on? Do > > you want me to check whether frames[0].document.designMode = "on" runs before > > iframes[1].document.body.innerHTML = uniqueValue, would that be helpful? > > Yes, i think that could be useful to know in which order these runs: > frames[0].document.designMode = "on" > frames[1].document.designMode = "on"; > iframes[1].document.body.innerHTML = uniqueValue OK, I'll get that information, but since I'll be in Mountain View next week it'll have to be the following week. > This is a point, but i'm still unsure that disabling a security test is a good > thing. I mean that if the fail should start failing permanently, that would > mean something broke it, while a random failure is not a sign of that. The random failure could be a sign that you can violate security if you're lucky.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1266621502.1266622335.15359.gz WINNT 5.2 mozilla-central opt test mochitest-other [testfailed] Started 15:18, finished 15:33

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1266699219.1266699919.27338.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/02/20 12:53:39 s: win32-slave31

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1266994248.1266995050.18145.gz WINNT 5.2 mozilla-central opt test mochitest-other

WINNT 5.2 mozilla-central opt test mochitest-other [testfailed] Started 07:02, finished 07:14 http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1267023720.1267024434.9667.gz&fulltext=1

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1267087500.1267089488.1683.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/02/25 00:45:00

> frames[0].document.designMode = "on" > frames[1].document.designMode = "on"; > iframes[1].document.body.innerHTML = uniqueValue This is the order they happen in. Is that unexpected? If so, what order do you expect them to happen in, and what do you think should be forcing them to happen in that order?

To be more precise, here's what I seem to be seeing: In the first tab: frames[0].document.designMode = "on" frames[1].document.designMode = "on" Then in the main test: iframes[1].document.body.innerHTML = uniqueValue Then in the second tab: frames[0].document.designMode = "on" frames[1].document.designMode = "on" Then finally is(iframes[1].document.body.innerHTML, uniqueValue I do *not* see any attempt by nsSessionStore.js to set innerHTML in the second tab. The only call to SetInnerHTML is in the test. nsSessionStore.js calls the designMode getter multiple times but always from updateTextAndScrollDataForFrame, never from restoreTextDataAndScrolling. Actually, I don't see anything in the test which guarantees that the test code in executeSoon happens after the session restore code that sets the innerHTML inside a setTimeout: http://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/test/browser/browser_459906.js#65 http://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/src/nsSessionStore.js#2388 Do you have a reason to believe that that executeSoon code will always happen after that setTimeout code has run?

no i don't have a reason, but previous code here was waiting 5 seconds or so without any luck. it's also true that it was not waiting for all frames to be loaded before setting innerHTML, that was fixed above. So, are you suggesting to executeSoon twice to be sure to enqueue after the SetTimeout(0)? this would cover the general case, not the case where ss retries, but could be worth trying (there is no reason to think ss will retry here, but i don't know implications of that code enough). otherwise, we could just restore the polling strategy i had put in as first try, keep retrying every 500ms till we reach 2 or 3s, then throw if value is still wrong.

> So, are you suggesting to executeSoon twice to be sure to enqueue after the > SetTimeout(0)? No, that might not work. > otherwise, we could just restore the polling strategy i had put in as first > try, keep retrying every 500ms till we reach 2 or 3s, then throw if value is > still wrong. That sounds reasonable to me. I'd make the retry timeout a lot smaller though, probably just use 10ms or something. And I wouldn't stop just because we reach 2 or 3s. Just keep trying, if the session restore never happens, then eventually the mochitest harness will time out. By the way, I'm quite surprised that sessionstore behaves like this, mutating the DOM some time after the load event has fired. That could interfere with Web applications.

> By the way, I'm quite surprised that sessionstore behaves like this, mutating > the DOM some time after the load event has fired. That could interfere with Web > applications. Paul or Simon can probably comment better than me on SS needs. I can just restore the polling code and see if it helps now. thanks for logging the failure.

Do we need to audit all the sessionstore tests to see if they are handling sessionstore completing correctly? Bug 538672 looks like another case where we should be polling.

(In reply to comment #100) > > By the way, I'm quite surprised that sessionstore behaves like this, mutating > > the DOM some time after the load event has fired. That could interfere with Web > > applications. > > Paul or Simon can probably comment better than me on SS needs. filed bug 549359 for this.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1267472789.1267474063.21437.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/03/01 11:46:29 s: win32-slave37

Comment on attachment 429535 [details] [diff] [review] polling patch (checked-in) Polling is OK, but it depends on the test actually passing (before actually calling is()). We'll see a timeout if it fails, so that's ok. r=zpao

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1267629067.1267630183.8968.gz WINNT 5.2 mozilla-central opt test mochitest-other on 2010/03/03 07:11:07 s: win32-slave34

Comment on attachment 429535 [details] [diff] [review] polling patch (checked-in) http://hg.mozilla.org/mozilla-central/rev/7beada3649b6

looks good so far, resolving

Checked today and cannot see any failure with browser_459906.js. VERIFIED FIXED.