Reduce APK size by disabling WebRTC.

remoting/BUILD.gn

Index: remoting/BUILD.gn
diff --git a/remoting/BUILD.gn b/remoting/BUILD.gn
index 9b834d0293bfb5b03d523f3681027a84df78ac2a..684bdcc58dcef6c0ccb11a2cf107046ba65f8ce7 100644
--- a/remoting/BUILD.gn
+++ b/remoting/BUILD.gn
@@ -12,6 +12,9 @@ import("//testing/test.gni")
 # Various remoting targets need this version definition.
 config("version") {
   defines = [ "VERSION=$chrome_version_full" ]
+  if (is_android) {

[Sergey Ulanov, 2016/03/16 19:12:35]

config(version) is not the right place to add this define. This config is there
to add to define VERSION. I suggest adding a new config, e.g. in
remoting_options.gni. Then you'll just need to use it in
remoting/client/BUILD.gn and remoting/android/BUILD.gn.

+    defines += [ "DISABLE_WEBRTC=1" ]

[Lambros, 2016/03/16 18:12:20]

I don't think this is the right way.

We already have a #define called "ENABLE_WEBRTC" and a GN flag called
enable_webrtc here:
https://code.google.com/p/chromium/codesearch#chromium/src/build/config/BUILD...

But I think if you were to disable that flag, libjingle would be knocked out as
well and our client wouldn't build at all.

So we need to define a new GN flag, named something like
"enable_webrtc_in_remoting_client"
(Sergey can maybe suggest a better name for this).

The way to define new flags in GN is to add them to a declare_args() section. We
have a file remoting/remoting_options.gni so that seems like the best place to
put the new flag. Default the flag to true, but add something that sets it to
false on Android.

I would also prefer that the #define be named ENABLE_XXX rather than
DISABLE_XXX.

[Yuwei, 2016/03/16 18:39:01]

To follow the pattern in remoting_options.gni, I think I should call it
remoting_enable_webrtc or enable_remoting_webrtc?

I can make it like:

declare_args() {
  ...
  remoting_enable_webrtc = true
}

and later:

if (is_android) {
  remoting_enable_webrtc = false
}

but then we are not able to set GN argument to enable WebRTC on Android... (Or
maybe we don't care?)

Should the `defines += [...]` thing also be in remoting_options?

[Sergey Ulanov, 2016/03/16 19:12:35]

Suggest calling it ENABLE_WEBRTC_REMOTING_CLIENT to make it clear what it's used
for. Also "enable" flags are easier to work with.

[Sergey Ulanov, 2016/03/16 19:12:35]

You need similar change for GYP. 
Just add a condition similar to this:
https://code.google.com/p/chromium/codesearch#chromium/src/remoting/remoting_...

here:
https://code.google.com/p/chromium/codesearch#chromium/src/remoting/remoting_...

Note that remoting_client.gyp is not used for NaCl build, so you just need to
disable this flag for non-official builds.

[Sergey Ulanov, 2016/03/16 19:12:36]

I think ideally we want to disable it only for offical builds. Also it should
always be disabled for NaCl builds:
 if (!is_official_build && !is_nacl) {
   defines += [ "ENABLE_WEBRTC_REMOTING_CLIENT=1" ]
 }

[Yuwei, 2016/03/16 19:45:34]

So we are disabling WebRTC on all platform, right?
It seems the `defines += [...]` thing only works in the config("version") {...}
section in remoting/BUILD.gn. Putting it somewhere else will not define the
marco. Not sure whether it is the right place

[Sergey Ulanov, 2016/03/16 20:13:12]

Correct. Beside Android that only makes difference on iOS.
The targets that use this config have to explicitly pull it, e.g.:
  configs += [ "//remoting:version" ]

If you add another config then you will need to enable it in the same way for
the targets that need this define, i.e. in remoting/client (client/BUILD.gn) and
remoting/android:remoting_client_jni (android/BUILD.gn)

+  }
 }
 
 group("remoting_all") {