Add a command line flag --v8-proxy-resolver, to select the new PAC implementa...

Add a command line flag --v8-proxy-resolver, to select the new PAC implementation.

When running in single process mode, this flag has no effect (since we can't run side by side with the renderer's V8).

In regular mode, the v8 resolver is currently running in the browser process.
This means it has to share with the v8 debugger shell.
Added locking around the debugger shell so they can peacefully co-exist.

When this flag is enabled, PAC scripts are downloaded through the browser.

BUG=74, 2764
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=10827

[eroman, 2009-03-03 00:39:04]

erikkay: please you review debugger_shell.cc.
wtc: please review everything.
darin: just an FYI, since you follow the net changes.

[darin (slow to review), 2009-03-03 01:04:54]

http://codereview.chromium.org/27365/diff/19/25
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/27365/diff/19/25#newcode241
Line 241: const wchar_t kV8ProxyResolver[]                   =
L"v8-proxy-resolver";
nit: looks like there is an intent to line up the values at the "=" char.

[eroman, 2009-03-03 01:16:21]

done.

[wtc, 2009-03-03 01:31:21]

LGTM.

I only did a superficial review of the V8 locking code in
debugger_shell.cc.

http://codereview.chromium.org/27365/diff/19/25
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/27365/diff/19/25#newcode239
Line 239: // scripts internally and executes them using V8. This cannot be used
in
Nit: you may want to note that "internally" is as opposed to
using WinHTTP.

http://codereview.chromium.org/27365/diff/19/20
File net/proxy/proxy_service.cc (right):

http://codereview.chromium.org/27365/diff/19/20#newcode217
Line 217: // TODO(eroman): Consolidate with ProxyService::Create().
I think it's fine to have duplicate code here.

http://codereview.chromium.org/27365/diff/19/21
File net/proxy/proxy_service.h (right):

http://codereview.chromium.org/27365/diff/19/21#newcode96
Line 96: // same as Create(ProxyInfo*), however under the hood it uses a
different
Nit: should you add 'const' to the parameter of Create()?

[eroman, 2009-03-03 01:40:52]

http://codereview.chromium.org/27365/diff/19/25
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/27365/diff/19/25#newcode239
Line 239: // scripts internally and executes them using V8. This cannot be used
in
On 2009/03/03 01:31:21, wtc wrote:
> Nit: you may want to note that "internally" is as opposed to
> using WinHTTP.

Done. New block shown below:

// Enable alternate proxy autoconfig implementation, which downloads PAC
// scripts internally and executes them using V8 (as opposed to fetching
// and executing using WinHTTP). This cannot be used in conjunction with
// --single-process.

(The winhttp comment obviously doesn't apply to mac).

http://codereview.chromium.org/27365/diff/19/20
File net/proxy/proxy_service.cc (right):

http://codereview.chromium.org/27365/diff/19/20#newcode217
Line 217: // TODO(eroman): Consolidate with ProxyService::Create().
On 2009/03/03 01:31:21, wtc wrote:
> I think it's fine to have duplicate code here.

Done. Removed the comment.

http://codereview.chromium.org/27365/diff/19/21
File net/proxy/proxy_service.h (right):

http://codereview.chromium.org/27365/diff/19/21#newcode96
Line 96: // same as Create(ProxyInfo*), however under the hood it uses a
different
On 2009/03/03 01:31:21, wtc wrote:
> Nit: should you add 'const' to the parameter of Create()?

Done.

[Erik does not do reviews, 2009-03-03 17:20:09]

LGTM - I think that you can address my comment in a later CL.

http://codereview.chromium.org/27365/diff/1019/1022
File chrome/browser/debugger/debugger_shell.cc (right):

http://codereview.chromium.org/27365/diff/1019/1022#newcode198
Line 198: void DebuggerShell::MessageListener(v8::Handle<v8::Message> message) {
Since there can only be one message listener, I assume we need to do something
different here.