Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2459793002:
Let RTC_[D]CHECK_op accept arguments of different signedness (Closed)
|
Created:
4 years, 1 month ago by kwiberg-webrtc Modified:
4 years, 1 month ago CC:
webrtc-reviews_webrtc.org, yujie_mao (webrtc), hlundin-webrtc, tlegrand-webrtc, tterriberry_mozilla.com, audio-team_agora.io, qiang.lu, niklas.enbom, peah-webrtc, minyue-webrtc Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionLet RTC_[D]CHECK_op accept arguments of different signedness
With this change, instead of
RTC_DCHECK_GE(unsigned_var, 17u);
we can simply write
RTC_DCHECK_GE(unsigned_var, 17);
or even
RTC_DCHECK_GE(unsigned_var, -17); // Always true.
and the mathematically sensible thing will happen.
Perhaps more importantly, we can replace checks like
// index is size_t, num_channels is int.
RTC_DCHECK(num_channels >= 0
&& index < static_cast<size_t>(num_channels));
or, even worse, just
// Surely num_channels isn't negative. That would be absurd!
RTC_DCHECK_LT(index, static_cast<size_t>(num_channels));
with simply
RTC_DCHECK_LT(index, num_channels);
In short, you no longer have to keep track of the signedness of the arguments, because the sensible thing will happen.
BUG=webrtc:6645
Committed: https://crrev.com/8a44e1d87b2f80db21eda24d0c0c5742c2060827
Cr-Commit-Position: refs/heads/master@{#14878}
Patch Set 1 #
Total comments: 15
Patch Set 2 : by value #
Messages
Total messages: 27 (13 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Patchset #1 (id:60001) has been deleted
kwiberg@webrtc.org changed reviewers: + ossu@webrtc.org
Weekend hacking project. Good idea? https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.h File webrtc/base/safe_compare.h (right): https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:136: static constexpr bool Op(T1&& a, T2&& b) { \ Should I perhaps take a and b by value instead?
Description was changed from ========== Let RTC_[D]CHECK_op accept arguments of different signedness With this change, instead of RTC_DCHECK_GE(unsigned_var, 17u); you can simply write RTC_DCHECK_GE(unsigned_var, 17); or even RTC_DCHECK_GE(unsigned_var, -17); // Always true. and the mathematically sensible thing will happen. BUG=webrtc:6620 ========== to ========== Let RTC_[D]CHECK_op accept arguments of different signedness With this change, instead of RTC_DCHECK_GE(unsigned_var, 17u); you can simply write RTC_DCHECK_GE(unsigned_var, 17); or even RTC_DCHECK_GE(unsigned_var, -17); // Always true. and the mathematically sensible thing will happen. ==========
It's a pretty sweet solution to the problem, but I'm not sure if it's worth it considering one could just put that little 'u' on the constant and be done with it. :) I think the main case we'd want covered (if any) is when using constants. Could this possibly hide some mistake when used with two variables rather than a variable and a constant? https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.h File webrtc/base/safe_compare.h (right): https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:28: // zero; in the remaining cases, it is just a few machine instructions (no I bet the ?: operator could turn into a branch on one platform or other, at some level of (non-)optimization. https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:136: static constexpr bool Op(T1&& a, T2&& b) { \ On 2016/10/31 09:16:01, kwiberg-webrtc wrote: > Should I perhaps take a and b by value instead? I'd say const&. That's what comparison operators usually work on. https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:150: #define RTC_SAFECMP_MAKE_FUN(name) \ I like fun! Make more fun! :D
https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.h File webrtc/base/safe_compare.h (right): https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:28: // zero; in the remaining cases, it is just a few machine instructions (no On 2016/10/31 16:44:57, ossu wrote: > I bet the ?: operator could turn into a branch on one platform or other, at some > level of (non-)optimization. Probably. I didn't even try to look at unoptimized assembly, because it's always so hard to read due to the redundant work being done all over the place. I'd say it's a safe bet that it'll work as advertised when compiling with optimizations, though. Compilers are good at replacing conditionals with bit twiddling (probably because it's a big gain), and that's exactly what we're asking for here. https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:136: static constexpr bool Op(T1&& a, T2&& b) { \ On 2016/10/31 16:44:57, ossu wrote: > On 2016/10/31 09:16:01, kwiberg-webrtc wrote: > > Should I perhaps take a and b by value instead? > > I'd say const&. That's what comparison operators usually work on. Yes, but this is specifically for built-in integer types. We usually pass those by value. (And as you can see, I consistently do so everywhere else.) https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:150: #define RTC_SAFECMP_MAKE_FUN(name) \ On 2016/10/31 16:44:57, ossu wrote: > I like fun! Make more fun! :D Acknowledged.
On 2016/10/31 16:44:57, ossu wrote: > It's a pretty sweet solution to the problem, but I'm not sure if it's worth it > considering one could just put that little 'u' on the constant and be done with > it. :) But it's *so irritating*...! > I think the main case we'd want covered (if any) is when using constants. Well, yes, for CHECKs that's probably the most common case. But comparing two variables is a common case too—consider e.g. DCHECK_LT(index, foo.size()). (Which, incidentally, is a pain to write safely if one is int and the other size_t, as happens sometimes. This is a better case for demonstrating the usefulness of the present CL than comparison with a literal.) > Could this possibly hide some mistake when used with two variables rather > than a variable and a constant? No, I don't think so. We're currently using a quite paranoid set of warnings, so the compiler will catch all cases of signed-unsigned comparisons and fore you to add u's. With this CL, signed-unsigned comparisons will just work (and the optimizer sees to it that you don't even have to pay for it at runtime).
On 2016/10/31 21:46:23, kwiberg-webrtc wrote: > Well, yes, for CHECKs that's probably the most common case. But comparing two > variables is a common case too---consider e.g. DCHECK_LT(index, foo.size()). > (Which, incidentally, is a pain to write safely if one is int and the other > size_t, as happens sometimes. This is a better case for demonstrating the > usefulness of the present CL than comparison with a literal.) "git grep -nH -e 'CHECK.*static_cast'" matches 56 lines in the WebRTC tree; I looked at a couple of dozen of them, and almost all of them could be simplified if this CL were to land. "git grep -nH -e 'CHECK.*[0-9][uU]\b'" matches 170 lines in the WebRTC tree, and for pretty much all of them we could simply remove the "u" if this CL were to land.
Now that you mention it: the non-constant version of these comparisons is actually _much_ more annoying to have to deal with than the constant one (due to casts and such). So, I'm beginning to like this more and more! https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.h File webrtc/base/safe_compare.h (right): https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:28: // zero; in the remaining cases, it is just a few machine instructions (no On 2016/10/31 21:32:26, kwiberg-webrtc wrote: > On 2016/10/31 16:44:57, ossu wrote: > > I bet the ?: operator could turn into a branch on one platform or other, at > some > > level of (non-)optimization. > > Probably. I didn't even try to look at unoptimized assembly, because it's always > so hard to read due to the redundant work being done all over the place. > > I'd say it's a safe bet that it'll work as advertised when compiling with > optimizations, though. Compilers are good at replacing conditionals with bit > twiddling (probably because it's a big gain), and that's exactly what we're > asking for here. Not sure if it matters, but perhaps turning the ?: into && and || would ensure it never branches, i.e.: (a < 0 && Op::Op(-1, 0)) || Op::Op(safe_cmp_impl::MakeUnsigned(a), b); https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:130: return b < 0 ? Op::Op(0, -1) : Op::Op(a, safe_cmp_impl::MakeUnsigned(b)); Isn't Op::Op(0, -1) a bad choice here if Op is supposed to work only with unsigneds? Or does the Op::Op to the left of the colon resolve to a different function than the Op::Op to the right of it. Wouldn't 1, 0 (and 0, 1 in the Cmp above) be better? https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:136: static constexpr bool Op(T1&& a, T2&& b) { \ On 2016/10/31 21:32:26, kwiberg-webrtc wrote: > On 2016/10/31 16:44:57, ossu wrote: > > On 2016/10/31 09:16:01, kwiberg-webrtc wrote: > > > Should I perhaps take a and b by value instead? > > > > I'd say const&. That's what comparison operators usually work on. > > Yes, but this is specifically for built-in integer types. We usually pass those > by value. (And as you can see, I consistently do so everywhere else.) Ah, right. Yes, pass by value instead!
https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.h File webrtc/base/safe_compare.h (right): https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:28: // zero; in the remaining cases, it is just a few machine instructions (no On 2016/11/01 15:36:57, ossu wrote: > On 2016/10/31 21:32:26, kwiberg-webrtc wrote: > > On 2016/10/31 16:44:57, ossu wrote: > > > I bet the ?: operator could turn into a branch on one platform or other, at > > some > > > level of (non-)optimization. > > > > Probably. I didn't even try to look at unoptimized assembly, because it's > always > > so hard to read due to the redundant work being done all over the place. > > > > I'd say it's a safe bet that it'll work as advertised when compiling with > > optimizations, though. Compilers are good at replacing conditionals with bit > > twiddling (probably because it's a big gain), and that's exactly what we're > > asking for here. > > Not sure if it matters, but perhaps turning the ?: into && and || would ensure > it never branches, i.e.: > (a < 0 && Op::Op(-1, 0)) || Op::Op(safe_cmp_impl::MakeUnsigned(a), b); No, this is the sort of transformation the compiler can do in its sleep. You and me, on the other hand, are sure to slip up and write bugs if we try this... (I'm exaggerating, but not by a lot; this version of the code is harder to read than the ?: version, so bugs have more complexity to hide behind.) https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:130: return b < 0 ? Op::Op(0, -1) : Op::Op(a, safe_cmp_impl::MakeUnsigned(b)); On 2016/11/01 15:36:57, ossu wrote: > Isn't Op::Op(0, -1) a bad choice here if Op is supposed to work only with > unsigneds? Or does the Op::Op to the left of the colon resolve to a different > function than the Op::Op to the right of it. > > Wouldn't 1, 0 (and 0, 1 in the Cmp above) be better? Op::Op<T1, T2> will accept any two types, and the compiler will dutifully complain if we use types that differ in signedness. But -1 and 0 are both int, so we're good. (I chose -1 and 0 because they're the canonical negative and nonnegative integers, so it's easier to see what the code does that way IMO. But 0 and 1 would of course work too.) https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:136: static constexpr bool Op(T1&& a, T2&& b) { \ On 2016/11/01 15:36:57, ossu wrote: > On 2016/10/31 21:32:26, kwiberg-webrtc wrote: > > On 2016/10/31 16:44:57, ossu wrote: > > > On 2016/10/31 09:16:01, kwiberg-webrtc wrote: > > > > Should I perhaps take a and b by value instead? > > > > > > I'd say const&. That's what comparison operators usually work on. > > > > Yes, but this is specifically for built-in integer types. We usually pass > those > > by value. (And as you can see, I consistently do so everywhere else.) > > Ah, right. Yes, pass by value instead! Done.
Alright, looks good to me. Maybe run it by someone else for approval as well. https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.h File webrtc/base/safe_compare.h (right): https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:28: // zero; in the remaining cases, it is just a few machine instructions (no On 2016/11/01 16:14:06, kwiberg-webrtc wrote: > On 2016/11/01 15:36:57, ossu wrote: > > On 2016/10/31 21:32:26, kwiberg-webrtc wrote: > > > On 2016/10/31 16:44:57, ossu wrote: > > > > I bet the ?: operator could turn into a branch on one platform or other, > at > > > some > > > > level of (non-)optimization. > > > > > > Probably. I didn't even try to look at unoptimized assembly, because it's > > always > > > so hard to read due to the redundant work being done all over the place. > > > > > > I'd say it's a safe bet that it'll work as advertised when compiling with > > > optimizations, though. Compilers are good at replacing conditionals with bit > > > twiddling (probably because it's a big gain), and that's exactly what we're > > > asking for here. > > > > Not sure if it matters, but perhaps turning the ?: into && and || would ensure > > it never branches, i.e.: > > (a < 0 && Op::Op(-1, 0)) || Op::Op(safe_cmp_impl::MakeUnsigned(a), b); > > No, this is the sort of transformation the compiler can do in its sleep. You and > me, on the other hand, are sure to slip up and write bugs if we try this... (I'm > exaggerating, but not by a lot; this version of the code is harder to read than > the ?: version, so bugs have more complexity to hide behind.) You think? Yeah, maybe. Granted, if the compiler isn't making any attempts to optimize, the && and || version of the code would still lead to the same problem: it would only allow one of the Op::Ops to get called and I don't think that can be avoided without branches. I'll just tear down this bike shed and let you on your way. https://codereview.webrtc.org/2459793002/diff/80001/webrtc/base/safe_compare.... webrtc/base/safe_compare.h:130: return b < 0 ? Op::Op(0, -1) : Op::Op(a, safe_cmp_impl::MakeUnsigned(b)); On 2016/11/01 16:14:06, kwiberg-webrtc wrote: > On 2016/11/01 15:36:57, ossu wrote: > > Isn't Op::Op(0, -1) a bad choice here if Op is supposed to work only with > > unsigneds? Or does the Op::Op to the left of the colon resolve to a different > > function than the Op::Op to the right of it. > > > > Wouldn't 1, 0 (and 0, 1 in the Cmp above) be better? > > Op::Op<T1, T2> will accept any two types, and the compiler will dutifully > complain if we use types that differ in signedness. But -1 and 0 are both int, > so we're good. (I chose -1 and 0 because they're the canonical negative and > nonnegative integers, so it's easier to see what the code does that way IMO. But > 0 and 1 would of course work too.) Ah, right, yes. I somehow expected them to be typed based on T1 or something... not sure why. Proceed.
Oh, and that's lgtm in computer-speak. Looked through the individual tests as well. Now my brain hurts.
Description was changed from ========== Let RTC_[D]CHECK_op accept arguments of different signedness With this change, instead of RTC_DCHECK_GE(unsigned_var, 17u); you can simply write RTC_DCHECK_GE(unsigned_var, 17); or even RTC_DCHECK_GE(unsigned_var, -17); // Always true. and the mathematically sensible thing will happen. ========== to ========== Let RTC_[D]CHECK_op accept arguments of different signedness With this change, instead of RTC_DCHECK_GE(unsigned_var, 17u); we can simply write RTC_DCHECK_GE(unsigned_var, 17); or even RTC_DCHECK_GE(unsigned_var, -17); // Always true. and the mathematically sensible thing will happen. Perhaps more importantly, we can replace checks like // index is size_t, SIZE is #defined to some constant (probably int). RTC_DCHECK(SIZE >= 0 && index < static_cast<size_t>(SIZE)); or, even worse, just // Hopefully, SIZE isn't some expression that's negative by mistake. RTC_DCHECK_LT(index, static_cast<size_t>(SIZE)); with simply RTC_DCHECK_LT(index, SIZE); ==========
Description was changed from
==========
Let RTC_[D]CHECK_op accept arguments of different signedness
With this change, instead of
RTC_DCHECK_GE(unsigned_var, 17u);
we can simply write
RTC_DCHECK_GE(unsigned_var, 17);
or even
RTC_DCHECK_GE(unsigned_var, -17); // Always true.
and the mathematically sensible thing will happen.
Perhaps more importantly, we can replace checks like
// index is size_t, SIZE is #defined to some constant (probably int).
RTC_DCHECK(SIZE >= 0 && index < static_cast<size_t>(SIZE));
or, even worse, just
// Hopefully, SIZE isn't some expression that's negative by mistake.
RTC_DCHECK_LT(index, static_cast<size_t>(SIZE));
with simply
RTC_DCHECK_LT(index, SIZE);
==========
to
==========
Let RTC_[D]CHECK_op accept arguments of different signedness
With this change, instead of
RTC_DCHECK_GE(unsigned_var, 17u);
we can simply write
RTC_DCHECK_GE(unsigned_var, 17);
or even
RTC_DCHECK_GE(unsigned_var, -17); // Always true.
and the mathematically sensible thing will happen.
Perhaps more importantly, we can replace checks like
// index is size_t, num_channels is int.
RTC_DCHECK(num_channels >= 0
&& index < static_cast<size_t>(num_channels));
or, even worse, just
// Surely num_channels isn't negative. That would be absurd!
RTC_DCHECK_LT(index, static_cast<size_t>(num_channels));
with simply
RTC_DCHECK_LT(index, num_channels);
In short, you no longer have to keep track of the signedness of the arguments,
because the sensible thing will happen.
==========
kwiberg@webrtc.org changed reviewers: + tommi@webrtc.org
Adding an additional reviewer. In addition to the commit message, the
tl;dr is
* "git grep -nH -e 'CHECK.*static_cast'" matches 56 lines in the
WebRTC tree; I looked at a couple of dozen of them, and almost all
of them could be simplified if this CL were to land.
* "git grep -nH -e 'CHECK.*[0-9][uU]\b'" matches 170 lines in the
WebRTC tree, and for pretty much all of them we could simply
remove the "u" if this CL were to land.
The run-time cost of this is either zero (in cases where we manually
checked everything we needed to check), or a few non-branch
instructions (in any cases where we currently fail to check
everything).
lgtm. nice!
Description was changed from
==========
Let RTC_[D]CHECK_op accept arguments of different signedness
With this change, instead of
RTC_DCHECK_GE(unsigned_var, 17u);
we can simply write
RTC_DCHECK_GE(unsigned_var, 17);
or even
RTC_DCHECK_GE(unsigned_var, -17); // Always true.
and the mathematically sensible thing will happen.
Perhaps more importantly, we can replace checks like
// index is size_t, num_channels is int.
RTC_DCHECK(num_channels >= 0
&& index < static_cast<size_t>(num_channels));
or, even worse, just
// Surely num_channels isn't negative. That would be absurd!
RTC_DCHECK_LT(index, static_cast<size_t>(num_channels));
with simply
RTC_DCHECK_LT(index, num_channels);
In short, you no longer have to keep track of the signedness of the arguments,
because the sensible thing will happen.
==========
to
==========
Let RTC_[D]CHECK_op accept arguments of different signedness
With this change, instead of
RTC_DCHECK_GE(unsigned_var, 17u);
we can simply write
RTC_DCHECK_GE(unsigned_var, 17);
or even
RTC_DCHECK_GE(unsigned_var, -17); // Always true.
and the mathematically sensible thing will happen.
Perhaps more importantly, we can replace checks like
// index is size_t, num_channels is int.
RTC_DCHECK(num_channels >= 0
&& index < static_cast<size_t>(num_channels));
or, even worse, just
// Surely num_channels isn't negative. That would be absurd!
RTC_DCHECK_LT(index, static_cast<size_t>(num_channels));
with simply
RTC_DCHECK_LT(index, num_channels);
In short, you no longer have to keep track of the signedness of the arguments,
because the sensible thing will happen.
BUG=webrtc:6645
==========
The CQ bit was checked by kwiberg@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from
==========
Let RTC_[D]CHECK_op accept arguments of different signedness
With this change, instead of
RTC_DCHECK_GE(unsigned_var, 17u);
we can simply write
RTC_DCHECK_GE(unsigned_var, 17);
or even
RTC_DCHECK_GE(unsigned_var, -17); // Always true.
and the mathematically sensible thing will happen.
Perhaps more importantly, we can replace checks like
// index is size_t, num_channels is int.
RTC_DCHECK(num_channels >= 0
&& index < static_cast<size_t>(num_channels));
or, even worse, just
// Surely num_channels isn't negative. That would be absurd!
RTC_DCHECK_LT(index, static_cast<size_t>(num_channels));
with simply
RTC_DCHECK_LT(index, num_channels);
In short, you no longer have to keep track of the signedness of the arguments,
because the sensible thing will happen.
BUG=webrtc:6645
==========
to
==========
Let RTC_[D]CHECK_op accept arguments of different signedness
With this change, instead of
RTC_DCHECK_GE(unsigned_var, 17u);
we can simply write
RTC_DCHECK_GE(unsigned_var, 17);
or even
RTC_DCHECK_GE(unsigned_var, -17); // Always true.
and the mathematically sensible thing will happen.
Perhaps more importantly, we can replace checks like
// index is size_t, num_channels is int.
RTC_DCHECK(num_channels >= 0
&& index < static_cast<size_t>(num_channels));
or, even worse, just
// Surely num_channels isn't negative. That would be absurd!
RTC_DCHECK_LT(index, static_cast<size_t>(num_channels));
with simply
RTC_DCHECK_LT(index, num_channels);
In short, you no longer have to keep track of the signedness of the arguments,
because the sensible thing will happen.
BUG=webrtc:6645
==========
Message was sent while issue was closed.
Committed patchset #2 (id:100001)
Message was sent while issue was closed.
Description was changed from
==========
Let RTC_[D]CHECK_op accept arguments of different signedness
With this change, instead of
RTC_DCHECK_GE(unsigned_var, 17u);
we can simply write
RTC_DCHECK_GE(unsigned_var, 17);
or even
RTC_DCHECK_GE(unsigned_var, -17); // Always true.
and the mathematically sensible thing will happen.
Perhaps more importantly, we can replace checks like
// index is size_t, num_channels is int.
RTC_DCHECK(num_channels >= 0
&& index < static_cast<size_t>(num_channels));
or, even worse, just
// Surely num_channels isn't negative. That would be absurd!
RTC_DCHECK_LT(index, static_cast<size_t>(num_channels));
with simply
RTC_DCHECK_LT(index, num_channels);
In short, you no longer have to keep track of the signedness of the arguments,
because the sensible thing will happen.
BUG=webrtc:6645
==========
to
==========
Let RTC_[D]CHECK_op accept arguments of different signedness
With this change, instead of
RTC_DCHECK_GE(unsigned_var, 17u);
we can simply write
RTC_DCHECK_GE(unsigned_var, 17);
or even
RTC_DCHECK_GE(unsigned_var, -17); // Always true.
and the mathematically sensible thing will happen.
Perhaps more importantly, we can replace checks like
// index is size_t, num_channels is int.
RTC_DCHECK(num_channels >= 0
&& index < static_cast<size_t>(num_channels));
or, even worse, just
// Surely num_channels isn't negative. That would be absurd!
RTC_DCHECK_LT(index, static_cast<size_t>(num_channels));
with simply
RTC_DCHECK_LT(index, num_channels);
In short, you no longer have to keep track of the signedness of the arguments,
because the sensible thing will happen.
BUG=webrtc:6645
Committed: https://crrev.com/8a44e1d87b2f80db21eda24d0c0c5742c2060827
Cr-Commit-Position: refs/heads/master@{#14878}
==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/8a44e1d87b2f80db21eda24d0c0c5742c2060827 Cr-Commit-Position: refs/heads/master@{#14878} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
