Use clampTo instead of chaining std::max(std::min(...))

Use clampTo instead of chaining std::max(std::min(...))

It's common to make a value end up between two other values by using
std::min and std::max but we have a clampTo function that will
make the code much easier to read so we should use it.

The performance is the same (both end up doing inline comparisons and
value selection) but not having to include <algorithm> can bring a
very slight compilation speed boost.

BUG=563433
Committed: https://crrev.com/881b6566ecacfe9154e1f115c588e76cec8a513a
Cr-Commit-Position: refs/heads/master@{#366585}

[Daniel Bratell, 2015-12-17 16:51:35]

bratell@opera.com changed reviewers:
+ kbr@chromium.org, rtoy@chromium.org

[Daniel Bratell, 2015-12-17 16:51:36]

Can you please look at this switch to clampTo() in platform and in particular in
platform/audio (and a little in platform/graphics)?

My long term plan is to maybe get rid of a few #include <algorithm> but
regardless if I get there, this seems like the right thing to do.

[Ken Russell (switch to Gerrit), 2015-12-17 19:13:09]

kbr@chromium.org changed reviewers:
+ hongchan@chromium.org

[Ken Russell (switch to Gerrit), 2015-12-17 19:13:10]

+hongchan. I'd like to defer review to Ray and Hongchan.

[Raymond Toy, 2015-12-17 19:45:55]

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/Biquad.cpp (right):

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/Biquad.cpp:40: #include <complex>
Why is <complex> needed now?

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/DynamicsCompressorKernel.cpp
(right):

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/DynamicsCompressorKernel.cpp:39:
#include <cmath>
Are both of these needed?  Previously, you replaced algorithm with cmath.

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/HRTFKernel.cpp (right):

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/HRTFKernel.cpp:38: #include <algorithm>
Why is algorithm needed now?

[Daniel Bratell, 2015-12-17 22:56:16]

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/DynamicsCompressorKernel.cpp
(right):

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/DynamicsCompressorKernel.cpp:39:
#include <cmath>
On 2015/12/17 19:45:55, Raymond Toy wrote:
> Are both of these needed?  Previously, you replaced algorithm with cmath.

DynamicsCompressorKernel::process uses std::max() so <algorithm> is still
needed. <cmath> is needed for std::isinf() and std::isnan() used in that same
method. I'm not sure where it gets it today, but it should include what it uses
(though I as most people rely on compilation errors to tell me when I forgot
something).

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/HRTFKernel.cpp (right):

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/HRTFKernel.cpp:38: #include <algorithm>
On 2015/12/17 19:45:55, Raymond Toy wrote:
> Why is algorithm needed now?

The code still uses std::min in the HRTFKernel constructor. It used to rely on
someone else(TM) to include <algorithm> but that someone else may be going away.

[Daniel Bratell, 2015-12-17 22:58:19]

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/Biquad.cpp (right):

https://codereview.chromium.org/1530723004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/Biquad.cpp:40: #include <complex>
On 2015/12/17 19:45:55, Raymond Toy wrote:
> Why is <complex> needed now?

It should always have been there but luckily someone else included it so it
worked anyway. While trying to clean up <algorithm> I also picked up other
includes that were missing and corrected that.

[Raymond Toy, 2015-12-17 23:03:39]

Thanks for the explanations.

lgtm for the platform/audio parts.

[hongchan, 2015-12-18 15:46:16]

I had a same set of questions with rtoy@. Thanks for the explanation.

lgtm

[Daniel Bratell, 2015-12-18 16:07:11]

On 2015/12/18 15:46:16, hoch wrote:
> I had a same set of questions with rtoy@. Thanks for the explanation.
> 
> lgtm

Thanks!

kbr, still need your ok I think since hoch isn't in the OWNERS file.

[Ken Russell (switch to Gerrit), 2015-12-21 22:09:23]

lgtm

[Daniel Bratell, 2015-12-22 09:02:02]

The CQ bit was checked by bratell@opera.com

[commit-bot: I haz the power, 2015-12-22 09:02:36]

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

[commit-bot: I haz the power, 2015-12-22 09:04:35]

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

[commit-bot: I haz the power, 2015-12-22 09:04:36]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[Daniel Bratell, 2015-12-22 09:39:39]

The CQ bit was checked by bratell@opera.com

[Daniel Bratell, 2015-12-22 09:39:40]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
rtoy@chromium.org, hongchan@chromium.org
Link to the patchset: https://codereview.chromium.org/1530723004/#ps80001
(title: "More rebase")

[commit-bot: I haz the power, 2015-12-22 09:39:56]

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

[commit-bot: I haz the power, 2015-12-22 10:58:47]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-12-22 10:59:36]

Description was changed from

==========
Use clampTo instead of chaining std::max(std::min(...))

It's common to make a value end up between two other values by using
std::min and std::max but we have a clampTo function that will
make the code much easier to read so we should use it.

The performance is the same (both end up doing inline comparisons and
value selection) but not having to include <algorithm> can bring a
very slight compilation speed boost.

BUG=563433
==========

to

==========
Use clampTo instead of chaining std::max(std::min(...))

It's common to make a value end up between two other values by using
std::min and std::max but we have a clampTo function that will
make the code much easier to read so we should use it.

The performance is the same (both end up doing inline comparisons and
value selection) but not having to include <algorithm> can bring a
very slight compilation speed boost.

BUG=563433

Committed: https://crrev.com/881b6566ecacfe9154e1f115c588e76cec8a513a
Cr-Commit-Position: refs/heads/master@{#366585}
==========

[commit-bot: I haz the power, 2015-12-22 10:59:37]

Patchset 5 (id:??) landed as
https://crrev.com/881b6566ecacfe9154e1f115c588e76cec8a513a
Cr-Commit-Position: refs/heads/master@{#366585}