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

Issue 177123004: Use StaticAtomicSequenceNumber instead of subtle::Atomic32 in IPCMessage. (Closed)

Created:
4 years, 2 months ago by tulloch
Modified:
4 years, 2 months ago
CC:
chromium-reviews, darin-cc_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Use StaticAtomicSequenceNumber instead of subtle::Atomic32 in IPCMessage. BUG= Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=253169

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -3 lines) Patch
M AUTHORS View 1 chunk +1 line, -0 lines 0 comments Download
M ipc/ipc_message.cc View 3 chunks +3 lines, -3 lines 1 comment Download

Messages

Total messages: 18 (0 generated)
tulloch
Hi, I thought this was a slight improvement, making the intended use of g_ref_num clearer.
4 years, 2 months ago (2014-02-24 19:23:45 UTC) #1
tulloch
https://codereview.chromium.org/177123004/diff/1/ipc/ipc_message.cc File ipc/ipc_message.cc (right): https://codereview.chromium.org/177123004/diff/1/ipc/ipc_message.cc#newcode25 ipc/ipc_message.cc:25: int32 count = g_ref_num.GetNext(); Note that now `g_ref_num` counts ...
4 years, 2 months ago (2014-02-24 19:38:10 UTC) #2
Tom Sepez
This does look cleaner, thanks. I noticed you're adding yourself to authors; have you signed ...
4 years, 2 months ago (2014-02-24 19:39:51 UTC) #3
tulloch
On 2014/02/24 19:39:51, Tom Sepez wrote: > This does look cleaner, thanks. > I noticed ...
4 years, 2 months ago (2014-02-24 19:59:29 UTC) #4
Tom Sepez
LGTM.
4 years, 2 months ago (2014-02-24 20:17:12 UTC) #5
Tom Sepez
The CQ bit was checked by tsepez@chromium.org
4 years, 2 months ago (2014-02-24 20:17:14 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/andrew@tullo.ch/177123004/1
4 years, 2 months ago (2014-02-24 20:36:43 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/andrew@tullo.ch/177123004/1
4 years, 2 months ago (2014-02-24 21:17:29 UTC) #8
cpu_(ooo_6.6-7.5)
I don't see the benefit, and it adds overhead (the - 1) there.
4 years, 2 months ago (2014-02-25 00:26:07 UTC) #9
Tom Sepez
On 2014/02/25 00:26:07, cpu wrote: > I don't see the benefit, and it adds overhead ...
4 years, 2 months ago (2014-02-25 00:27:48 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/andrew@tullo.ch/177123004/1
4 years, 2 months ago (2014-02-25 01:28:04 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/andrew@tullo.ch/177123004/1
4 years, 2 months ago (2014-02-25 07:31:04 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/andrew@tullo.ch/177123004/1
4 years, 2 months ago (2014-02-25 09:08:36 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/andrew@tullo.ch/177123004/1
4 years, 2 months ago (2014-02-25 09:26:10 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/andrew@tullo.ch/177123004/1
4 years, 2 months ago (2014-02-25 13:33:34 UTC) #15
commit-bot: I haz the power
Change committed as 253169
4 years, 2 months ago (2014-02-25 15:30:16 UTC) #16
cpu_(ooo_6.6-7.5)
why was this committed without an owner's lgtm?
4 years, 2 months ago (2014-02-25 21:33:31 UTC) #17
Tom Sepez
4 years, 2 months ago (2014-02-25 23:19:48 UTC) #18
Message was sent while issue was closed.
On 2014/02/25 21:33:31, cpu wrote:
> why was this committed without an owner's lgtm?
It wasn't.
$ grep tsepez ipc/OWNERS 
tsepez@chromium.org

Powered by Google App Engine
This is Rietveld 408576698