Cleanups in remoting gn files.

Cleanups in remoting gn files.

1. Removed remoting/codec:codec. Now dependent target have to depend
   either on encoder, decoder or both
2. Cleaned up the logic used to include host-specific code that depends 
   on the encoder.
3. enable_remoting is set to false when enable_webrtc is false.
4. Added assert_no_deps for Android JNI target to ensure it doesn't 
   depend on ffmpeg

Review-Url: https://codereview.chromium.org/2640783004
Cr-Commit-Position: refs/heads/master@{#445461}
Committed: https://chromium.googlesource.com/chromium/src/+/5342d96f531165d81953c4e39537491dcb7712c8

[Sergey Ulanov, 2017-01-20 21:24:09]

Description was changed from

==========
Cleanups in remoting gn files.

1. Removed remoting/codec:codec to ensure remoting/client doesn't depend
   on encoder.
2. Cleaned up the logic used to include webrtc protocol.
==========

to

==========
Cleanups in remoting gn files.

1. Removed remoting/codec:codec. Now dependent target have to depend
   either on encoder, decoder or both
2. Cleaned up the logic used to include host-specific code that depends 
   on the encoder.
3. Added assert_no_deps for Android JNI target to ensure it doesn't 
   depend on ffmpet
==========

[Sergey Ulanov, 2017-01-20 21:26:55]

Description was changed from

==========
Cleanups in remoting gn files.

1. Removed remoting/codec:codec. Now dependent target have to depend
   either on encoder, decoder or both
2. Cleaned up the logic used to include host-specific code that depends 
   on the encoder.
3. Added assert_no_deps for Android JNI target to ensure it doesn't 
   depend on ffmpet
==========

to

==========
Cleanups in remoting gn files.

1. Removed remoting/codec:codec. Now dependent target have to depend
   either on encoder, decoder or both
2. Cleaned up the logic used to include host-specific code that depends 
   on the encoder.
3. Added assert_no_deps for Android JNI target to ensure it doesn't 
   depend on ffmpeg
==========

[Sergey Ulanov, 2017-01-20 21:27:00]

Patchset #1 (id:1) has been deleted

[Sergey Ulanov, 2017-01-20 21:36:03]

Description was changed from

==========
Cleanups in remoting gn files.

1. Removed remoting/codec:codec. Now dependent target have to depend
   either on encoder, decoder or both
2. Cleaned up the logic used to include host-specific code that depends 
   on the encoder.
3. Added assert_no_deps for Android JNI target to ensure it doesn't 
   depend on ffmpeg
==========

to

==========
Cleanups in remoting gn files.

1. Removed remoting/codec:codec. Now dependent target have to depend
   either on encoder, decoder or both
2. Cleaned up the logic used to include host-specific code that depends 
   on the encoder.
3. enable_remoting is set to false when enable_webrtc is false.
4. Added assert_no_deps for Android JNI target to ensure it doesn't 
   depend on ffmpeg
==========

[Sergey Ulanov, 2017-01-20 21:36:27]

Patchset #1 (id:20001) has been deleted

[Sergey Ulanov, 2017-01-20 21:37:28]

sergeyu@chromium.org changed reviewers:
+ jamiewalch@chromium.org, nicholss@chromium.org

[Jamie, 2017-01-20 21:44:19]

LGTM. Does this need to be merged to M57?

[Jamie, 2017-01-20 21:44:20]

LGTM. Does this need to be merged to M57?

[Sergey Ulanov, 2017-01-20 22:35:34]

On 2017/01/20 21:44:20, Jamie wrote:
> LGTM. Does this need to be merged to M57?

No, I don't think it needs to be merged

[nicholss, 2017-01-20 23:05:23]

https://codereview.chromium.org/2640783004/diff/40001/remoting/codec/BUILD.gn
File remoting/codec/BUILD.gn (left):

https://codereview.chromium.org/2640783004/diff/40001/remoting/codec/BUILD.gn...
remoting/codec/BUILD.gn:5: source_set("codec") {
why delete this? A dep on  :encoder :decoder was :codec (the file name) and then
you can just include the file path in gn without any targets.

https://codereview.chromium.org/2640783004/diff/40001/remoting/remoting_enabl...
File remoting/remoting_enable.gni (left):

https://codereview.chromium.org/2640783004/diff/40001/remoting/remoting_enabl...
remoting/remoting_enable.gni:11: enable_remoting_host = false
I am not sure it is better to move these to the remoting_options. These are
enable flags. Options to me mean more like feature or implementation flags.

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

https://codereview.chromium.org/2640783004/diff/40001/remoting/test/BUILD.gn#...
remoting/test/BUILD.gn:77: "//remoting/codec:encoder",
If you leave the default target for //remoting/codec you don't need to have
both. Or move the decoder and encoder target to public deps in the :codec target
and it will trickle up as you expect.

[Sergey Ulanov, 2017-01-20 23:41:41]

https://codereview.chromium.org/2640783004/diff/40001/remoting/codec/BUILD.gn
File remoting/codec/BUILD.gn (left):

https://codereview.chromium.org/2640783004/diff/40001/remoting/codec/BUILD.gn...
remoting/codec/BUILD.gn:5: source_set("codec") {
On 2017/01/20 23:05:22, nicholss wrote:
> why delete this? A dep on  :encoder :decoder was :codec (the file name) and
then
> you can just include the file path in gn without any targets.

It's to ensure that we don't have any targets that unintentionally depend on on
the encoder when they need only decoder.

https://codereview.chromium.org/2640783004/diff/40001/remoting/remoting_enabl...
File remoting/remoting_enable.gni (left):

https://codereview.chromium.org/2640783004/diff/40001/remoting/remoting_enabl...
remoting/remoting_enable.gni:11: enable_remoting_host = false
On 2017/01/20 23:05:22, nicholss wrote:
> I am not sure it is better to move these to the remoting_options. These are
> enable flags. Options to me mean more like feature or implementation flags.

Only the values declared in declare_args() can be overridden in gn args. These 2
were not in declare_args(). Note that remoting_options.gni also has
declare_args() section with some values there. Every other value that's not in
declare_args() is a function of these args and the build environment (such as
is_os flags).
The difference between remoting_options.gni and remoting_enable.gni files is
completely unclear, and I'm not sure we need both. For example, why
enable_remoting_host was in this file, while remoting_multi_process is in
remoting_options.gni? For now I kept enable_remoting in this file because it is
referenced outside of the remoting directory, but in the future I think we
should merged these 2 files. I don't see any reason to keep them separate. WDYT?

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

https://codereview.chromium.org/2640783004/diff/40001/remoting/test/BUILD.gn#...
remoting/test/BUILD.gn:77: "//remoting/codec:encoder",
On 2017/01/20 23:05:22, nicholss wrote:
> If you leave the default target for //remoting/codec you don't need to have
> both. Or move the decoder and encoder target to public deps in the :codec
target
> and it will trickle up as you expect.

The goal is to force every target that needs encoders to depend on in explicitly
as it pulls media and ffmpeg deps, that are not needed on the client side.

[Sergey Ulanov, 2017-01-20 23:41:57]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-20 23:42:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-20 23:49:22]

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

[commit-bot: I haz the power, 2017-01-20 23:49:23]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[nicholss, 2017-01-20 23:51:21]

lgtm

[Sergey Ulanov, 2017-01-20 23:55:24]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2017-01-20 23:55:24]

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

[commit-bot: I haz the power, 2017-01-20 23:55:57]

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

[commit-bot: I haz the power, 2017-01-21 01:28:09]

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

[commit-bot: I haz the power, 2017-01-21 01:28:10]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Sergey Ulanov, 2017-01-23 18:55:47]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2017-01-23 18:55:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
nicholss@chromium.org, jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/2640783004/#ps80001
(title: ".")

[commit-bot: I haz the power, 2017-01-23 18:56:25]

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

[commit-bot: I haz the power, 2017-01-23 20:18:57]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1485197747637320,
"parent_rev": "1222fa1971bebdc5325b3d6ab4de003f54ec0783", "commit_rev":
"5342d96f531165d81953c4e39537491dcb7712c8"}

[commit-bot: I haz the power, 2017-01-23 20:19:30]

Description was changed from

==========
Cleanups in remoting gn files.

1. Removed remoting/codec:codec. Now dependent target have to depend
   either on encoder, decoder or both
2. Cleaned up the logic used to include host-specific code that depends 
   on the encoder.
3. enable_remoting is set to false when enable_webrtc is false.
4. Added assert_no_deps for Android JNI target to ensure it doesn't 
   depend on ffmpeg
==========

to

==========
Cleanups in remoting gn files.

1. Removed remoting/codec:codec. Now dependent target have to depend
   either on encoder, decoder or both
2. Cleaned up the logic used to include host-specific code that depends 
   on the encoder.
3. enable_remoting is set to false when enable_webrtc is false.
4. Added assert_no_deps for Android JNI target to ensure it doesn't 
   depend on ffmpeg

Review-Url: https://codereview.chromium.org/2640783004
Cr-Commit-Position: refs/heads/master@{#445461}
Committed:
https://chromium.googlesource.com/chromium/src/+/5342d96f531165d81953c4e39537...
==========

[commit-bot: I haz the power, 2017-01-23 20:19:32]

Committed patchset #3 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/5342d96f531165d81953c4e39537...