Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(41)

Issue 27365: Add a command line flag --v8-proxy-resolver, to select the new PAC implementa... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 6 months ago by eroman
Modified:
6 years, 3 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

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

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 7

Patch Set 3 : '' #

Patch Set 4 : Address wtc's comments #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+93 lines, -9 lines) Patch
M chrome/browser/debugger/debugger_shell.cc View 10 chunks +11 lines, -0 lines 1 comment Download
M chrome/browser/net/chrome_url_request_context.cc View 3 chunks +24 lines, -6 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 1 chunk +6 lines, -0 lines 0 comments Download
M net/proxy/proxy_service.h View 1 2 3 2 chunks +18 lines, -3 lines 0 comments Download
M net/proxy/proxy_service.cc View 1 2 3 2 chunks +33 lines, -0 lines 0 comments Download
Trybot results:
Commit queue not available (can’t edit this change).

Messages

Total messages: 6 (0 generated)
eroman
erikkay: please you review debugger_shell.cc. wtc: please review everything. darin: just an FYI, since you ...
8 years, 6 months ago (2009-03-03 00:39:04 UTC) #1
darin (slow to review)
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 ...
8 years, 6 months ago (2009-03-03 01:04:54 UTC) #2
eroman
done.
8 years, 6 months ago (2009-03-03 01:16:21 UTC) #3
wtc
LGTM. I only did a superficial review of the V8 locking code in debugger_shell.cc. http://codereview.chromium.org/27365/diff/19/25 ...
8 years, 6 months ago (2009-03-03 01:31:21 UTC) #4
eroman
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. ...
8 years, 6 months ago (2009-03-03 01:40:52 UTC) #5
Erik does not do reviews
8 years, 6 months ago (2009-03-03 17:20:09 UTC) #6
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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b