Fixing invalid operator< implementation.

Fixing invalid operator< implementation.

It was possible that "A < B" and "B < A" both evaluated to true.
This manifested as an assert on Windows, and a memory leak on Linux.

Note that the concept of "less than" is meaningless for this object.
The operator is only needed so the object can be used as a key in an
std::map.

BUG=webrtc:6068
R=honghaiz@webrtc.org, kjellander@webrtc.org, skvlad@webrtc.org

Committed: https://crrev.com/734262c765dbd13b34da1d016c82c7d3d2c4c50c
Cr-Commit-Position: refs/heads/master@{#13598}

[Taylor Brandstetter, 2016-07-27 17:53:35]

deadbeef@webrtc.org changed reviewers:
+ honghaiz@webrtc.org, skvlad@webrtc.org

[Taylor Brandstetter, 2016-07-27 17:53:36]

PTAL

[skvlad, 2016-07-27 18:00:56]

lgtm

Good catch!

[honghaiz3, 2016-07-27 18:38:13]

https://codereview.webrtc.org/2187913002/diff/1/webrtc/p2p/base/turnserver.cc
File webrtc/p2p/base/turnserver.cc (right):

https://codereview.webrtc.org/2187913002/diff/1/webrtc/p2p/base/turnserver.cc...
webrtc/p2p/base/turnserver.cc:547: if (src_ != c.src_) {
Probably this is a good place to use std::tie since it is a new c++ 11 feature
allowed in Chromium.

https://codereview.webrtc.org/2187913002/diff/1/webrtc/p2p/base/turnserver.cc...
webrtc/p2p/base/turnserver.cc:548: return src_ < c.src_;
Plus, could we add a test to make sure the operator is valid?

[Taylor Brandstetter, 2016-07-28 22:42:08]

https://codereview.webrtc.org/2187913002/diff/1/webrtc/p2p/base/turnserver.cc
File webrtc/p2p/base/turnserver.cc (right):

https://codereview.webrtc.org/2187913002/diff/1/webrtc/p2p/base/turnserver.cc...
webrtc/p2p/base/turnserver.cc:547: if (src_ != c.src_) {
On 2016/07/27 18:38:13, honghaiz3 wrote:
> Probably this is a good place to use std::tie since it is a new c++ 11 feature
> allowed in Chromium. 

Good point! I didn't know about that. Done.

https://codereview.webrtc.org/2187913002/diff/1/webrtc/p2p/base/turnserver.cc...
webrtc/p2p/base/turnserver.cc:548: return src_ < c.src_;
On 2016/07/27 18:38:13, honghaiz3 wrote:
> Plus, could we add a test to make sure the operator is valid?

Done. It's hard to cover every possibility of operator< being valid, but I at
least have a test that catches this issue.

[honghaiz3, 2016-07-28 22:54:45]

Do you forget to upload the change?

[Taylor Brandstetter, 2016-07-29 00:59:24]

On 2016/07/28 22:54:45, honghaiz3 wrote:
> Do you forget to upload the change?

Uploaded to the wrong CL... Check again now.

[honghaiz3, 2016-07-29 15:40:55]

lgtm
Thanks for fixing this!

[Taylor Brandstetter, 2016-08-01 18:48:07]

The CQ bit was checked by deadbeef@webrtc.org

[Taylor Brandstetter, 2016-08-01 18:48:08]

The patchset sent to the CQ was uploaded after l-g-t-m from skvlad@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2187913002/#ps20001 (title:
"Using std::tie, added unit test.")

[commit-bot: I haz the power, 2016-08-01 18:48:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-08-01 18:58:47]

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

[commit-bot: I haz the power, 2016-08-01 18:58:48]

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

[Taylor Brandstetter, 2016-08-01 20:02:54]

deadbeef@webrtc.org changed reviewers:
+ kjellander@webrtc.org

[Taylor Brandstetter, 2016-08-01 20:02:54]

kjellander: Need your approval for change to BUILD.gn.

[kjellander_webrtc, 2016-08-01 20:10:36]

*.gn,*.gyp: lgtm

[kjellander_webrtc, 2016-08-01 20:10:37]

*.gn,*.gyp: lgtm

[Taylor Brandstetter, 2016-08-01 23:37:25]

Description was changed from

==========
Fixing invalid operator< implementation.

It was possible that "A < B" and "B < A" both evaluated to true.
This manifested as an assert on Windows, and a memory leak on Linux.

Note that the concept of "less than" is meaningless for this object.
The operator is only needed so the object can be used as a key in an
std::map.

BUG=webrtc:6068
==========

to

==========
Fixing invalid operator< implementation.

It was possible that "A < B" and "B < A" both evaluated to true.
This manifested as an assert on Windows, and a memory leak on Linux.

Note that the concept of "less than" is meaningless for this object.
The operator is only needed so the object can be used as a key in an
std::map.

BUG=webrtc:6068
R=honghaiz@webrtc.org, kjellander@webrtc.org, skvlad@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/734262c765dbd13b34da1d016...
==========

[Taylor Brandstetter, 2016-08-01 23:37:27]

Committed patchset #4 (id:60001) manually as
734262c765dbd13b34da1d016c82c7d3d2c4c50c (presubmit successful).

[commit-bot: I haz the power, 2016-08-01 23:37:30]

Description was changed from

==========
Fixing invalid operator< implementation.

It was possible that "A < B" and "B < A" both evaluated to true.
This manifested as an assert on Windows, and a memory leak on Linux.

Note that the concept of "less than" is meaningless for this object.
The operator is only needed so the object can be used as a key in an
std::map.

BUG=webrtc:6068
R=honghaiz@webrtc.org, kjellander@webrtc.org, skvlad@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/734262c765dbd13b34da1d016...
==========

to

==========
Fixing invalid operator< implementation.

It was possible that "A < B" and "B < A" both evaluated to true.
This manifested as an assert on Windows, and a memory leak on Linux.

Note that the concept of "less than" is meaningless for this object.
The operator is only needed so the object can be used as a key in an
std::map.

BUG=webrtc:6068
R=honghaiz@webrtc.org, kjellander@webrtc.org, skvlad@webrtc.org

Committed: https://crrev.com/734262c765dbd13b34da1d016c82c7d3d2c4c50c
Cr-Commit-Position: refs/heads/master@{#13598}
==========