Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(737)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1769783002: Migrate net::IPAddressList to net::IPAddress. (Closed)

Created:
4 years, 9 months ago by martijnc
Modified:
4 years, 9 months ago
CC:
cbentzel+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, jam, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, mmenke
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Migrate net::IPAddressList to net::IPAddress. BUG=496258 Committed: https://crrev.com/9940329b5eb962cf258951c15902bf9752e99009 Cr-Commit-Position: refs/heads/master@{#380467}

Patch Set 1 #

Total comments: 2

Patch Set 2 : comments eroman #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -9 lines) Patch
M content/browser/renderer_host/p2p/socket_dispatcher_host.h View 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/p2p/socket_dispatcher_host.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/common/p2p_messages.h View 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/p2p/host_address_request.h View 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/p2p/ipc_socket_factory.cc View 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/p2p/socket_dispatcher.h View 1 chunk +1 line, -0 lines 2 comments Download
M net/base/address_list_unittest.cc View 2 chunks +4 lines, -3 lines 0 comments Download
M net/base/ip_address.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M net/base/ip_address_number.h View 1 chunk +0 lines, -1 line 0 comments Download
M net/dns/dns_response.cc View 3 chunks +6 lines, -3 lines 0 comments Download

Messages

Total messages: 23 (9 generated)
martijnc
This is still a typedef but it's only used in a limited number of files. ...
4 years, 9 months ago (2016-03-08 20:34:17 UTC) #2
eroman
LGTM > (Thank you for reviewing all these patches :)!) No problem, thanks for writing ...
4 years, 9 months ago (2016-03-08 21:04:03 UTC) #3
martijnc
https://codereview.chromium.org/1769783002/diff/1/net/base/ip_address.h File net/base/ip_address.h (right): https://codereview.chromium.org/1769783002/diff/1/net/base/ip_address.h#newcode108 net/base/ip_address.h:108: typedef std::vector<IPAddress> IPAddressList; On 2016/03/08 at 21:04:03, eroman wrote: ...
4 years, 9 months ago (2016-03-09 16:44:27 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1769783002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1769783002/20001
4 years, 9 months ago (2016-03-09 16:44:35 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/154865)
4 years, 9 months ago (2016-03-09 16:52:59 UTC) #9
martijnc
+sergeyu@chromium.org for content/browser/renderer_host/p2p/* and content/renderer/p2p/* +mkwst@chromium.org for content/common/p2p_messages.h Can you take a look? Thanks!
4 years, 9 months ago (2016-03-09 17:01:25 UTC) #11
Sergey Ulanov
lgtm https://codereview.chromium.org/1769783002/diff/20001/content/renderer/p2p/socket_dispatcher.h File content/renderer/p2p/socket_dispatcher.h (right): https://codereview.chromium.org/1769783002/diff/20001/content/renderer/p2p/socket_dispatcher.h#newcode38 content/renderer/p2p/socket_dispatcher.h:38: #include "net/base/ip_address.h" nit: Do you need this include ...
4 years, 9 months ago (2016-03-09 18:04:56 UTC) #12
martijnc
https://codereview.chromium.org/1769783002/diff/20001/content/renderer/p2p/socket_dispatcher.h File content/renderer/p2p/socket_dispatcher.h (right): https://codereview.chromium.org/1769783002/diff/20001/content/renderer/p2p/socket_dispatcher.h#newcode38 content/renderer/p2p/socket_dispatcher.h:38: #include "net/base/ip_address.h" On 2016/03/09 at 18:04:56, Sergey Ulanov wrote: ...
4 years, 9 months ago (2016-03-09 18:39:16 UTC) #13
Mike West
Adding the .h to IPC LGTM.
4 years, 9 months ago (2016-03-10 08:47:45 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1769783002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1769783002/20001
4 years, 9 months ago (2016-03-10 16:38:48 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/194621)
4 years, 9 months ago (2016-03-10 18:08:09 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1769783002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1769783002/20001
4 years, 9 months ago (2016-03-10 21:14:42 UTC) #20
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 9 months ago (2016-03-10 21:21:07 UTC) #21
commit-bot: I haz the power
4 years, 9 months ago (2016-03-10 21:22:45 UTC) #23
Message was sent while issue was closed. 
Patchset 2 (id:??) landed as
https://crrev.com/9940329b5eb962cf258951c15902bf9752e99009
Cr-Commit-Position: refs/heads/master@{#380467}

Powered by Google App Engine
This is Rietveld 408576698