Issue 391062: Move NULLing of OCSP context from UI thread to IO thread, to avoid possible r...

Issue 391062: Move NULLing of OCSP context from UI thread to IO thread, to avoid possible r... (Closed)

Created:
11 years, 1 month ago by eroman

Modified:
9 years, 6 months ago

Reviewers:
ukai, wtc

CC:
chromium-reviews_googlegroups.com, darin (slow to review), ben+cc_chromium.org

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Move NULLing of OCSP context from UI thread to IO thread, to avoid possible races. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=32104

Patch Set 1 #

Patch Set 2 : remove comment #

Patch Set 3 : Add missing files to CL #

Patch Set 4 : actually have it compile #

Total comments: 2

Patch Set 5 : remove curly braces on one line if #

Patch Set 6 : sync #

Created: 11 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+17 lines, -12 lines)			Patch
M	chrome/browser/net/chrome_url_request_context.cc	View	1 2 3 4 5	1 chunk	+7 lines, -0 lines	0 comments	Download
M	chrome/browser/profile.cc	View	1 2 3 4 5	2 chunks	+1 line, -8 lines	0 comments	Download
M	net/ocsp/nss_ocsp.h	View	3 4 5	1 chunk	+1 line, -0 lines	0 comments	Download
M	net/ocsp/nss_ocsp.cc	View	3 4 5	3 chunks	+8 lines, -4 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

eroman

Actually this isn't right.. please hold off on reviewing this.

11 years, 1 month ago (2009-11-14 00:32:51 UTC) #2

wtc

11 years, 1 month ago (2009-11-16 19:49:00 UTC) #6

LGTM.

http://codereview.chromium.org/391062/diff/3007/2010
File chrome/browser/net/chrome_url_request_context.cc (right):

http://codereview.chromium.org/391062/diff/3007/2010#newcode200
Line 200: // TODO(ukai): find a better way to set the URLRequestContext for
OCSP.
eroman: This is the TODO comment that I just told you about.
When an HTTPS URLRequest verifies the server certificate at
the end of SSL connection setup, it creates an OCSP
URLRequest.  It seems that the OCSP URLRequest should use the
URLRequestContext of the HTTPS URLRequest.  I remember we
found it hard to implement this, so we ended up using a
single URLRequestContext for all OCSP URLRequests, and
Chrome sets this global URLRequestContext for OCSP when
it creates the "original" URLRequestContext here.

http://codereview.chromium.org/391062/diff/3007/2009
File chrome/browser/profile.cc (right):

http://codereview.chromium.org/391062/diff/3007/2009#newcode778
Line 778: if (default_request_context_ == request_context_) {
Nit: remove the curly braces {}?

Expand Messages | Collapse Messages