Cleanup inconsistencies between GN and GYP in //remoting

Cleanup inconsistencies between GN and GYP in //remoting

1. GN builds were not including some tests included in GYP.
2. Replaced test_common target in gyp with test_support.
3. Removed extra *_common targets for test_driver and moved everything
   to test_support.
4. test_driver can be compiled using GN now.
5. Various other small cleanups.

Committed: https://crrev.com/15945fd48ba40b6133434c8dfb7c31b4525af64a
Cr-Commit-Position: refs/heads/master@{#339119}

[Sergey Ulanov, 2015-07-15 19:25:07]

sergeyu@chromium.org changed reviewers:
+ joedow@chromium.org

[joedow, 2015-07-16 02:45:15]

https://codereview.chromium.org/1228283004/diff/1/remoting/BUILD.gn
File remoting/BUILD.gn (right):

https://codereview.chromium.org/1228283004/diff/1/remoting/BUILD.gn#newcode74
remoting/BUILD.gn:74: # GYP version:
remoting/remoting_test.gypi:remoting_test_common
s/remoting_test_common/remoting_test_support

https://codereview.chromium.org/1228283004/diff/1/remoting/remoting_test.gypi
File remoting/remoting_test.gypi (left):

https://codereview.chromium.org/1228283004/diff/1/remoting/remoting_test.gypi...
remoting/remoting_test.gypi:154: 'target_name': 'ar_test_driver_common',
Removing this target will affect the internal ar_test_driver target, and
including remoting_test_support won't work in this change because the internal
target pulls in an internal *app_details.cc file so the public app_details.cc
file may need to get moved down to ar_sample_test_driver target

https://codereview.chromium.org/1228283004/diff/1/remoting/test/BUILD.gn
File remoting/test/BUILD.gn (right):

https://codereview.chromium.org/1228283004/diff/1/remoting/test/BUILD.gn#newc...
remoting/test/BUILD.gn:103: ]
nit: the newlines between blocks seems inconsistent (in all of the files),
should there be breaks in between blocks (or is there a formatter that will
handle it automatically)?

[Sergey Ulanov, 2015-07-16 17:39:12]

https://codereview.chromium.org/1228283004/diff/1/remoting/BUILD.gn
File remoting/BUILD.gn (right):

https://codereview.chromium.org/1228283004/diff/1/remoting/BUILD.gn#newcode74
remoting/BUILD.gn:74: # GYP version:
remoting/remoting_test.gypi:remoting_test_common
On 2015/07/16 02:45:14, joedow wrote:
> s/remoting_test_common/remoting_test_support

Done.

https://codereview.chromium.org/1228283004/diff/1/remoting/remoting_test.gypi
File remoting/remoting_test.gypi (left):

https://codereview.chromium.org/1228283004/diff/1/remoting/remoting_test.gypi...
remoting/remoting_test.gypi:154: 'target_name': 'ar_test_driver_common',
On 2015/07/16 02:45:14, joedow wrote:
> Removing this target will affect the internal ar_test_driver target, and
> including remoting_test_support won't work in this change because the internal
> target pulls in an internal *app_details.cc file so the public app_details.cc
> file may need to get moved down to ar_sample_test_driver target

I see. I've added ar_test_driver_common back, but now it depends on
remoting_test_support. This still avoids compiling some files twice for
different targets.

But I don't think this kind of dependencies between targets is a good practice.
Essentially here we have target B that depends on A, but some code in A also
depends on code in B, i.e. we've got circular dependencies between the two
target. Not only it makes it harder to make changes in the build configuration
(because there is an implicit dependency between files), but also makes the code
harder to read. Can we avoid this? E.g. maybe pass the initialization hooks to
the AppRemotingTestDriverEnvironment constructor. Or you can make
PopulateApplicationNames() and PopulateApplicationDetailsMap() pure virtual and
override them in app-specific child classes.

https://codereview.chromium.org/1228283004/diff/1/remoting/test/BUILD.gn
File remoting/test/BUILD.gn (right):

https://codereview.chromium.org/1228283004/diff/1/remoting/test/BUILD.gn#newc...
remoting/test/BUILD.gn:103: ]
On 2015/07/16 02:45:14, joedow wrote:
> nit: the newlines between blocks seems inconsistent (in all of the files),
> should there be breaks in between blocks (or is there a formatter that will
> handle it automatically)?

There is a formatter for gn, but it doesn't insert or remove these empty lines

[joedow, 2015-07-16 18:37:59]

lgtm

[Sergey Ulanov, 2015-07-16 19:03:24]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2015-07-16 19:05:30]

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

[commit-bot: I haz the power, 2015-07-16 19:13:01]

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

[commit-bot: I haz the power, 2015-07-16 19:13:02]

Try jobs failed on following builders:
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

(exceeded global retry quota)

[Sergey Ulanov, 2015-07-16 19:20:29]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2015-07-16 19:20:30]

The patchset sent to the CQ was uploaded after l-g-t-m from joedow@chromium.org
Link to the patchset: https://codereview.chromium.org/1228283004/#ps60001
(title: " ")

[commit-bot: I haz the power, 2015-07-16 19:23:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1228283004/60001

[commit-bot: I haz the power, 2015-07-16 19:34:00]

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

[commit-bot: I haz the power, 2015-07-16 19:34:02]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

(exceeded global retry quota)

[Sergey Ulanov, 2015-07-16 19:48:35]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2015-07-16 19:50:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1228283004/60001

[commit-bot: I haz the power, 2015-07-16 21:06:12]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-07-16 21:07:16]

Patchset 4 (id:??) landed as
https://crrev.com/15945fd48ba40b6133434c8dfb7c31b4525af64a
Cr-Commit-Position: refs/heads/master@{#339119}