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

Issue 19617: Add thread checks to the IPC channel.... (Closed)

Created:
10 years, 6 months ago by rvargas (doing something else)
Modified:
8 years, 2 months ago
Reviewers:
jam
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Add thread checks to the IPC channel. Almost every method of the IPC channel must be invoked from a single thread. This adds the corresponding DCHECKS, and should confirm bug 6489 as a cause of flakiness on the UI tests. BUG: 6489. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=8837

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -2 lines) Patch
M base/non_thread_safe.h View 2 chunks +0 lines, -2 lines 0 comments Download
M chrome/common/ipc_channel_win.h View 2 chunks +4 lines, -0 lines 1 comment Download
M chrome/common/ipc_channel_win.cc View 8 chunks +13 lines, -0 lines 1 comment Download

Messages

Total messages: 4 (0 generated)
rvargas (doing something else)
10 years, 6 months ago (2009-01-28 02:00:39 UTC) #1
jam
http://codereview.chromium.org/19617/diff/1/3 File chrome/common/ipc_channel_win.cc (right): http://codereview.chromium.org/19617/diff/1/3#newcode176 Line 176: if (!thread_check_.get()) since this is only used in ...
10 years, 6 months ago (2009-01-28 03:33:58 UTC) #2
rvargas (doing something else)
On 2009/01/28 03:33:58, John Abd-El-Malek wrote: > http://codereview.chromium.org/19617/diff/1/3 > File chrome/common/ipc_channel_win.cc (right): > > http://codereview.chromium.org/19617/diff/1/3#newcode176 ...
10 years, 6 months ago (2009-01-28 03:49:57 UTC) #3
jam
10 years, 6 months ago (2009-01-28 18:09:45 UTC) #4
On 2009/01/28 03:49:57, rvargas wrote:
> On 2009/01/28 03:33:58, John Abd-El-Malek wrote:
> > http://codereview.chromium.org/19617/diff/1/3
> > File chrome/common/ipc_channel_win.cc (right):
> > 
> > http://codereview.chromium.org/19617/diff/1/3#newcode176
> > Line 176: if (!thread_check_.get())
> > since this is only used in debug builds, it should probably be surrounded by
> an
> > ifdef
> > 
> > http://codereview.chromium.org/19617/diff/1/4
> > File chrome/common/ipc_channel_win.h (right):
> > 
> > http://codereview.chromium.org/19617/diff/1/4#newcode78
> > Line 78: scoped_ptr<NonThreadSafe> thread_check_;
> > surround by ifdef since only used in debug
> 
> NonThreadSafe is already defined to empty methods for release builds so the
> actual cost here is minimum. Do we really want to have more #ifdefs on the
code
> for this?

ah, I didn't know that.  lgtm

Powered by Google App Engine
This is Rietveld 408576698