Move FirewallSocketServer to test code.

Move FirewallSocketServer to test code.

BUG=webrtc:6424
Committed: https://crrev.com/90c024fc1cc79e35231c223269f8eed31848213b
Cr-Commit-Position: refs/heads/master@{#15154}

[nisse-webrtc, 2016-11-18 11:17:45]

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

[nisse-webrtc, 2016-11-18 11:17:45]

FirewallSocketServer became unused in production code when I deleted
base/httprequest.* the other day.

I need some help to sort it the dependencies when moving it into a test target.
I put it in test_common, not sure if that's the best place.

https://codereview.webrtc.org/2515443003/diff/1/webrtc/api/BUILD.gn
File webrtc/api/BUILD.gn (right):

https://codereview.webrtc.org/2515443003/diff/1/webrtc/api/BUILD.gn#newcode453
webrtc/api/BUILD.gn:453: "../modules/video_capture:video_capture",
I don't understand why this becomes needed now. Without it, I get link errors.

https://codereview.webrtc.org/2515443003/diff/1/webrtc/api/BUILD.gn#newcode456
webrtc/api/BUILD.gn:456: "../test:test_common",
And this is to get FirewallSocketServer.

https://codereview.webrtc.org/2515443003/diff/1/webrtc/test/BUILD.gn
File webrtc/test/BUILD.gn (right):

https://codereview.webrtc.org/2515443003/diff/1/webrtc/test/BUILD.gn#newcode336
webrtc/test/BUILD.gn:336: "../base:rtc_base",
This violates dependency rules. But imo, test code ought to be allowed to depend
on more or less anything. How do we solve this?

It's needed because the base class of FirewallSocketServer is defined in
base/socketserver.cc, which is in rtc_base but (for good reason, I guess) not in
rtc_base_approved.

https://codereview.webrtc.org/2515443003/diff/1/webrtc/test/firewallsocketser...
File webrtc/test/firewallsocketserver.h (right):

https://codereview.webrtc.org/2515443003/diff/1/webrtc/test/firewallsocketser...
webrtc/test/firewallsocketserver.h:94: class FirewallManager {
I think this class is unused. I'll consider deleting it, after the other issues
in this cl are solved.

[kjellander_webrtc, 2016-11-18 13:52:40]

https://codereview.webrtc.org/2515443003/diff/1/webrtc/BUILD.gn
File webrtc/BUILD.gn (right):

https://codereview.webrtc.org/2515443003/diff/1/webrtc/BUILD.gn#newcode541
webrtc/BUILD.gn:541: "modules/video_capture:video_capture_internal_impl",
If you need this to be able to depend on test:test_common, I suggest just moving
firewallsocketserver.{cc,h} into rtc_base_tests_utils instead, which is already
depended upon.

https://codereview.webrtc.org/2515443003/diff/1/webrtc/api/BUILD.gn
File webrtc/api/BUILD.gn (right):

https://codereview.webrtc.org/2515443003/diff/1/webrtc/api/BUILD.gn#newcode453
webrtc/api/BUILD.gn:453: "../modules/video_capture:video_capture",
On 2016/11/18 11:17:45, nisse-webrtc wrote:
> I don't understand why this becomes needed now. Without it, I get link errors.

It's because of test_common. See
https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_capture...
(you have to pick one of the implementations).

https://codereview.webrtc.org/2515443003/diff/1/webrtc/base/BUILD.gn
File webrtc/base/BUILD.gn (right):

https://codereview.webrtc.org/2515443003/diff/1/webrtc/base/BUILD.gn#newcode766
webrtc/base/BUILD.gn:766: rtc_source_set("rtc_base_tests_utils") {
Isn't it a lot easier to move it into this target instead?

[nisse-webrtc, 2016-11-18 14:15:42]

https://codereview.webrtc.org/2515443003/diff/1/webrtc/base/BUILD.gn
File webrtc/base/BUILD.gn (right):

https://codereview.webrtc.org/2515443003/diff/1/webrtc/base/BUILD.gn#newcode766
webrtc/base/BUILD.gn:766: rtc_source_set("rtc_base_tests_utils") {
On 2016/11/18 13:52:40, kjellander_webrtc wrote:
> Isn't it a lot easier to move it into this target instead? 

Indeed, that's a lot simpler. See new patch set.

[kjellander_webrtc, 2016-11-18 14:19:57]

lgtm

[nisse-webrtc, 2016-11-18 14:54:08]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-11-18 14:54:22]

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-11-18 15:10:44]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-18 15:10:56]

Description was changed from

==========
Move FirewallSocketServer to test code.

BUG=webrtc:6424
==========

to

==========
Move FirewallSocketServer to test code.

BUG=webrtc:6424

Committed: https://crrev.com/90c024fc1cc79e35231c223269f8eed31848213b
Cr-Commit-Position: refs/heads/master@{#15154}
==========

[commit-bot: I haz the power, 2016-11-18 15:10:57]

Patchset 2 (id:??) landed as
https://crrev.com/90c024fc1cc79e35231c223269f8eed31848213b
Cr-Commit-Position: refs/heads/master@{#15154}