Last Comment Bug 615923 - mochitests-2: intermittent "test_fallback.html | Exited with code 1 during test run" (ASSERTION: XPConnect is being called on a scope without a 'Components' property!)
: mochitests-2: intermittent "test_fallback.html | Exited with code 1 during te...
Status: VERIFIED FIXED
[test which aborts the suite]
: assertion, intermittent-failure
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- major (vote)
: mozilla2.0b8
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
http://mxr.mozilla.org/mozilla-centra...
Depends on: 443017
Blocks: 615546 402272 438871
  Show dependency treegraph
 
Reported: 2010-12-01 10:39 PST by Josh Matthews [:jdm]
Modified: 2013-04-04 13:53 PDT (History)
2 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.14-fixed
.17-fixed


Attachments
(Av1) offlineTests.js: Fix finish(), Remove unused _slaveWindow._OfflineSlaveWindow, Merge _isMaster, _slaveWindow and hasSlave() into new _hasSlave [Checked in: Comment 13 & 26 & 28] (3.54 KB, patch)
2010-12-02 06:00 PST, Serge Gautherie (:sgautherie)
honzab.moz: review+
Details | Diff | Splinter Review
(Bv1) offlineTests.js: Fully fix finish(), 1 s/dump(/ok(false,/, Fix some nits [Checked in: Comment 21 & 26 & 28] (3.76 KB, patch)
2010-12-04 00:48 PST, Serge Gautherie (:sgautherie)
honzab.moz: review+
Details | Diff | Splinter Review
(Cv1) test_fallback.html: Remove unused gManifestUpdated, Fix some nits [Checked in: See comment 24+27 & 26 & 28] (3.36 KB, patch)
2010-12-04 00:50 PST, Serge Gautherie (:sgautherie)
honzab.moz: review+
Details | Diff | Splinter Review

Description Josh Matthews [:jdm] 2010-12-01 10:39:09 PST
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291226867.1291227768.14402.gz&fulltext=1#err1
Rev3 Fedora 12 mozilla-central debug test mochitests-2/5 on 2010/12/01 10:07:47

s: talos-r3-fed-025
TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | Exited with code 1 during test run
PROCESS-CRASH | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | missing output line for total leaks!

###!!! ASSERTION: XPConnect is being called on a scope without a 'Components' property!: 'Error', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 779
DEBUG_CheckForComponentsInScope [xpcwrappednativescope.cpp:780]
XPCWrappedNativeScope::FindInJSObjectScope [xpcwrappednativescope.cpp:838]
XPCWrappedNativeScope::FindInJSObjectScope [xpcprivate.h:1541]
GetContextFromObject [xpcwrappedjsclass.cpp:563]
nsXPCWrappedJSClass::CallMethod [xpcwrappedjsclass.cpp:1287]
nsXPCWrappedJS::CallMethod [xpcwrappedjs.cpp:588]
PrepareAndDispatch [xptcstubs_gcc_x86_unix.cpp:95]
nsThread::ProcessNextEvent [nsThread.cpp:626]
NS_ProcessNextEvent_P [nsThreadUtils.cpp:250]
mozilla::ipc::MessagePump::Run [MessagePump.cpp:110]
MessageLoop::RunInternal [message_loop.cc:220]
MessageLoop::RunHandler [message_loop.cc:203]
MessageLoop::Run [message_loop.cc:176]
nsBaseAppShell::Run [nsBaseAppShell.cpp:198]
nsAppStartup::Run [nsAppStartup.cpp:191]
XRE_main [nsAppRunner.cpp:3691]
main [nsBrowserApp.cpp:158]
libc.so.6 + 0x16bb6
Assertion failure: obj->containsSlot(slot), at ../../../js/src/jsinterp.cpp:5315
NEXT ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | Exited with code 1 during test run
INFO | automation.py | Application ran for: 0:09:57.954633
INFO | automation.py | Reading PID log: /tmp/tmpyPxkAJpidlog
PROCESS-CRASH | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | application crashed (minidump found)
Operating system: Linux
                  0.0.0 Linux 2.6.31.5-127.fc12.i686.PAE #1 SMP Sat Nov 7 21:25:57 EST 2009 i686
CPU: x86
     GenuineIntel family 6 model 23 stepping 10
     2 CPUs

Crash reason:  SIGABRT
Crash address: 0x7af

Thread 0 (crashed)
 0  linux-gate.so + 0x424
    eip = 0x0048e424   esp = 0xbfafbb10   ebp = 0xbfafbb28   ebx = 0x000007af
    esi = 0x00000000   edi = 0x00c7cff4   eax = 0x00000000   ecx = 0x000007af
    edx = 0x00000006   efl = 0x00200206
    Found by: given as instruction pointer in context
 1  libxul.so!JS_Assert [jsutil.cpp : 83 + 0xb]
    eip = 0x02a581f3   esp = 0xbfafbb30   ebp = 0xbfafbb48
    Found by: previous frame's frame pointer
 2  libxul.so!js::Interpret [jsinterp.cpp : 5315 + 0x3e]
    eip = 0x02bbeac7   esp = 0xbfafbb50   ebp = 0xbfafca38   ebx = 0x03467194
    Found by: call frame info
 3  libxul.so!js::RunScript [jsinterp.cpp : 657 + 0x21]
    eip = 0x0299ca68   esp = 0xbfafca40   ebp = 0xbfafca68   ebx = 0x03467194
    esi = 0x00000000   edi = 0xbfafcda0
    Found by: call frame info
 4  libxul.so!js::Invoke [jsinterp.cpp : 737 + 0x18]
    eip = 0x0299d758   esp = 0xbfafca70   ebp = 0xbfafcad8   ebx = 0x03467194
    esi = 0x00000000   edi = 0xbfafcda0
    Found by: call frame info
 5  libxul.so!js::ExternalInvoke [jsinterp.cpp : 858 + 0x19]
    eip = 0x0299e04d   esp = 0xbfafcae0   ebp = 0xbfafcb28   ebx = 0x03467194
    esi = 0x00000000   edi = 0xbfafcda0
    Found by: call frame info
 6  libxul.so!js::ExternalInvoke [jsinterp.h : 962 + 0x38]
    eip = 0x028f2948   esp = 0xbfafcb30   ebp = 0xbfafcb68   ebx = 0x03467194
    esi = 0xbfafce18   edi = 0xbfafcda0
    Found by: call frame info
 7  libxul.so!JS_CallFunctionValue [jsapi.cpp : 4973 + 0x3c]
    eip = 0x028f2aa8   esp = 0xbfafcb70   ebp = 0xbfafcbb8   ebx = 0x03467194
    esi = 0xbfafce18   edi = 0xbfafcda0
    Found by: call frame info
 8  libxul.so!nsXPCWrappedJSClass::CallMethod [xpcwrappedjsclass.cpp : 1694 + 0x4a]
    eip = 0x01ebf882   esp = 0xbfafcbc0   ebp = 0xbfafd048   ebx = 0x03467194
    esi = 0xb36fe020   edi = 0x01b279fe
    Found by: call frame info
 9  libxul.so!nsXPCWrappedJS::CallMethod [xpcwrappedjs.cpp : 588 + 0x35]
    eip = 0x01eb623b   esp = 0xbfafd050   ebp = 0xbfafd098   ebx = 0x03467194
    esi = 0x01ebe37e   edi = 0x01eb60d2
    Found by: call frame info
10  libxul.so!PrepareAndDispatch [xptcstubs_gcc_x86_unix.cpp : 95 + 0x3a]
    eip = 0x02729b1c   esp = 0xbfafd0a0   ebp = 0xbfafd198   ebx = 0x03467194
    esi = 0x00000003   edi = 0x01eb60d2
    Found by: call frame info
11  libxul.so!nsThread::ProcessNextEvent [nsThread.cpp : 626 + 0x18]
    eip = 0x02708f0a   esp = 0xbfafd1a0   ebp = 0xbfafd238   ebx = 0x03467194
    esi = 0x0945d414   edi = 0x02441ca2
    Found by: call frame info
12  libxul.so!NS_ProcessNextEvent_P [nsThreadUtils.cpp : 250 + 0x1f]
    eip = 0x02690690   esp = 0xbfafd240   ebp = 0xbfafd278   ebx = 0x03467194
    esi = 0x00000001   edi = 0x02441ca2
    Found by: call frame info
13  libxul.so!mozilla::ipc::MessagePump::Run [MessagePump.cpp : 110 + 0x15]
    eip = 0x024d98dc   esp = 0xbfafd280   ebp = 0xbfafd2c8   ebx = 0x03467194
    esi = 0x00000001   edi = 0x02441ca2
    Found by: call frame info
14  libxul.so!MessageLoop::RunInternal [message_loop.cc : 219 + 0x22]
    eip = 0x02774b9f   esp = 0xbfafd2d0   ebp = 0xbfafd2f8   ebx = 0x03467194
    esi = 0x09bef080   edi = 0x02441ca2
    Found by: call frame info
15  libxul.so!MessageLoop::RunHandler [message_loop.cc : 202 + 0xa]
    eip = 0x02774bb7   esp = 0xbfafd300   ebp = 0xbfafd308   ebx = 0x03467194
    esi = 0x09bef080   edi = 0x02441ca2
    Found by: call frame info
16  libxul.so!MessageLoop::Run [message_loop.cc : 176 + 0xa]
    eip = 0x02774c1b   esp = 0xbfafd310   ebp = 0xbfafd328   ebx = 0x03467194
    esi = 0x09bef080   edi = 0x02441ca2
    Found by: call frame info
17  libxul.so!nsBaseAppShell::Run [nsBaseAppShell.cpp : 192 + 0xc]
    eip = 0x02369a1e   esp = 0xbfafd330   ebp = 0xbfafd368   ebx = 0x03467194
    esi = 0x09bef080   edi = 0x02441ca2
    Found by: call frame info
18  libxul.so!nsAppStartup::Run [nsAppStartup.cpp : 191 + 0x1b]
    eip = 0x0209c069   esp = 0xbfafd370   ebp = 0xbfafd3a8   ebx = 0x03467194
    esi = 0x09bef080   edi = 0x02441ca2
    Found by: call frame info
19  libxul.so!XRE_main [nsAppRunner.cpp : 3691 + 0x1b]
    eip = 0x00fab497   esp = 0xbfafd3b0   ebp = 0xbfafd948   ebx = 0x03467194
    esi = 0x09bef080   edi = 0x02441ca2
    Found by: call frame info
20  firefox-bin!main [nsBrowserApp.cpp : 158 + 0x1d]
    eip = 0x08048e42   esp = 0xbfafd950   ebp = 0xbfafd9b8   ebx = 0x0804bacc
    esi = 0x093f20f0   edi = 0x026f36fc
    Found by: call frame info
21  libc-2.11.so + 0x16bb5
    eip = 0x00afebb6   esp = 0xbfafd9d0   ebp = 0xbfafda48   ebx = 0x00c58ff4
    esi = 0x00000000   edi = 0x00000000
    Found by: call frame info
22  firefox-bin + 0x9f0
    eip = 0x080489f1   esp = 0xbfafda50   ebp = 0x00000000
    Found by: previous frame's frame pointer
23  firefox-bin!Output [nsBrowserApp.cpp : 77 + 0x5]
    eip = 0x08048b42   esp = 0xbfafda54   ebp = 0x00000000
    Found by: stack scanning
24  ld-2.11.so + 0xecef
    eip = 0x00ad4cf0   esp = 0xbfafda68   ebp = 0x00000000
    Found by: stack scanning
Comment 1 Treeherder Robot 2010-12-01 12:14:00 PST
sgautherie.bz%free.fr
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291228903.1291229770.23041.gz
Rev3 Fedora 12 mozilla-central debug test mochitests-2/5 on 2010/12/01 10:41:43

s: talos-r3-fed-051
TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | Exited with code 1 during test run
PROCESS-CRASH | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | missing output line for total leaks!
Comment 2 Treeherder Robot 2010-12-01 12:14:28 PST
sgautherie.bz%free.fr
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291233303.1291234126.8303.gz
Rev3 Fedora 12 mozilla-central debug test mochitests-2/5 on 2010/12/01 11:55:03

s: talos-r3-fed-019
TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | Exited with code 1 during test run
PROCESS-CRASH | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | missing output line for total leaks!
Comment 3 Treeherder Robot 2010-12-01 12:16:49 PST
sgautherie.bz%free.fr
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291229211.1291230100.24185.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitests-2/5 on 2010/12/01 10:46:51

s: talos-r3-snow-015
TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | Exited with code 1 during test run
PROCESS-CRASH | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | missing output line for total leaks!
Comment 4 Serge Gautherie (:sgautherie) 2010-12-01 12:24:58 PST
This started in the changeset after the one for bug 615546.
Comment 5 Treeherder Robot 2010-12-01 12:49:28 PST
sgautherie.bz%free.fr
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291234890.1291236307.17567.gz
WINNT 5.2 mozilla-central debug test mochitests-2/5 on 2010/12/01 12:21:30

s: win32-slave49
610 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_bfcache.html | Test timed out.
TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | Exited with code -1073741819 during test run
PROCESS-CRASH | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | missing output line for total leaks!
Comment 6 Treeherder Robot 2010-12-01 12:56:39 PST
sgautherie.bz%free.fr
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291235614.1291236581.18814.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitests-2/5 on 2010/12/01 12:33:34

s: talos-r3-snow-054
TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | Exited with code 1 during test run
PROCESS-CRASH | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | missing output line for total leaks!
Comment 7 Treeherder Robot 2010-12-01 12:58:03 PST
philringnalda%gmail.com
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291235614.1291236581.18814.gz
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitests-2/5 on 2010/12/01 12:33:34

s: talos-r3-snow-054
TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | Exited with code 1 during test run
PROCESS-CRASH | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | missing output line for total leaks!
Comment 8 Serge Gautherie (:sgautherie) 2010-12-01 13:39:23 PST
The previous build:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291223602.1291226192.7754.gz&fulltext=1
Rev3 Fedora 12 mozilla-central debug test mochitests-2/5 on 2010/12/01 09:13:22
{
5081 INFO TEST-PASS | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | http://mochi.test:8888/tests/dom/tests/mochitest/ajax/offline/namespace2/non-existing.html should not exist in the offline cache
5082 INFO TEST-END | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | finished in 6794ms
}

*****

The first build, which was green:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291224891.1291227563.13488.gz&fulltext=1
Rev3 Fedora 12 mozilla-central debug test mochitests-2/5 on 2010/12/01 09:34:51
{
...
5083 INFO TEST-PASS | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | http://mochi.test:8888/tests/dom/tests/mochitest/ajax/offline/namespace2/non-existing.html should not exist in the offline cache
###!!! ASSERTION: XPConnect is being called on a scope without a 'Components' property!: 'Error', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 779
...
JavaScript error: http://mochi.test:8888/tests/SimpleTest/SimpleTest.js, line 555: SimpleTest is undefined
5084 INFO TEST-END | /tests/dom/tests/mochitest/ajax/offline/test_fallback.html | finished in 6033ms
}
Same "XPConnect" assertion,
but "JavaScript error" instead of "Assertion failure: obj->containsSlot(slot)".

Code was:
{
549SimpleTest._finishNow = function () {
550 if (parentRunner) {
551 // The test is running in an iframe, and its parent has a TestRunner.
552 parentRunner.testFinished(SimpleTest._tests);
553 } else {
554 // The test is running alone in the window.
555 SimpleTest.showReport();
556 }
557};
}

***

Test code is:

http://mxr.mozilla.org/mozilla-central/source/dom/tests/mochitest/ajax/offline/test_fallback.html?force=1
107 function finalize()
114   OfflineTest.checkCache("http://mochi.test:8888/tests/dom/tests/mochitest/ajax/offline/namespace2/non-existing.html", false);
115 
116   OfflineTest.teardown();
117   OfflineTest.finish();

http://mxr.mozilla.org/mozilla-central/source/dom/tests/mochitest/ajax/offline/offlineTests.js
145 finish: function()
146 {
147   SimpleTest.finish();
148 
149   if (this._masterWindow) {
150     this._masterWindow.OfflineTest.finish();
151     window.close();
152   }
153 },

"I" will have to check how this finish() code is supposed to work exactly...
Hints welcomed!
Comment 9 Serge Gautherie (:sgautherie) 2010-12-02 06:00:46 PST
Created attachment 494690 [details] [diff] [review]
(Av1) offlineTests.js: Fix finish(), Remove unused _slaveWindow._OfflineSlaveWindow, Merge _isMaster, _slaveWindow and hasSlave() into new _hasSlave
[Checked in: Comment 13 & 26 & 28]

Fixes the 'JavaScript error' on my local Windows 2000.
Comment 10 Honza Bambas (:mayhemer) 2010-12-03 08:29:23 PST
I can steal the review here.

What is exactly cause of the assertion failure?  Is it because _slaveWindow is pointing to something non-existing?
Comment 11 Serge Gautherie (:sgautherie) 2010-12-03 08:55:02 PST
(In reply to comment #10)
> What is exactly cause of the assertion failure?

The issue(s) is that SimpleTest.finish() gets called
1) twice (first from slave window then from master window)
2) synchronously (when test has obviously not fully ended yet)

NB: I'm not adding another executeSoon, as bug 615546 will do that automatically.
Comment 12 Honza Bambas (:mayhemer) 2010-12-03 09:08:42 PST
Comment on attachment 494690 [details] [diff] [review]
(Av1) offlineTests.js: Fix finish(), Remove unused _slaveWindow._OfflineSlaveWindow, Merge _isMaster, _slaveWindow and hasSlave() into new _hasSlave
[Checked in: Comment 13 & 26 & 28]

Yes, that's what I found out during the review as well.  Good fix.

r=honzab
Comment 13 Serge Gautherie (:sgautherie) 2010-12-03 09:36:39 PST
Comment on attachment 494690 [details] [diff] [review]
(Av1) offlineTests.js: Fix finish(), Remove unused _slaveWindow._OfflineSlaveWindow, Merge _isMaster, _slaveWindow and hasSlave() into new _hasSlave
[Checked in: Comment 13 & 26 & 28]

http://hg.mozilla.org/mozilla-central/rev/2357fce3dceb
Comment 14 Serge Gautherie (:sgautherie) 2010-12-03 11:27:45 PST
Hum, the 'offline' tests are still passing, but we get this assertion after each of them now :-/
I need to compile a debug build to check that...
Comment 15 Serge Gautherie (:sgautherie) 2010-12-04 00:48:58 PST
Created attachment 495220 [details] [diff] [review]
(Bv1) offlineTests.js: Fully fix finish(), 1 s/dump(/ok(false,/, Fix some nits
[Checked in: Comment 21 & 26 & 28]

I had thought about that |this._masterWindow.| then I forgot about it as I was testing with an opt build :-/
Comment 16 Serge Gautherie (:sgautherie) 2010-12-04 00:50:10 PST
Created attachment 495221 [details] [diff] [review]
(Cv1) test_fallback.html: Remove unused gManifestUpdated, Fix some nits
[Checked in: See comment 24+27 & 26 & 28]
Comment 17 Serge Gautherie (:sgautherie) 2010-12-04 00:56:13 PST
NB: These 'offline' tests report some errors in the Error Console and trigger 3 other assertions ... but all those are unrelated to this bug, so I won't care atm.
Comment 18 Serge Gautherie (:sgautherie) 2010-12-05 05:24:42 PST
Ftr, both Bv1 and Cv1 patches passed on Try: see bug 615546 comment 10.
Comment 19 Honza Bambas (:mayhemer) 2010-12-07 14:45:09 PST
Comment on attachment 495220 [details] [diff] [review]
(Bv1) offlineTests.js: Fully fix finish(), 1 s/dump(/ok(false,/, Fix some nits
[Checked in: Comment 21 & 26 & 28]

r=honzab
Comment 20 Honza Bambas (:mayhemer) 2010-12-07 14:50:02 PST
Comment on attachment 495221 [details] [diff] [review]
(Cv1) test_fallback.html: Remove unused gManifestUpdated, Fix some nits
[Checked in: See comment 24+27 & 26 & 28]

>-  if (gStep == 105) {
>+  if (gStep++ == 105)
>     finalize();
>-  }
>-
>-  ++gStep;

You are just simplifying or want to have gStep increased before call to finalize()?  If the former, I would rather leave the previous form.  ++ in the condition could be overlooked.  Up to you.

r=honzab.
Comment 21 Serge Gautherie (:sgautherie) 2010-12-07 16:07:53 PST
Comment on attachment 495220 [details] [diff] [review]
(Bv1) offlineTests.js: Fully fix finish(), 1 s/dump(/ok(false,/, Fix some nits
[Checked in: Comment 21 & 26 & 28]

http://hg.mozilla.org/mozilla-central/rev/6d18466cf3e6
Comment 22 Serge Gautherie (:sgautherie) 2010-12-07 16:16:56 PST
(In reply to comment #20)
> Comment on attachment 495221 [details] [diff] [review]
> (Cv1) test_fallback.html: Remove unused gManifestUpdated, Fix some nits
> 
> >-  if (gStep == 105) {
> >+  if (gStep++ == 105)
> >     finalize();
> >-  }
> >-
> >-  ++gStep;
> 
> You are just simplifying or want to have gStep increased before call to
> finalize()?  If the former, I would rather leave the previous form.  ++ in the
> condition could be overlooked.  Up to you.

Though it's just a nit atm, my main goal is to have nothing executing after finalize().
Having the last/extra gStep increase before the call is just some kind of bonus.
I'm fine with leaving the increase as is but adding a |return;| after finalize() instead, if you prefer.
Comment 23 Honza Bambas (:mayhemer) 2010-12-07 16:28:56 PST
(In reply to comment #22)
> I'm fine with leaving the increase as is but adding a |return;| after
> finalize() instead, if you prefer.

Agree.
Comment 24 Serge Gautherie (:sgautherie) 2010-12-07 17:00:16 PST
Comment on attachment 495221 [details] [diff] [review]
(Cv1) test_fallback.html: Remove unused gManifestUpdated, Fix some nits
[Checked in: See comment 24+27 & 26 & 28]

http://hg.mozilla.org/mozilla-central/rev/25a128ff3cbf
Comment 25 Serge Gautherie (:sgautherie) 2010-12-07 19:04:07 PST
V.Fixed, per
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291771001.1291773143.27597.gz&fulltext=1
WINNT 5.2 mozilla-central debug test mochitests-2/5 on 2010/12/07 17:16:41
and
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291774138.1291776469.9513.gz&fulltext=1
WINNT 5.2 mozilla-central debug test mochitests-2/5 on 2010/12/07 18:08:58
Comment 26 Serge Gautherie (:sgautherie) 2010-12-07 19:27:07 PST
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/f1ebb30ba752
Comment 27 Serge Gautherie (:sgautherie) 2010-12-07 19:33:20 PST
Comment on attachment 495221 [details] [diff] [review]
(Cv1) test_fallback.html: Remove unused gManifestUpdated, Fix some nits
[Checked in: See comment 24+27 & 26 & 28]

(In reply to comment #24)
> http://hg.mozilla.org/mozilla-central/rev/25a128ff3cbf

Cv1, with comment 23 suggestion(s).
Comment 28 Serge Gautherie (:sgautherie) 2010-12-08 06:08:28 PST
(In reply to comment #26)
> http://hg.mozilla.org/releases/mozilla-1.9.2/rev/f1ebb30ba752

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ec5602f129b5
without the s/dump/ok/ change, as this block was not landed there.
Comment 29 Honza Bambas (:mayhemer) 2010-12-08 07:31:08 PST
And thanks for fixing this, Serge.

Note You need to log in before you can comment on or make changes to this bug.