Bug 18661 - Proxy server issue in Sunday's Nightly
: Proxy server issue in Sunday's Nightly
Status: RESOLVED FIXED
: WebKit
Platform
: 528+ (Nightly build)
: Macintosh Mac OS X 10.5
: P2 Major
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2008-04-21 10:31 PST by
Modified: 2008-04-28 23:59 PST (History)


Attachments
Fix - work in progress (79.69 KB, patch)
2008-04-25 00:03 PST, Alexey Proskuryakov
no flags Review Patch | Details | Formatted Diff | Diff
proposed fix (79.10 KB, patch)
2008-04-28 08:32 PST, Alexey Proskuryakov
darin: review-
Review Patch | Details | Formatted Diff | Diff
updated fix (78.30 KB, patch)
2008-04-28 10:00 PST, Alexey Proskuryakov
darin: review+
Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2008-04-21 10:31:47 PST
After downloading Sunday's Nightly build i had no issues at home. but when coming to work where we have a proxy server, i get a "you are not connected to the internet" message when browsing. 

Safari 3.1.1 and older nightly builds as well as other browsers and apps all function as expected.
------- Comment #1 From 2008-04-21 16:33:24 PST -------
I can confirm on Mac OS X 10.4.12.  Regression testing puts the change between r32284 and r32341.

Error message on Mac OS X 10.4.12 is the following, regardless of whether the server is being accessed through the proxy:

Safari can’t connect to the server.
Safari can’t open the page “http://nightly.webkit.org/start/trunk/32341” because it could not connect to the server “nightly.webkit.org”.
------- Comment #2 From 2008-04-22 12:44:44 PST -------
Are you using a PAC file to configure proxy support?
------- Comment #3 From 2008-04-22 13:04:19 PST -------
we have both PAC file and a traditional HTTP proxy and  it looks like it is only happening when i use a pac file.

i personally have the pac file saved on my local machine, but most users have a URL pointing to it.
------- Comment #4 From 2008-04-22 13:08:08 PST -------
Would you be able to upload the PAC file here for testing (if it doesn't contain any sensitive information)?

There were some recent changes that could easily affect PAC file handling, but those were all before r32284. Could you please double-check if that revision works for you? Also, could you try manually configuring proxy settings to make sure that it is PAC file handling that is broken?
------- Comment #5 From 2008-04-23 04:30:41 PST -------
I do see some weirdness with nightly builds and PAC files, and will try to fix what I see - but it would still help to get answers to the above questions.
------- Comment #6 From 2008-04-24 18:15:31 PST -------
i'll be able to look into this more next week from the office. sorry for the delay
------- Comment #7 From 2008-04-24 23:22:03 PST -------
After some testing I found WebKit 31916 the last working version, 31981 contains the bug. I scanned all ChangeSets between 31917 and 31981, but found nothing obvious. 

I don't get the same error message as Kenny, however. In my case Gmail doesn't load: The "Loading..." doesn't go away and progress stops. This doesn't happen all the time. When it works WebKit is much slower than normal, though.

@Alexey: I will send my PAC file by direct email, I don't want it to be publicly available.
------- Comment #8 From 2008-04-25 00:03:07 PST -------
Created an attachment (id=20814) [details]
Fix - work in progress

This should fix the problem, but causes a performance regression on SunSpider, so it needs more work.
------- Comment #9 From 2008-04-28 08:32:10 PST -------
Created an attachment (id=20866) [details]
proposed fix

** TOTAL **:           3419.0ms +/- 0.1%   3419.7ms +/- 0.2% 


  3d:                  *1.005x as slow*
    cube:              -               
    morph:             *1.003x as slow*
    raytrace:          *1.013x as slow*

  access:              1.006x as fast  
    binary-trees:      ??              
    fannkuch:          1.009x as fast  
    nbody:             1.006x as fast  
    nsieve:            ??              

  bitops:              *1.005x as slow*
    3bit-bits-in-byte: *1.026x as slow*
    bits-in-byte:      -               
    bitwise-and:       *1.004x as slow*
    nsieve-bits:       -               

  controlflow:         -               
    recursive:         -               

  crypto:              ??              
    aes:               *1.008x as slow*
    md5:               -               
    sha1:              -               

  date:                ??              
    format-tofte:      ??              
    format-xparb:      -               

  math:                -               
    cordic:            -               
    partial-sums:      -               
    spectral-norm:     1.004x as fast  

  regexp:              1.004x as fast  
    dna:               1.004x as fast  

  string:              ??              
    base64:            ??              
    fasta:             -               
    tagcloud:          -               
    unpack-code:       *1.009x as slow*
    validate-input:    -               
------- Comment #10 From 2008-04-28 09:15:23 PST -------
(From update of attachment 20866 [details])
 192         ExecState(JSGlobalObject*, JSObject* thisObject, JSObject* globalThisValue, FunctionBodyNode*, ExecState* callingExecState, FunctionImp*, const List& args) __attribute__ ((__always_inline__));

This needs to use the ALWAYS_INLINE macro so it will compile in non-GCC compilers.

 66 extern HashTable arrayTable;
 67 extern HashTable dateTable;
 68 extern HashTable mathTable;
 69 extern HashTable numberTable;
 70 extern HashTable RegExpImpTable;
 71 extern HashTable RegExpObjectImpTable;
 72 extern HashTable stringTable;

Since these are global identifiers with external linkage, I would prefer that they had more-specific suffixes on the names -- "Table" doesn't say it all to me. But I guess these names aren't new.

6465     class SyntaxErrorPrototype;
 66     struct ThreadClassInfoHashTables;
6567     class TypeError;

We normally put the struct declarations in a separate paragraph below the class ones.

 72         if (classPropHashTableGetterFunction)
 73             return classPropHashTableGetterFunction(exec);
 74         else
 75             return staticPropHashTable;

We normally omit else before return.

 134         usleep(1000);

Is this portable to all platforms where we use pthreads?

r=me otherwise

review- because of the ALWAYS_INLINE issue above.
------- Comment #11 From 2008-04-28 10:00:21 PST -------
Created an attachment (id=20869) [details]
updated fix

(In reply to comment #10)
> This needs to use the ALWAYS_INLINE macro so it will compile in non-GCC
> compilers.

Oops - this wasn't even supposed to be in the patch, was just an experiment.

> We normally put the struct declarations in a separate paragraph below the class
> ones.

Fixed.

> We normally omit else before return.

Fixed.

>  134         usleep(1000);

This file is currently only used on Mac.
------- Comment #12 From 2008-04-28 10:12:43 PST -------
(From update of attachment 20869 [details])
r=me
------- Comment #13 From 2008-04-28 11:25:58 PST -------
Committed revision 32652.
------- Comment #14 From 2008-04-28 23:59:57 PST -------
I tested nightly build #32652 with the proxy PAC file that let previous WebKit fail: It now works. 
Thanks for your work, Alexey!