Fixing building of remoting_unittests_apk target.

Fixing building of remoting_unittests_apk target.

BUG=

Committed: https://crrev.com/a78c3daa7c5a8fc1e3e81cb42f17287c3ff601ad
Cr-Commit-Position: refs/heads/master@{#322482}

[Łukasz Anforowicz, 2015-03-23 23:57:11]

Lambros, could you please take a look?

FWIW, I tried building for Android (*) and Linux/desktop (**) and could
successfully build remoting_unittests target after my changes.  I didn't verify
that the tests pass on Android (couldn't figure out the test_runner
incantation), but building works now, so sending the review out to discuss the
gyp changes.

(*) GYP_DEFINES="use_goma=1 gomadir=/usr/local/google/home/lukasza/goma
OS=android target_arch=ia32" GYP_GENERATOR_FLAGS="output_dir=out_andr" gclient
runhooks

(**) GYP_DEFINES="use_goma=1 gomadir=/usr/local/google/home/lukasza/goma"
gclient runhooks

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

https://codereview.chromium.org/1010803014/diff/1/remoting/remoting_test.gypi...
remoting/remoting_test.gypi:249: 'signaling/fake_signal_strategy.cc',
The removed source files are also present in remoting_test_common target. 
Without removing these files, linking would fail with multiple cases of:

/usr/local/google/home/lukasza/src/chromium/src/third_party/android_tools/ndk/toolchains/x86-4.9/prebuilt/linux-x86_64/bin/../lib/gcc/i686-linux-android/4.9/../../../../i686-linux-android/bin/ld.gold:
error:
obj/remoting/libremoting_test_common.a(obj/remoting/signaling/remoting_test_common.fake_signal_strategy.o):
multiple definition of 'remoting::FakeSignalStrategy::GetState() const'
/usr/local/google/home/lukasza/src/chromium/src/third_party/android_tools/ndk/toolchains/x86-4.9/prebuilt/linux-x86_64/bin/../lib/gcc/i686-linux-android/4.9/../../../../i686-linux-android/bin/ld.gold:
obj/remoting/signaling/remoting_unittests.fake_signal_strategy.o: previous
definition here

Interestingly, there are other source files that appears both in
remoting_test_common and in remoting_unittests targets (i.e.
host/fake_resktop_capturer.cc).  FWIW, these do not cause a linking error on
Android, so for now I am making a minimal change to enable building (and later)
running remoting_unittests on Android.

https://codereview.chromium.org/1010803014/diff/1/remoting/remoting_test.gypi
File remoting/remoting_test.gypi (right):

https://codereview.chromium.org/1010803014/diff/1/remoting/remoting_test.gypi...
remoting/remoting_test.gypi:284: 'client/plugin/touch_input_scaler_unittest.cc',
Since we are removing remoting_client_plugin when building for Android, we also
need to remove tests that depend on remoting_client_plugin.

Example linking error when linking object files created from the test sources
being removed:
../../remoting/client/plugin/empty_cursor_filter_unittest.cc:69: error:
undefined reference to 'remoting::EmptyCursorShape()'

[Lambros, 2015-03-24 00:19:20]

lambroslambrou@chromium.org changed reviewers:
+ lambroslambrou@chromium.org

[Lambros, 2015-03-24 00:19:20]

https://codereview.chromium.org/1010803014/diff/1/remoting/remoting_test.gypi
File remoting/remoting_test.gypi (right):

https://codereview.chromium.org/1010803014/diff/1/remoting/remoting_test.gypi...
remoting/remoting_test.gypi:280: 'sources!': [
I think you should remove everything from plugin/ directory like this:

  'sources/': [
    ['exclude', '^client/plugin/'],
  ]

(See line 87 above, where we do something similar for host/ stuff)

[Lambros, 2015-03-24 00:30:54]

I think you might also need to update client/BUILD.gn

[Łukasz Anforowicz, 2015-03-24 17:10:23]

I've added BUILD.gn coverage but things still don't feel 100% right:

I can build fine with:
    $ cat out/gn_linux/args.gn
    is_debug = true
    use_goma = true
but tests fail:
    $ gn gen out/gn_linux
    ...
    $ ninja -C out/gn_linux -j 200 remoting_unittests
    ...
    $ out/gn_linux/remoting_unittests 
    10 tests failed:
        Me2MeNativeMessagingHostTest.All
        Me2MeNativeMessagingHostTest.GetPinHashNoHostId
        Me2MeNativeMessagingHostTest.GetPinHashNoPin
        Me2MeNativeMessagingHostTest.InvalidType
        Me2MeNativeMessagingHostTest.MissingType
        Me2MeNativeMessagingHostTest.StartDaemonInvalidConfig
        Me2MeNativeMessagingHostTest.StartDaemonNoConsent
        Me2MeNativeMessagingHostTest.UpdateDaemonConfigInvalidConfig
        Me2MeNativeMessagingHostTest.WrongFormat
        ServerLogEntryClientTest.AddClientFields    

Also - I canno build remoting_unittests for Android - surporisingly
remoting_unittests target is not recognized:
    $ cat out/gn_andr/args.gn
    is_debug = true
    target_os = "android"
    use_goma = true
    $ gn gen out/gn_andr
    Done. Wrote 4291 targets from 550 files in 1701ms
    $ ninja -C out/gn_andr -j 200 remoting_unittests
    ninja: Entering directory `out/gn_andr'
    ninja: error: unknown target 'remoting_unittests'

Given that we don't have BUILD.gn for remoting/android, maybe we want to push
the current changes out (to unblock test-recipe-authoring and running these
tests on FYI Android bots) and tackle gn-related stuff later on (asap as a
separate bug OR when we transition to gn "for real" [because if we make the
fixes today then they might again "rot" unless gn is actively used by the team
and bots]).

https://codereview.chromium.org/1010803014/diff/1/remoting/remoting_test.gypi
File remoting/remoting_test.gypi (right):

https://codereview.chromium.org/1010803014/diff/1/remoting/remoting_test.gypi...
remoting/remoting_test.gypi:280: 'sources!': [
On 2015/03/24 00:19:20, Lambros wrote:
> I think you should remove everything from plugin/ directory like this:
> 
>   'sources/': [
>     ['exclude', '^client/plugin/'],
>   ]
> 
> (See line 87 above, where we do something similar for host/ stuff)

Good point.  Done.

https://codereview.chromium.org/1010803014/diff/20001/remoting/client/plugin/...
File remoting/client/plugin/BUILD.gn (right):

https://codereview.chromium.org/1010803014/diff/20001/remoting/client/plugin/...
remoting/client/plugin/BUILD.gn:44: source_set("unit_tests") {
The "unit_tests" target is mostly copied from remoting/client/BUILD.gn, but
limited only to tests under "plugin".

https://codereview.chromium.org/1010803014/diff/20001/remoting/client/plugin/...
remoting/client/plugin/BUILD.gn:50: "touch_input_scaler_unittest.cc",
touch_input_scaler_unittest.cc was present in gyp but not in GN.

[Łukasz Anforowicz, 2015-03-26 18:21:56]

Lambros?

[Lambros, 2015-03-26 21:19:27]

lgtm

[Łukasz Anforowicz, 2015-03-26 21:54:17]

The CQ bit was checked by lukasza@chromium.org

[commit-bot: I haz the power, 2015-03-26 21:54:54]

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

[commit-bot: I haz the power, 2015-03-26 22:37:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-26 22:38:18]

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