Closed Bug 536603 Opened 12 years ago Closed 2 years ago

Intermittent layout/style/test/test_css_cross_domain.html | quirks IB1l - got rgb(255, 0, 0), expected rgb(0, 255, 0)

Categories

(Testing :: General, defect)

Product:
Testing
Component:
General
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox18 disabled, firefox19 disabled)

Status:
RESOLVED INACTIVE
Tracking Flags:
Tracking Status
firefox18 --- disabled
firefox19 --- disabled

People

(Reporter: zwol, Assigned: gbrown)

References

Details

(Keywords: intermittent-failure, Whiteboard: [test disabled on Android][leave open])

Attachments

(3 files, 4 obsolete files)

candidate fix
1.97 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
"tweak to avoid hardwired constants", as landed on trunk
2.60 KB, patch
zwol
: review+
Details | Diff | Splinter Review
rollup patch for branches
2.43 KB, patch
Details | Diff | Splinter Review 
See e.g. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261597890.1261598690.3423.gz (OSX 10.5.2 opt mochitests-4/5).  The test uses .sjs scripts extensively, and the only plausible source of an intermittent failure is some sort of glitch in httpd.js.
Blocks: 438871
Whiteboard: [orange]
The original failure, in slightly more detail, was:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261597890.1261598690.3423.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/23 11:51:30  
s: moz2-darwin9-slave17
6311 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6317 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"

And here's a new one:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261604924.1261605755.17082.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/23 13:48:44
s: bm-xserve17
6309 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC1i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
I landed some debugging code whose output might help figure this out:

http://hg.mozilla.org/mozilla-central/rev/76e9dfaba122
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261608593.1261609725.27944.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/23 14:49:53
s: moz2-darwin9-slave05
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261610267.1261611889.20304.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/23 15:17:47
s: moz2-darwin9-slave17
(before the debug code)
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261612426.1261615009.23822.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/23 15:53:46
s: moz2-darwin9-slave14
(with debug output)

and the utterly freaky

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261610251.1261613876.11407.gz
WINNT 5.2 mozilla-central debug test mochitests-4/5 on 2009/12/23 15:17:31
s: win32-slave17
6247 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | Test timed out.
6250 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_eof_handling.html | quirks IA1i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"

which appears to be a claim that test_css_cross_domain.html timed out, followed by tests from it claiming that they are tests from test_css_eof_handling.html (followed eventually by a too many timeouts abort of the suite).
(In reply to comment #5)
> http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261612426.1261615009.23822.gz
> OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/23 15:53:46
> s: moz2-darwin9-slave14
> (with debug output)

Which showed that when the test fails, I get:

6305 INFO TEST-PASS | /tests/layout/style/test/test_css_cross_domain.html | @import rule importing redirect.sjs?http://example.org/tests/layout/style/test/ccd.sjs?IC2iq has no rules

whereas when it passed I got:

6254 INFO TEST-PASS | /tests/layout/style/test/test_css_cross_domain.html | selector text for rule @import-ed at redirect.sjs?http://example.org/tests/layout/style/test/ccd.sjs?IC2iq that specifies color lime
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261613534.1261616193.4233.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/23 16:12:14
s: moz2-darwin9-slave09
It could be we're hitting the network load timeout because we can't load that many files in the amount of time before we time out a network load.  (Or there's clock skew.)
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261616158.1261618031.24756.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/23 16:55:58
s: moz2-darwin9-slave17
(late reporting pre-debug)
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/23 21:07:14
s: moz2-darwin9-slave09
6360 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6366 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6552 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6558 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261627835.1261630512.1161.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/23 20:10:35
s: moz2-darwin9-slave12
6358 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC1i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6360 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6550 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC1i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6552 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
And, to be clear, I landed a second debugging patch:
http://hg.mozilla.org/mozilla-central/rev/5275dda44819


http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261657781.1261658471.17410.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/24 04:29:41
s: moz2-darwin9-slave09
6360 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6366 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6552 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6558 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
It seems interesting that all the failures are the cases where we're handling a redirect for an @import.  That makes me wonder if this is actually a code bug.
I backed out the current debugging code:
http://hg.mozilla.org/mozilla-central/rev/91bb153b4f87
but I hope to add some more in a bit.
Though this is also OS X only, which makes it seem more likely to be a threading bug (happens when not running in a VM), which would be more likely to be in the server.
Landed http://hg.mozilla.org/mozilla-central/rev/aabd98c04dc9 to see if the server is getting different requests in the failure case.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261657781.1261658471.17410.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/24 04:29:41
s: moz2-darwin9-slave09

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261664140.1261665265.300.gz
OS X 10.5.2 mozilla-central test mochitests-4/5 on 2009/12/24 06:15:40  
s: moz2-darwin9-slave05

(both before the change)
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261694148.1261695251.12278.gz

It looks like the IC2iq and ID2iq requests never even got to redirect.js, never mind to ccd.js.
And all the failures are also the quirks mode case.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261744023.1261744778.26042.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/25 04:27:03
s: moz2-darwin9-slave14
6312 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6318 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261740924.1261742836.2503.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/25 03:35:24
s: moz2-darwin9-slave13
6312 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6318 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261740930.1261742831.2493.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2009/12/25 03:35:30  
s: moz2-darwin9-slave16
6310 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC1i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6312 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6318 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6324 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks JA2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
I wonder if it makes a difference if you swap the IFRAMEs for ccd-quirks.html and ccd-standards.html (i.e. make them load in the opposite order).
So last night I landed one additional round of debugging code:
http://hg.mozilla.org/mozilla-central/rev/f60b3bbfa8ce

There's only been one non-optimized failure since that landed:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1261717178.1261718162.30060.gz
but none of my printfs showed up.

So I backed out the debugging code:
http://hg.mozilla.org/mozilla-central/rev/fa5f65c611ce
http://hg.mozilla.org/mozilla-central/rev/69a9fe6eb6fb

and, for now, gave up and disabled the quirks mode test on Mac:
http://hg.mozilla.org/mozilla-central/rev/a35c4e47392d


And, for the record, I also backed out the other debugging code that I landed:
http://hg.mozilla.org/mozilla-central/rev/91bb153b4f87
and:
http://hg.mozilla.org/mozilla-central/rev/14710fc3a5a7
http://hg.mozilla.org/mozilla-central/rev/9fb14305689f
OS: Linux → Mac OS X
Summary: Mochitest layout/style/test/test_css_cross_domain.html intermittent failure → Mochitest layout/style/test/test_css_cross_domain.html intermittent failure (test half disabled on Mac)
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1262676477.1262678934.28068.gz
Linux mozilla-central debug test mochitests-4/5 on 2010/01/04 23:27:57
As an experiment, I hacked up layout/style/test/Makefile.in to present 1000 copies of the problem test to the mochitest driver, rather than its usual set of tests.  I then ran just that directory (on my Linux box -- OSX will be next).  So far it's processed 150 or so of the copies; no failures yet, but httpd.js is getting progressively slower and slower about responding to requests.
Further to previous, patching server.js to GC more often (as per bug 508128) helps but only a little.  I think I'm giving up on trying to reproduce it on Linux, even though that would be more convenient for debugging.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1262996123.1262998450.25035.gz
OS X 10.5.2 mozilla-central debug test mochitests-4/5 on 2010/01/08 16:15:23
s: moz2-darwin9-slave17
6427 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | standards IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
OS X 10.5.2 mozilla-central debug test mochitests-4/5
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263018828.1263021304.8706.gz

TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | standards ID2l - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
Linux mozilla-central debug test mochitests-4/5 [testfailed] Started 13:28, finished 14:14
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263245290.1263248035.10061.gz&fulltext=1
I've taken all of dbaron's debugging code, added some more of my own, turned the quirks test back on for Mac, and pushed the result to the try server.  If that goes orange, I am hoping there will be enough information in the logs to figure out what's really going on.  If not, I may need to put that patch on m-c temporarily.

I have not been able to reproduce the problem at all on Mac, with or without the debugging code; however, I've noticed that if I load the test page and hit ^R a whole bunch of times, on some of the cycles the iframes take several seconds to load.  This makes me wonder if the "real" mochitests have aggressive network timeouts that are not in place when I'm running them locally.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263849429.1263852151.32628.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2010/01/18 13:17:09
s: moz2-darwin9-slave21
6495 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6501 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6687 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6693 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263851899.1263853054.10176.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2010/01/18 13:58:19
s: moz2-darwin9-slave15
6495 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6501 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6687 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6693 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
This morning, I landed the patch I mentioned in comment 32 on m-c to get more data:
http://hg.mozilla.org/mozilla-central/rev/153390d0f83b

The logs philor noted in comments 33 and 34 have the additional data I wanted (and it looks like there's more coming) so I backed it out again:
http://hg.mozilla.org/mozilla-central/rev/bb9789de8b20
http://hg.mozilla.org/mozilla-central/rev/b22a96691204
I'm not going to look at this in detail till tomorrow (it *is* a holiday here in the states) but here's the one-sentence cursory analysis from the logs: Under load, either httpd.js is silently dropping a couple of requests on the floor (and by silently, I mean there is not a single line of its debug output that mentions them) or the browser is silently failing to issue a couple of requests in the first place!
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263856185.1263857222.23142.gz
OS X 10.5.2 mozilla-central opt test mochitests-4/5 on 2010/01/18 15:09:45
s: moz2-darwin9-slave14
Could you be exceeding the size of the server-socket's queue of pending connections, possibly, and the resultant failure is silent?  I have no idea what exactly the failure condition is when that case (looks like the default is 5 connections, but we could specify a much larger limit very easily) is hit.
Attached patch candidate fix — — Splinter Review 
I think you might be right.  The aBacklog argument to nsIServerSocket::Init[WithAddress] is passed directly to PR_Listen, which calls listen(2) [through a ridiculous number of layers of indirection], for which the manpage says

       The backlog argument defines the maximum length to which the queue of
       pending connections for sockfd may grow. If a connection request
       arrives when the queue is full, the client may receive an error with an
       indication of ECONNREFUSED or, if the underlying protocol supports
       retransmission, the request may be ignored so that a later reattempt at
       connection succeeds.

Meantime, browser-side, our default for network.http.max-connections-per-server is 15, and this test kicks off nearly 100 network requests.  I can definitely see httpd.js falling behind under load to the point where a measly 5 pending accept() requests is not enough.
Assignee: nobody → zweinberg
Status: NEW → ASSIGNED
Comment on attachment 422276 [details] [diff] [review]
candidate fix

Seems sane enough
Attachment #422276 - Flags: review+
We've now had four green Mo4 and Md4 cycles on OSX since I landed the fix; I'm calling this done.  Feel free to reopen if it comes back.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Version: unspecified → Trunk
Great debugging work!
Gavin suggested on IRC that httpd.js should look up network.http.max-connections-per-server rather than hardwire a constant that needs to be bigger.  This patch (on top of the previous one) makes that change.

If approved, I'd like to backport these changes to all active branches.  This was no fun to track down, and might be causing other random failures as well.
Attachment #422390 - Flags: review?(jwalden+bmo)
Reopening to track the pref-vs-hardwired-number issue.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Attachment #422390 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 422390 [details] [diff] [review]
tweak to avoid hardwired constants

>diff --git a/netwerk/test/httpserver/httpd.js b/netwerk/test/httpserver/httpd.js

>+/** The XPCOM prefs service.  */

Extra space at end of comment.


>+var gRootPrefBranch = null;
>+function getRootPrefBranch()
>+{
>+  if (!gRootPrefBranch)
>+    gRootPrefBranch = Cc["@mozilla.org/preferences-service;1"]
>+      .getService(Ci.nsIPrefBranch);

The body of this |if| is multiline, so it should be braced, and the alignment should be fixed as follows:

>+  if (!gRootPrefBranch)
>+  {
>+    gRootPrefBranch = Cc["@mozilla.org/preferences-service;1"]
>+                        .getService(Ci.nsIPrefBranch);
>+  }


>+    // The listen queue needs to be long enough to handle
>+    // network.http.max-connections-per-server concurrent connections,
>+    // plus a safety margin in case some other process is talking to
>+    // the server as well.
>+    var pb = getRootPrefBranch();
>+    var maxConn = pb.getIntPref("network.http.max-connections-per-server") + 5;

|prefs| and |maxConnections|, please.


r=me with those changes
I've made all the changes you requested, but I would like to explain some of my style idiosyncracies a little...

(In reply to comment #46)
> >+/** The XPCOM prefs service.  */
> 
> Extra space at end of comment.

Sorry - I habitually type two spaces after each period, even at the end of the comment (this was required style in GCC).  Moz doesn't seem to be consistent about the number of spaces after sentence-ending punctuation, even within a file, so I wasn't looking for file style.

> >+  if (!gRootPrefBranch)
> >+  {
> >+    gRootPrefBranch = Cc["@mozilla.org/preferences-service;1"]
> >+                        .getService(Ci.nsIPrefBranch);
> >+  }

Changed, but I'd like to register my total amazement to find code within Mozilla where the open brace after an "if" goes on its own line, and to hear of a coding style where single-statement blocks should be braced only if they take more than one line (I'm used to "always" or "never" for that question).

Also, I do think the statement reads better with .getService lined up with the Cc[] but Emacs doesn't want to do that automatically for me, so I'm liable to forget it in the future.  I apologize in advance.

> >+    var pb = getRootPrefBranch();
> >+    var maxConn = pb.getIntPref("network.http.max-connections-per-server") + 5;
> 
> |prefs| and |maxConnections|, please.

This makes the "var maxConnections" line not fit on one 80-column line, so I have now made it read

    var maxConnections =
      prefs.getIntPref("network.http.max-connections-per-server") + 5;

Hope that's okay.
Here's the patch I just pushed to trunk.  Branch patch to follow.
Attachment #422390 - Attachment is obsolete: true
Attachment #424367 - Flags: review+
Here's the combined version that I will shortly land on 1.9.2 and 1.9.1.  I'm not going to put it on any CVS-based branches unless someone specifically wants it there.
(In reply to comment #47)
> Sorry - I habitually type two spaces after each period, even at the end of the
> comment (this was required style in GCC).  Moz doesn't seem to be consistent
> about the number of spaces after sentence-ending punctuation, even within a
> file, so I wasn't looking for file style.

Sure, we're inconsistent about French-style sentence spacing; at end-of-comment I can't remember seeing anything but a single space.

Also, GNU style FAIL yet again.  :-)  This one is even more bizarre than indenting blocks and *further* indenting their contents!


> > >+  if (!gRootPrefBranch)
> > >+  {
> > >+    gRootPrefBranch = Cc["@mozilla.org/preferences-service;1"]
> > >+                        .getService(Ci.nsIPrefBranch);
> > >+  }
> 
> Changed, but I'd like to register my total amazement to find code within
> Mozilla where the open brace after an "if" goes on its own line, and to hear
> of a coding style where single-statement blocks should be braced only if they
> take more than one line (I'm used to "always" or "never" for that question).

SpiderMonkey style has always been to brace the body of if/if-else/while/for if the body's multiline, the condition is multiline, or the init/step/terminate is multiline:

https://wiki.mozilla.org/JavaScript:SpiderMonkey:C_Coding_Style

I picked it up from there, and it makes a lot of sense to me.  The goal in this case, I believe, is that if there's no brace, you can always brainlessly skip over exactly one line when skimming the code, versus having to look to the end of every line beneath to determine whether the statement ends there or not.

As for open-brace on new line, I wrote the file, I used the style rules I've come to prefer over time.  Everyone else is just wrong.  :-P  I like it because braces are always aligned, which makes it much easier to match up opening and closing braces and to find the start of a block -- if you see a } you're not looking for one of half a dozen things, you're looking for exactly one, in an exact location -- {.  It also makes the transition from single-line statement to multi-line block not disturb the line containing the condition.  I'm sure I could come up with a few other advantages if I felt like spending time doing so.

Brace on new line is less common in Mozilla, but there's still a fair amount of it.  Try this command in a source directory to get a good number of examples (and false positives, which -B 1 makes easy to skim over):

pcregrep -B 1 '--include=^.*\.(cpp|js|c|h)$' -r '^\s+\{\s+$' . | less


> This makes the "var maxConnections" line not fit on one 80-column line, so I
> have now made it read
> 
>     var maxConnections =
>       prefs.getIntPref("network.http.max-connections-per-server") + 5;
> 
> Hope that's okay.

Exactly the right change!
I object to the idea that just because you get to create a new file, you can make up whatever style rules you like.
Not just a new file, a new component and set of associated files -- a full server is rather different, it seems, than, say, nsDOMDOMTokenList.cpp (if such a thing exists).  httpd.js analogizes more closely to qcms than to an additional file in an existing component, I think.
(I don't know whether qcms adopted its own style -- although I know it did with respect to using C99 integer types, a similar issue that's not completely the same -- but I think it would have been well-justifiable for it to have done so if it wanted.)
Sure looks to have reappeared, or something similar.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1265957405.1265958780.23335.gz
OS X 10.5.2 mozilla-central debug test mochitests-4/5 on 2010/02/11 22:50:05  
s: bm-xserve17
6501 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC1i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6503 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6509 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6510 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks ID2l - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6531 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks JD1i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6532 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks JD1l - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6533 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks JD2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6538 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | standards IA1l - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6550 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | standards IC1l - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6551 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | standards IC2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6552 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | standards IC2l - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6556 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | standards ID1l - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6557 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | standards ID2i - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
6558 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | standards ID2l - got "rgb(255, 0, 0)", expected "rgb(0, 255, 0)"
TEST-UNEXPECTED-FAIL | automation.py | application timed out after 330 seconds with no output
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1267403433.1267405779.18973.gz
WINNT 5.2 mozilla-central debug test mochitests-4/5 on 2010/02/28 16:30:33
s: win32-slave34
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1267475153.1267477288.32650.gz
OS X 10.5.2 mozilla-central debug test mochitests-4/5 on 2010/03/01 12:25:53
s: bm-xserve12
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1267617432.1267620781.32051.gz
WINNT 5.2 mozilla-central debug test mochitests-4/5 on 2010/03/03 03:57:12
s: win32-slave01
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1267972551.1267975082.20116.gz
WINNT 5.2 mozilla-central debug test mochitests-4/5 on 2010/03/07 06:35:51
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268108429.1268110790.26227.gz
OS X 10.5.2 mozilla-central debug test mochitests-4/5 on 2010/03/08 20:20:29
s: moz2-darwin9-slave05
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268478659.1268481385.22543.gz
WINNT 5.2 mozilla-central debug test mochitests-4/5 on 2010/03/13 03:10:59
s: win32-slave17
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268475242.1268477726.14265.gz
WINNT 5.2 mozilla-central debug test mochitests-4/5 on 2010/03/13 02:14:02
s: win32-slave17
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1269142630.1269144921.20035.gz
OS X 10.5.2 mozilla-central debug test mochitests-4/5 on 2010/03/20 20:37:10
s: moz2-darwin9-slave24
I was hoping this would go away on its own, but clearly it's not gonna.  I'm out of town for the next week and a half.  Could I persuade someone to merge up and reapply the debugging patch in http://hg.mozilla.org/mozilla-central/rev/153390d0f83b (you won't need the changes to httpd.js or server.js) and leave it like that until we see a few more failures, continuing to post the links here?  I'll stare at the logs when I get back.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1269249377.1269251622.10305.gz
WINNT 5.2 mozilla-central debug test mochitests-4/5 on 2010/03/22 02:16:17
s: win32-slave35
Assuming I get record/replay all set up and working in time, I'll probably try to reproduce and debug this.  I have it mostly set up now, so with luck I should be working at reproducing this later today.
(In reply to comment #67)
> Assuming I get record/replay all set up and working in time, I'll probably try
> to reproduce and debug this.  I have it mostly set up now, so with luck I
> should be working at reproducing this later today.

Cool.  I'll be back from vacation as of tomorrow (Mountain View time) so don't hesitate to poke me if you need help.
Since this hasn't popped up since March, I'm going to close it again.  Please do reopen if it recurs.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Seems to have popped back up :/
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I no longer work for Mozilla and have no intention of working on this bug as a
volunteer.
Assignee: zackw → nobody
However, I am happy to *discuss* this bug with anyone who wishes to try to pin it down.
Just for **** and giggles, how about we change httpd.js's

      prefs.getIntPref("network.http.max-connections-per-server") + 5;

to

      prefs.getIntPref("network.http.max-connections-per-server") * 4;

and see if that makes the problem go away.
Should be fixed again now.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 9 years ago
Resolution: --- → FIXED
Guess not...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
So uh, how hard would it be to add extensions to data: that would let it emulate 3xx redirects and both same- and cross-domain loads?  Because I really have no idea what else to try.
Attached patch another stab at a fix (obsolete) — — Splinter Review 
It finally dawned on me that the remaining problem here isn't httpd.js; it's that we're relying on window.onload in the outermost document to tell us that two iframe documents are completely done loading.  That's documented to work (https://developer.mozilla.org/en-US/docs/DOM/window.onload) but it seems to be folk wisdom among web developers that it doesn't work reliably.  Since this test isn't _about_ iframes, let's just avoid the issue.  This patch splits up test_css_cross_domain.html into four subtests: test_css_crossdomain_{standards,quirks}_{import,link}.html and so eliminates the iframes.  This also reduces the load on httpd.js considerably, since each subtest will only do 24 resource loads (not counting redirects) rather than one test doing 98.

I'm not sure who's an appropriate reviewer here.
Assignee: nobody → zackw
Comment on attachment 661781 [details] [diff] [review]
another stab at a fix

for lack of a better idea
Attachment #661781 - Flags: review?(dbaron)
(In reply to Zack Weinberg (:zwol) from comment #112)
> It finally dawned on me that the remaining problem here isn't httpd.js; it's

I disagree; I think it is httpd.js.

In particular, a change to our HTTP connection settings made the problem reappear on Mac OS X.  Then a change to httpd.js to match that caused it to go away on Mac OS X, but to appear on Android.
The failures never completely went away, though, and they have never been limited to OSX or Android.  It just seems to be failing more often on those platforms than on Windows or Linux.

Do you think splitting up the test (and also making it not assume that iframes are fully loaded when window.onload fires) is actively a bad idea?
I've landed the attached patch temporarily on m-c to get more diagnostics for this bug.  It adds a whole bunch of event listeners and changes when the link tags become "live", and therefore might perturb the bug out of existence -- it's unfortunate that there is no JS property on link elements (and @import rules) that just *says* whether the stylesheet load has completed.  Anyway, at the rate we are getting failures here, we should know pretty quick.

https://hg.mozilla.org/mozilla-central/rev/db922dc3669e
Attachment #661781 - Flags: review?(dbaron)
Ok, the previous round of diagnostics has convinced me that this really is a problem on the httpd.js side: specifically, all of the <link> tags report themselves as having either loaded successfully or received an HTTP error (some of the subtests here intentionally produce HTTP errors) even when styles fail to get applied.  Also, the failures are always cases where the external stylesheet should have been applied, but wasn't.

So, this round of diagnostics instruments the .sjs script, so we can find out whether control is reaching it reliably, and whether it is producing the response bodies it's supposed to.

https://hg.mozilla.org/mozilla-central/rev/784278a21091 (revert prev round)
https://hg.mozilla.org/mozilla-central/rev/645f41404b82 (new round)
Attachment #661781 - Attachment is obsolete: true
Attachment #663813 - Attachment is obsolete: true
I have backed the "round 2" diagnostics out again because the dump() calls from inside the .sjs script could get printed in the middle of some other test's logs, potentially causing great confusion.

Anyhow, two fails is enough to indicate that this is still a case of requests getting lost before they get to the .sjs script -- from the first log, we have:

TEST-INFO | ccd.sjs | IB1ls | text/css | 200 | #IB1l{background-color:lime}
TEST-INFO | ccd.sjs | IB2ls | text/css | 200 | <html>{}#IB2l{background-color:lime}</html>
18789 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IB1l - got rgb(255, 0, 0), expected rgb(0, 255, 0)
18791 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_css_cross_domain.html | quirks IB2l - got rgb(255, 0, 0), expected rgb(0, 255, 0)
18837 INFO TEST-PASS | /tests/layout/style/test/test_css_cross_domain.html | standards IB1l - rgb(0, 255, 0) should equal rgb(0, 255, 0)
18839 INFO TEST-PASS | /tests/layout/style/test/test_css_cross_domain.html | standards IB2l - rgb(0, 255, 0) should equal rgb(0, 255, 0)

There should be "ccd.sjs | IB1lq" and "ccd.sjs | IB2lq" lines to match the "quirks IB1l" and "quirks IB2l" lines that failed.  Same phenomenon in the other log.
Attachment #666976 - Attachment is obsolete: true
What doesn't make sense here is, if this is still the original problem of requests getting dropped from the accept() queue at the TCP level, then diagnostic round 1 should *not* have indicated successful load completion for all the link tags.

... Unless it was getting masked by the "or error" check, come to think of it.
Summary: Mochitest layout/style/test/test_css_cross_domain.html intermittent failure (test half disabled on Mac) → Intermittent layout/style/test/test_css_cross_domain.html | quirks IB1l - got rgb(255, 0, 0), expected rgb(0, 255, 0)
Toporange with several hundred failures not telling us anything new. Disabled for now on Android as part of the drive to reduce our OrangeFactor:
https://hg.mozilla.org/integration/mozilla-inbound/rev/057844e11446
Whiteboard: [orange] → [orange][test disabled on Android][leave open]
Whiteboard: [orange][test disabled on Android][leave open] → [test disabled on Android][leave open]
Disabled on beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/a4045d9e4dfd
status-firefox18: --- → disabled
status-firefox19: --- → disabled
Target Milestone: mozilla1.9.1 → ---
Resetting owner to default per Zack's request.
Assignee: zackw → nobody
Component: httpd.js → General
Assignee: nobody → gbrown
Status: REOPENED → RESOLVED
Closed: 9 years ago → 2 years ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.