Safe numeric library: base/numerics (copied from Chromium)

Safe numeric library added: base/numerics (copied from Chromium)

This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h

A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.

This CL also adds a DEPS file to webrtc/base.

NOPRESUBMIT=True
BUG=webrtc:5548, webrtc:5623
Committed: https://crrev.com/de1c81b2d2196be611674aa6019b9db3a9329042
Cr-Commit-Position: refs/heads/master@{#11907}

[hbos, 2016-03-02 14:01:45]

Patchset #1 (id:1) has been deleted

[hbos, 2016-03-02 14:12:26]

Patchset #2 (id:40001) has been deleted

[hbos, 2016-03-02 15:34:21]

Description was changed from

==========
Safe numeric library: base/numerics (copied from Chromium)

BUG=
==========

to

==========
Safe numeric library added: base/numerics (copied from Chromium)

This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h

A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.

BUG=webrtc:5548
==========

[hbos, 2016-03-02 15:46:50]

hbos@webrtc.org changed reviewers:
+ kjellander@webrtc.org, tommi@webrtc.org

[hbos, 2016-03-02 15:46:51]

Please take a look, kjellander & tommi.

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
File webrtc/base/numerics/BUILD.gn (right):

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
webrtc/base/numerics/BUILD.gn:16: ]
Is this correct (and similar in GYP as a static_library)?

What I'm wondering about is this uses some of the stuff from base
(checks.h/logging.h), so there is a dependency on base. It compiles as a part of
base (is a dependency of it) though so this works fine.
However as a stand-alone target we would have to add a dependency on base, which
would result in a circular dependency base -> safe_numerics -> base.

If this is wrong and a dep on base is needed but not allowed due to circularity,
then I suggest we create a .gypi variable for the sources and reference it from
the base target as to not have to work on breaking this free from base (not have
a stand-alone build target), at least not in this CL.

(Or perhaps I mean base_approved when I say base, same concern...)

[tommi, 2016-03-02 16:13:53]

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
File webrtc/base/numerics/BUILD.gn (right):

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
webrtc/base/numerics/BUILD.gn:16: ]
On 2016/03/02 15:46:51, hbos wrote:
> Is this correct (and similar in GYP as a static_library)?
> 
> What I'm wondering about is this uses some of the stuff from base
> (checks.h/logging.h), so there is a dependency on base. It compiles as a part
of
> base (is a dependency of it) though so this works fine.
> However as a stand-alone target we would have to add a dependency on base,
which
> would result in a circular dependency base -> safe_numerics -> base.
> 
> If this is wrong and a dep on base is needed but not allowed due to
circularity,
> then I suggest we create a .gypi variable for the sources and reference it
from
> the base target as to not have to work on breaking this free from base (not
have
> a stand-alone build target), at least not in this CL.
> 
> (Or perhaps I mean base_approved when I say base, same concern...)

Can we make this depend on base and base not depend on this?

[kjellander_webrtc, 2016-03-02 18:40:05]

to protect against future updates missing to to similar updates, can you add a
DEPS file in webrtc/base with the following contents?

include_rules = [
  "-base",
  "+json",
  "+testing",
  "+third_party/jsoncpp",
  "+webrtc",
  "+webrtc/base",
]

That's the ones I could find. I think you need to add a few more like "config.h"
as well (you'll see).
For more info, see
https://code.google.com/p/chromium/codesearch#chromium/src/buildtools/checkde...
Feel free to reference https://bugs.chromium.org/p/webrtc/issues/detail?id=5623
too, for that.

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
File webrtc/base/numerics/BUILD.gn (right):

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
webrtc/base/numerics/BUILD.gn:16: ]
On 2016/03/02 16:13:53, tommi-webrtc wrote:
> On 2016/03/02 15:46:51, hbos wrote:
> > Is this correct (and similar in GYP as a static_library)?

Please don't add a new BUILD.gn file, add a new target in webrtc/base/BUILD.gn
instead.

> > What I'm wondering about is this uses some of the stuff from base
> > (checks.h/logging.h), so there is a dependency on base. It compiles as a
part
> of
> > base (is a dependency of it) though so this works fine.
> > However as a stand-alone target we would have to add a dependency on base,
> which
> > would result in a circular dependency base -> safe_numerics -> base.
> > 
> > If this is wrong and a dep on base is needed but not allowed due to
> circularity,
> > then I suggest we create a .gypi variable for the sources and reference it
> from
> > the base target as to not have to work on breaking this free from base (not
> have
> > a stand-alone build target), at least not in this CL.
> > 
> > (Or perhaps I mean base_approved when I say base, same concern...)
> 
> Can we make this depend on base and base not depend on this?

We talked about removing the base/logging.h dependency from the source, right? 
Changing it to webrtc/base/logging.h would also work as long as you don't plan
to introduce a dependency from the other base source to base/numerics - or is
that something you plan to do?

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/saf...
File webrtc/base/numerics/safe_numerics.gyp (right):

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_numerics.gyp:12: # GN version:
webrtc/base/numerics/BUILD.gn
Please add this target to webrtc/base/base.gyp instead. Each new .gyp file slows
down GYP processing.

[kwiberg-webrtc, 2016-03-02 20:14:11]

kwiberg@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[kwiberg-webrtc, 2016-03-02 20:14:13]

Great initiative!

Does Chromium have tests for these? If so, we should borrow those as well, since
we're making changes, and might not have the exact same environment.

https://codereview.webrtc.org/1753293002/diff/60001/webrtc/base/numerics/safe...
File webrtc/base/numerics/safe_conversions.h (right):

https://codereview.webrtc.org/1753293002/diff/60001/webrtc/base/numerics/safe...
webrtc/base/numerics/safe_conversions.h:127: "- base::CheckedNumeric");
Forgot these.

https://codereview.webrtc.org/1753293002/diff/60001/webrtc/base/numerics/safe...
webrtc/base/numerics/safe_conversions.h:151: explicit StrictNumeric(const
StrictNumeric<Src>& rhs)
Copy constructors should be implicit. The style guide has a special exception
for them.

https://codereview.webrtc.org/1753293002/diff/60001/webrtc/base/numerics/safe...
webrtc/base/numerics/safe_conversions.h:157: explicit StrictNumeric(Src value)
Why make this explicit? It was made implicit on purpose, and it's clearly
neither dangerous nor surprising.

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
File webrtc/base/numerics/safe_math.h (right):

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_math.h:77: explicit CheckedNumeric(Src value)
Same complaint as earlier about the addition of "explicit".

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_math.h:86: explicit CheckedNumeric(StrictNumeric<Src>
value)
Same complaint as earlier about the addition of "explicit".

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
File webrtc/base/numerics/BUILD.gn (right):

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
webrtc/base/numerics/BUILD.gn:16: ]
On 2016/03/02 16:13:53, tommi-webrtc wrote:
> On 2016/03/02 15:46:51, hbos wrote:
> > Is this correct (and similar in GYP as a static_library)?
> > 
> > What I'm wondering about is this uses some of the stuff from base
> > (checks.h/logging.h), so there is a dependency on base. It compiles as a
part
> of
> > base (is a dependency of it) though so this works fine.
> > However as a stand-alone target we would have to add a dependency on base,
> which
> > would result in a circular dependency base -> safe_numerics -> base.
> > 
> > If this is wrong and a dep on base is needed but not allowed due to
> circularity,
> > then I suggest we create a .gypi variable for the sources and reference it
> from
> > the base target as to not have to work on breaking this free from base (not
> have
> > a stand-alone build target), at least not in this CL.
> > 
> > (Or perhaps I mean base_approved when I say base, same concern...)
> 
> Can we make this depend on base and base not depend on this?

Specifically, can we have base depend on numerics which in turn depends on
base_approved?

[kjellander_webrtc, 2016-03-03 10:25:20]

Chromium has one unit test:
https://code.google.com/p/chromium/codesearch#chromium/src/base/numerics/safe...
but it's depending on a lot of other things in Chromium's src/base, so we can't
add it here since we cannot depend on that in WebRTC code.

As long as we only modify the logging statements and the namespace, we should be
fine with having the code being tested in Chromium, IMO.

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
File webrtc/base/numerics/safe_conversions_impl.h (right):

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_conversions_impl.h:19: #include <limits>
keep limits.h unchanged or is there any benefit from switching? If so, document
why.

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_conversions_impl.h:22: #include
"webrtc/base/logging.h"
No need for webrtc/base/logging.h here.

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
File webrtc/base/numerics/safe_math.h (right):

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_math.h:20: #include <limits>
Is this used for anything?

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
File webrtc/base/numerics/BUILD.gn (right):

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
webrtc/base/numerics/BUILD.gn:16: ]
On 2016/03/02 20:14:12, kwiberg-webrtc wrote:
> On 2016/03/02 16:13:53, tommi-webrtc wrote:
> > On 2016/03/02 15:46:51, hbos wrote:
> > > Is this correct (and similar in GYP as a static_library)?
> > > 
> > > What I'm wondering about is this uses some of the stuff from base
> > > (checks.h/logging.h), so there is a dependency on base. It compiles as a
> part
> > of
> > > base (is a dependency of it) though so this works fine.
> > > However as a stand-alone target we would have to add a dependency on base,
> > which
> > > would result in a circular dependency base -> safe_numerics -> base.
> > > 
> > > If this is wrong and a dep on base is needed but not allowed due to
> > circularity,
> > > then I suggest we create a .gypi variable for the sources and reference it
> > from
> > > the base target as to not have to work on breaking this free from base
(not
> > have
> > > a stand-alone build target), at least not in this CL.
> > > 
> > > (Or perhaps I mean base_approved when I say base, same concern...)
> > 
> > Can we make this depend on base and base not depend on this?
> 
> Specifically, can we have base depend on numerics which in turn depends on
> base_approved?

I guess that would work since it's not a cycle.

[hbos, 2016-03-03 10:30:43]

Description was changed from

==========
Safe numeric library added: base/numerics (copied from Chromium)

This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h

A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.

BUG=webrtc:5548
==========

to

==========
Safe numeric library added: base/numerics (copied from Chromium)

This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h

A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.

This CL also adds a DEPS file to webrtc/base.

BUG=webrtc:5548, webrtc:5623
==========

[hbos, 2016-03-03 13:43:55]

Patchset #7 (id:90031) has been deleted

[hbos, 2016-03-03 14:17:07]

PTAL everyone (tommi, kjellander, kwiberg)

https://codereview.webrtc.org/1753293002/diff/60001/webrtc/base/numerics/safe...
File webrtc/base/numerics/safe_conversions.h (right):

https://codereview.webrtc.org/1753293002/diff/60001/webrtc/base/numerics/safe...
webrtc/base/numerics/safe_conversions.h:127: "- base::CheckedNumeric");
On 2016/03/02 20:14:12, kwiberg-webrtc wrote:
> Forgot these.

Done.

https://codereview.webrtc.org/1753293002/diff/60001/webrtc/base/numerics/safe...
webrtc/base/numerics/safe_conversions.h:151: explicit StrictNumeric(const
StrictNumeric<Src>& rhs)
On 2016/03/02 20:14:12, kwiberg-webrtc wrote:
> Copy constructors should be implicit. The style guide has a special exception
> for them.

Done. Oh you're right, I got presubmit warnings about them and added "explicit"
without much thought, I reverted these changes and uploaded ignoring the
warning. Same for the other explicits.

https://codereview.webrtc.org/1753293002/diff/60001/webrtc/base/numerics/safe...
webrtc/base/numerics/safe_conversions.h:157: explicit StrictNumeric(Src value)
On 2016/03/02 20:14:12, kwiberg-webrtc wrote:
> Why make this explicit? It was made implicit on purpose, and it's clearly
> neither dangerous nor surprising.

Done.

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
File webrtc/base/numerics/safe_conversions_impl.h (right):

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_conversions_impl.h:19: #include <limits>
On 2016/03/03 10:25:19, kjellander (webrtc) wrote:
> keep limits.h unchanged or is there any benefit from switching? If so,
document
> why.

I got an "include what you use" presubmit warning saying I should use <limits>.
I'm guessing <limits> and <limits.h> are the same? Anyway, reverted the change
and ignored the presubmit warning.

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_conversions_impl.h:22: #include
"webrtc/base/logging.h"
On 2016/03/03 10:25:19, kjellander (webrtc) wrote:
> No need for webrtc/base/logging.h here.

Done. Turns out it's actually not needed in any of the files.

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
File webrtc/base/numerics/safe_math.h (right):

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_math.h:20: #include <limits>
On 2016/03/03 10:25:19, kjellander (webrtc) wrote:
> Is this used for anything?

No, nice catch.

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_math.h:77: explicit CheckedNumeric(Src value)
On 2016/03/02 20:14:12, kwiberg-webrtc wrote:
> Same complaint as earlier about the addition of "explicit".

Done.

https://codereview.webrtc.org/1753293002/diff/100001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_math.h:86: explicit CheckedNumeric(StrictNumeric<Src>
value)
On 2016/03/02 20:14:12, kwiberg-webrtc wrote:
> Same complaint as earlier about the addition of "explicit".

Done.

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
File webrtc/base/numerics/BUILD.gn (right):

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
webrtc/base/numerics/BUILD.gn:16: ]
On 2016/03/02 18:40:05, kjellander (webrtc) wrote:
> On 2016/03/02 16:13:53, tommi-webrtc wrote:
> > On 2016/03/02 15:46:51, hbos wrote:
> > > Is this correct (and similar in GYP as a static_library)?
> 
> Please don't add a new BUILD.gn file, add a new target in webrtc/base/BUILD.gn
> instead.
> 
> > > What I'm wondering about is this uses some of the stuff from base
> > > (checks.h/logging.h), so there is a dependency on base. It compiles as a
> part
> > of
> > > base (is a dependency of it) though so this works fine.
> > > However as a stand-alone target we would have to add a dependency on base,
> > which
> > > would result in a circular dependency base -> safe_numerics -> base.
> > > 
> > > If this is wrong and a dep on base is needed but not allowed due to
> > circularity,
> > > then I suggest we create a .gypi variable for the sources and reference it
> > from
> > > the base target as to not have to work on breaking this free from base
(not
> > have
> > > a stand-alone build target), at least not in this CL.
> > > 
> > > (Or perhaps I mean base_approved when I say base, same concern...)
> > 
> > Can we make this depend on base and base not depend on this?
> 
> We talked about removing the base/logging.h dependency from the source, right?

> Changing it to webrtc/base/logging.h would also work as long as you don't plan
> to introduce a dependency from the other base source to base/numerics - or is
> that something you plan to do?

I changed it to webrtc/base/logging.h but it turns out its not event used.

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/BUI...
webrtc/base/numerics/BUILD.gn:16: ]
On 2016/03/03 10:25:20, kjellander (webrtc) wrote:
> On 2016/03/02 20:14:12, kwiberg-webrtc wrote:
> > On 2016/03/02 16:13:53, tommi-webrtc wrote:
> > > On 2016/03/02 15:46:51, hbos wrote:
> > > > Is this correct (and similar in GYP as a static_library)?
> > > > 
> > > > What I'm wondering about is this uses some of the stuff from base
> > > > (checks.h/logging.h), so there is a dependency on base. It compiles as a
> > part
> > > of
> > > > base (is a dependency of it) though so this works fine.
> > > > However as a stand-alone target we would have to add a dependency on
base,
> > > which
> > > > would result in a circular dependency base -> safe_numerics -> base.
> > > > 
> > > > If this is wrong and a dep on base is needed but not allowed due to
> > > circularity,
> > > > then I suggest we create a .gypi variable for the sources and reference
it
> > > from
> > > > the base target as to not have to work on breaking this free from base
> (not
> > > have
> > > > a stand-alone build target), at least not in this CL.
> > > > 
> > > > (Or perhaps I mean base_approved when I say base, same concern...)
> > > 
> > > Can we make this depend on base and base not depend on this?
> > 
> > Specifically, can we have base depend on numerics which in turn depends on
> > base_approved?
> 
> I guess that would work since it's not a cycle.

Oh, nice. Yes that works. Now we have rtc_base -> safe_numerics ->
rtc_base_approved (and I verified that this compiles and can link into an
executable).

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/saf...
File webrtc/base/numerics/safe_numerics.gyp (right):

https://codereview.webrtc.org/1753293002/diff/120001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_numerics.gyp:12: # GN version:
webrtc/base/numerics/BUILD.gn
On 2016/03/02 18:40:05, kjellander (webrtc) wrote:
> Please add this target to webrtc/base/base.gyp instead. Each new .gyp file
slows
> down GYP processing.

Done. (Oh, shame!)

[kwiberg-webrtc, 2016-03-05 00:47:26]

https://codereview.webrtc.org/1753293002/diff/190001/webrtc/base/numerics/saf...
File webrtc/base/numerics/safe_conversions.h (right):

https://codereview.webrtc.org/1753293002/diff/190001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_conversions.h:67: RTC_NOTREACHED();
This was CHECK(false) in the original. Why not RTC_CHECK(false) here?

https://codereview.webrtc.org/1753293002/diff/190001/webrtc/base/numerics/saf...
File webrtc/base/numerics/safe_conversions_impl.h (right):

https://codereview.webrtc.org/1753293002/diff/190001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_conversions_impl.h:21: #include <limits.h>
The original includes these two in the other order, without a blank line in
between.

[hbos, 2016-03-07 09:18:02]

https://codereview.webrtc.org/1753293002/diff/190001/webrtc/base/numerics/saf...
File webrtc/base/numerics/safe_conversions.h (right):

https://codereview.webrtc.org/1753293002/diff/190001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_conversions.h:67: RTC_NOTREACHED();
On 2016/03/05 00:47:26, kwiberg-webrtc wrote:
> This was CHECK(false) in the original. Why not RTC_CHECK(false) here?

I guess I thought it was more elegant but I think there's a reason they choose
CHECK over NOTREACHED and I shouldn't make my own changes to the file,
RTC_CHECK(false) it is.

https://codereview.webrtc.org/1753293002/diff/190001/webrtc/base/numerics/saf...
File webrtc/base/numerics/safe_conversions_impl.h (right):

https://codereview.webrtc.org/1753293002/diff/190001/webrtc/base/numerics/saf...
webrtc/base/numerics/safe_conversions_impl.h:21: #include <limits.h>
On 2016/03/05 00:47:26, kwiberg-webrtc wrote:
> The original includes these two in the other order, without a blank line in
> between.

Done. (I think after the presubmit made me use limits instead of limits.h it
then asked me to change the order, something about c/c++ headers, forgot to
change back after ignoring the first warning.)

[kwiberg-webrtc, 2016-03-07 14:20:40]

lgtm

[hbos, 2016-03-07 15:50:46]

PTAL kjellander and tommi. Though tommi is travelling, not sure he does code
reviews this week.

First time touching a bunch of different files like this. I should TBR all
owners of the files where all I've changed is the #include, right?

[tlegrand-webrtc, 2016-03-07 16:27:16]

tina.legrand@webrtc.org changed reviewers:
+ tina.legrand@webrtc.org

[tlegrand-webrtc, 2016-03-07 16:27:18]

A small drive-by, since I saw that the includes were not always in alphabetic
order.

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/openssladapt...
File webrtc/base/openssladapter.cc (right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/openssladapt...
webrtc/base/openssladapter.cc:41: #include
"webrtc/base/numerics/safe_conversions.h"
This isn't i alphabetic order. It should be placed between logging.h and
openssl.h (since "numerics" starts with "n").

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/opensslstrea...
File webrtc/base/opensslstreamadapter.cc (right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/opensslstrea...
webrtc/base/opensslstreamadapter.cc:37: #include
"webrtc/base/numerics/safe_conversions.h"
Same here.

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/rtccertifica...
File webrtc/base/rtccertificate_unittests.cc (right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/rtccertifica...
webrtc/base/rtccertificate_unittests.cc:22: #include
"webrtc/base/numerics/safe_conversions.h"
And here.

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideocapturer.cc (right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideocapturer.cc:24: #include
"webrtc/base/numerics/safe_conversions.h"
And here. :)

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/neteq/neteq_impl.cc (right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/neteq/neteq_impl.cc:21: #include
"webrtc/base/numerics/safe_conversions.h"
And here.

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_pro...
File webrtc/modules/audio_processing/audio_processing_performance_unittest.cc
(right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/audio_processing_performance_unittest.cc:23:
#include "webrtc/base/numerics/safe_conversions.h"
Again

[hbos, 2016-03-07 16:49:05]

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/openssladapt...
File webrtc/base/openssladapter.cc (right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/openssladapt...
webrtc/base/openssladapter.cc:41: #include
"webrtc/base/numerics/safe_conversions.h"
On 2016/03/07 16:27:17, tlegrand-webrtc wrote:
> This isn't i alphabetic order. It should be placed between logging.h and
> openssl.h (since "numerics" starts with "n").

Oh, I thought it should be placed at the bottom of the "webrtc/base/..." list
anyway since numerics/ is a folder (ordering by files first, then folders, and
then alphabetical). Not sure why I thought that, making it entirely alphabetical
instead.

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/opensslstrea...
File webrtc/base/opensslstreamadapter.cc (right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/opensslstrea...
webrtc/base/opensslstreamadapter.cc:37: #include
"webrtc/base/numerics/safe_conversions.h"
On 2016/03/07 16:27:17, tlegrand-webrtc wrote:
> Same here.

Done.

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/rtccertifica...
File webrtc/base/rtccertificate_unittests.cc (right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/rtccertifica...
webrtc/base/rtccertificate_unittests.cc:22: #include
"webrtc/base/numerics/safe_conversions.h"
On 2016/03/07 16:27:17, tlegrand-webrtc wrote:
> And here.

Done.

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideocapturer.cc (right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideocapturer.cc:24: #include
"webrtc/base/numerics/safe_conversions.h"
On 2016/03/07 16:27:18, tlegrand-webrtc wrote:
> And here. :)

Done.

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/neteq/neteq_impl.cc (right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/neteq/neteq_impl.cc:21: #include
"webrtc/base/numerics/safe_conversions.h"
On 2016/03/07 16:27:18, tlegrand-webrtc wrote:
> And here.

Done.

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_pro...
File webrtc/modules/audio_processing/audio_processing_performance_unittest.cc
(right):

https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/audio_processing_performance_unittest.cc:23:
#include "webrtc/base/numerics/safe_conversions.h"
On 2016/03/07 16:27:18, tlegrand-webrtc wrote:
> Again

Done.

[kjellander_webrtc, 2016-03-08 01:34:51]

On 2016/03/07 16:49:05, hbos wrote:
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/openssladapt...
> File webrtc/base/openssladapter.cc (right):
> 
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/openssladapt...
> webrtc/base/openssladapter.cc:41: #include
> "webrtc/base/numerics/safe_conversions.h"
> On 2016/03/07 16:27:17, tlegrand-webrtc wrote:
> > This isn't i alphabetic order. It should be placed between logging.h and
> > openssl.h (since "numerics" starts with "n").
> 
> Oh, I thought it should be placed at the bottom of the "webrtc/base/..." list
> anyway since numerics/ is a folder (ordering by files first, then folders, and
> then alphabetical). Not sure why I thought that, making it entirely
alphabetical
> instead.
> 
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/opensslstrea...
> File webrtc/base/opensslstreamadapter.cc (right):
> 
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/opensslstrea...
> webrtc/base/opensslstreamadapter.cc:37: #include
> "webrtc/base/numerics/safe_conversions.h"
> On 2016/03/07 16:27:17, tlegrand-webrtc wrote:
> > Same here.
> 
> Done.
> 
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/rtccertifica...
> File webrtc/base/rtccertificate_unittests.cc (right):
> 
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/base/rtccertifica...
> webrtc/base/rtccertificate_unittests.cc:22: #include
> "webrtc/base/numerics/safe_conversions.h"
> On 2016/03/07 16:27:17, tlegrand-webrtc wrote:
> > And here.
> 
> Done.
> 
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/media/engine/webr...
> File webrtc/media/engine/webrtcvideocapturer.cc (right):
> 
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/media/engine/webr...
> webrtc/media/engine/webrtcvideocapturer.cc:24: #include
> "webrtc/base/numerics/safe_conversions.h"
> On 2016/03/07 16:27:18, tlegrand-webrtc wrote:
> > And here. :)
> 
> Done.
> 
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_cod...
> File webrtc/modules/audio_coding/neteq/neteq_impl.cc (right):
> 
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_cod...
> webrtc/modules/audio_coding/neteq/neteq_impl.cc:21: #include
> "webrtc/base/numerics/safe_conversions.h"
> On 2016/03/07 16:27:18, tlegrand-webrtc wrote:
> > And here.
> 
> Done.
> 
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_pro...
> File webrtc/modules/audio_processing/audio_processing_performance_unittest.cc
> (right):
> 
>
https://codereview.webrtc.org/1753293002/diff/210001/webrtc/modules/audio_pro...
> webrtc/modules/audio_processing/audio_processing_performance_unittest.cc:23:
> #include "webrtc/base/numerics/safe_conversions.h"
> On 2016/03/07 16:27:18, tlegrand-webrtc wrote:
> > Again
> 
> Done.

lgtm but please run trybots

[hbos, 2016-03-08 09:33:56]

The CQ bit was checked by hbos@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-08 09:34:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1753293002/230001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1753293002/230001

[commit-bot: I haz the power, 2016-03-08 09:35:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-08 09:35:39]

Dry run: Try jobs failed on following builders:
  ios64_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/5758)
  mac_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/1412)
  mac_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/7643)

[hbos, 2016-03-08 10:27:27]

The CQ bit was checked by hbos@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-08 10:27:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1753293002/250001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1753293002/250001

[tlegrand-webrtc, 2016-03-08 10:33:02]

Thanks! LGTM

[commit-bot: I haz the power, 2016-03-08 10:36:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-08 10:36:45]

Dry run: Try jobs failed on following builders:
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/3961)

[hbos, 2016-03-08 10:57:41]

On 2016/03/08 10:36:45, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   presubmit on tryserver.webrtc (JOB_FAILED,
> http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/3961)

I assume I can land with NOPRESUBMIT=True since the presubmit warnings we want
to ignore (implicit constructors, header files, etc)...

[hbos, 2016-03-08 12:29:46]

Description was changed from

==========
Safe numeric library added: base/numerics (copied from Chromium)

This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h

A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.

This CL also adds a DEPS file to webrtc/base.

BUG=webrtc:5548, webrtc:5623
==========

to

==========
Safe numeric library added: base/numerics (copied from Chromium)

This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h

A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.

This CL also adds a DEPS file to webrtc/base.

NOPRESUBMIT=True
BUG=webrtc:5548, webrtc:5623
==========

[hbos, 2016-03-08 12:29:57]

The CQ bit was checked by hbos@webrtc.org

[hbos, 2016-03-08 12:29:57]

The patchset sent to the CQ was uploaded after l-g-t-m from kwiberg@webrtc.org,
kjellander@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1753293002/#ps250001 (title:
"Rebase with master + correct new safe_conversions.h include")

[commit-bot: I haz the power, 2016-03-08 12:30:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1753293002/250001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1753293002/250001

[commit-bot: I haz the power, 2016-03-08 12:46:04]

Description was changed from

==========
Safe numeric library added: base/numerics (copied from Chromium)

This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h

A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.

This CL also adds a DEPS file to webrtc/base.

NOPRESUBMIT=True
BUG=webrtc:5548, webrtc:5623
==========

to

==========
Safe numeric library added: base/numerics (copied from Chromium)

This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h

A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.

This CL also adds a DEPS file to webrtc/base.

NOPRESUBMIT=True
BUG=webrtc:5548, webrtc:5623
==========

[commit-bot: I haz the power, 2016-03-08 12:46:05]

Committed patchset #11 (id:250001)

[commit-bot: I haz the power, 2016-03-08 12:46:14]

Description was changed from

==========
Safe numeric library added: base/numerics (copied from Chromium)

This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h

A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.

This CL also adds a DEPS file to webrtc/base.

NOPRESUBMIT=True
BUG=webrtc:5548, webrtc:5623
==========

to

==========
Safe numeric library added: base/numerics (copied from Chromium)

This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h

A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.

This CL also adds a DEPS file to webrtc/base.

NOPRESUBMIT=True
BUG=webrtc:5548, webrtc:5623

Committed: https://crrev.com/de1c81b2d2196be611674aa6019b9db3a9329042
Cr-Commit-Position: refs/heads/master@{#11907}
==========

[commit-bot: I haz the power, 2016-03-08 12:46:16]

Patchset 11 (id:??) landed as
https://crrev.com/de1c81b2d2196be611674aa6019b9db3a9329042
Cr-Commit-Position: refs/heads/master@{#11907}

[tommi, 2016-03-12 01:00:43]

A revert of this CL (patchset #11 id:250001) has been created in
https://codereview.webrtc.org/1792613002/ by tommi@webrtc.org.

The reason for reverting is: Looks like the Chrome iOS build is broken because
of these two changes.  So I'm going to have to revert.  Here's the error:

https://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_n...

FAILED: rm -f arch/libsafe_numerics.arm64.a && ./gyp-mac-tool filter-libtool
libtool  -static -o arch/libsafe_numerics.arm64.a 
error:
/Applications/Xcode7.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool:
no files specified
Usage:
/Applications/Xcode7.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool
-static [-] file [...] [-filelist listfile[,dirname]] [-arch_only arch] [-sacLT]
[-no_warning_for_no_symbols]
Usage:
/Applications/Xcode7.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool
-dynamic [-] file [...] [-filelist listfile[,dirname]] [-arch_only arch] [-o
output] [-install_name name] [-compatibility_version #] [-current_version #]
[-seg1addr 0x#] [-segs_read_only_addr 0x#] [-segs_read_write_addr 0x#]
[-seg_addr_table <filename>] [-seg_addr_table_filename <file_system_path>]
[-all_load] [-noall_load]
FAILED: rm -f arch/libsafe_numerics.armv7.a && ./gyp-mac-tool filter-libtool
libtool  -static -o arch/libsafe_numerics.armv7.a 
error:
/Applications/Xcode7.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool:
no files specified
Usage:
/Applications/Xcode7.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool
-static [-] file [...] [-filelist listfile[,dirname]] [-arch_only arch] [-sacLT]
[-no_warning_for_no_symbols]
Usage:
/Applications/Xcode7.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool
-dynamic [-] file [...] [-filelist listfile[,dirname]] [-arch_only arch] [-o
output] [-install_name name] [-compatibility_version #] [-current_version #]
[-seg1addr 0x#] [-segs_read_only_addr 0x#] [-segs_read_write_addr 0x#]
[-seg_addr_table <filename>] [-seg_addr_table_filename <file_system_path>]
[-all_load] [-noall_load]
ninja: build stopped: subcommand failed..