Bug 18661 - Proxy server issue in Sunday's Nightly
Summary: Proxy server issue in Sunday's Nightly
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Major
Assignee: Alexey Proskuryakov
Depends on:
Reported: 2008-04-21 10:31 PDT by Kenny Sabarese
Modified: 2008-04-28 23:59 PDT (History)
3 users (show)

See Also:

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

Note You need to log in before you can comment on or make changes to this bug.
Description Kenny Sabarese 2008-04-21 10:31:47 PDT
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 Justin 2008-04-21 16:33:24 PDT
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 Alexey Proskuryakov 2008-04-22 12:44:44 PDT
Are you using a PAC file to configure proxy support?
Comment 3 Kenny Sabarese 2008-04-22 13:04:19 PDT
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 Alexey Proskuryakov 2008-04-22 13:08:08 PDT
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 Alexey Proskuryakov 2008-04-23 04:30:41 PDT
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 Kenny Sabarese 2008-04-24 18:15:31 PDT
i'll be able to look into this more next week from the office. sorry for the delay
Comment 7 Ruben Bakker 2008-04-24 23:22:03 PDT
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 Alexey Proskuryakov 2008-04-25 00:03:07 PDT
Created attachment 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 Alexey Proskuryakov 2008-04-28 08:32:10 PDT
Created attachment 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 Darin Adler 2008-04-28 09:15:23 PDT
Comment on attachment 20866 [details]
proposed fix

 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 Alexey Proskuryakov 2008-04-28 10:00:21 PDT
Created attachment 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.


> We normally omit else before return.


>  134         usleep(1000);

This file is currently only used on Mac.
Comment 12 Darin Adler 2008-04-28 10:12:43 PDT
Comment on attachment 20869 [details]
updated fix

Comment 13 Alexey Proskuryakov 2008-04-28 11:25:58 PDT
Committed revision 32652.

Comment 14 Ruben Bakker 2008-04-28 23:59:57 PDT
I tested nightly build #32652 with the proxy PAC file that let previous WebKit fail: It now works. 
Thanks for your work, Alexey!