Build Chrome Remote Desktop for iOS in Chromium trybots.

Build Chrome Remote Desktop for iOS in Chromium trybots.

This CL will default the flag for enable_webrtc to true for iOS, in the past this was set to false for iOS builds in chromium but the remoting app needs webrtc to communicate. This CL will also build the iOS app by default as part of the //remoting:remoting_all target if the arch is set to iOS.

BUG=652781
Review-Url: https://codereview.chromium.org/2809793002
Cr-Commit-Position: refs/heads/master@{#464202}
Committed: https://chromium.googlesource.com/chromium/src/+/1ac03b1c431bb1e187147ed6ca748b6b00fa1297

[nicholss, 2017-04-11 17:23:20]

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

[commit-bot: I haz the power, 2017-04-11 17:24:14]

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

[nicholss, 2017-04-11 17:27:43]

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

[commit-bot: I haz the power, 2017-04-11 17:28:21]

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-04-11 17:52:11]

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

[commit-bot: I haz the power, 2017-04-11 17:52:12]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[nicholss, 2017-04-11 17:53:58]

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

[commit-bot: I haz the power, 2017-04-11 17:54:50]

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-04-11 19:39:08]

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

[commit-bot: I haz the power, 2017-04-11 19:39:09]

Dry run: This issue passed the CQ dry run.

[sdefresne, 2017-04-12 15:27:52]

https://codereview.chromium.org/2809793002/diff/130001/media/media_options.gni
File media/media_options.gni (right):

https://codereview.chromium.org/2809793002/diff/130001/media/media_options.gn...
media/media_options.gni:25: if (is_ios) {
I think this condition will always be false. I would move this outside of the
declare_args:

declare_args() {
  ...
  # Enable usage of FFmpeg within the media library. Used for most software
  # based decoding, demuxing, and sometimes optimized FFTs. If disabled,
  # implementors must provide their own demuxers and decoders.
  media_use_ffmpeg = true

  # Enable usage of libvpx within the media library. Used for software based
  # decoding of VP9 and VP8A type content.
  media_use_libvpx = true
  ...
}

# iOS doesn't use ffmpeg, libvpx.
if (is_ios) {
  media_use_ffmpeg = false
  media_use_libvpx = false
}

If done, this should probably be done as a followup CL.

https://codereview.chromium.org/2809793002/diff/130001/remoting/client/ios/ho...
File remoting/client/ios/host_preferences.mm (right):

https://codereview.chromium.org/2809793002/diff/130001/remoting/client/ios/ho...
remoting/client/ios/host_preferences.mm:47: if (!keychainError) {
This is https://codereview.chromium.org/2817513004/, right? Maybe rebase your CL
on top of it so that this is marked as a dependent CL and does not show in this
one :-)

[nicholss, 2017-04-12 15:35:32]

https://codereview.chromium.org/2809793002/diff/130001/media/media_options.gni
File media/media_options.gni (right):

https://codereview.chromium.org/2809793002/diff/130001/media/media_options.gn...
media/media_options.gni:25: if (is_ios) {
On 2017/04/12 15:27:52, sdefresne wrote:
> I think this condition will always be false. I would move this outside of the
> declare_args:
> 
> declare_args() {
>   ...
>   # Enable usage of FFmpeg within the media library. Used for most software
>   # based decoding, demuxing, and sometimes optimized FFTs. If disabled,
>   # implementors must provide their own demuxers and decoders.
>   media_use_ffmpeg = true
> 
>   # Enable usage of libvpx within the media library. Used for software based
>   # decoding of VP9 and VP8A type content.
>   media_use_libvpx = true
>   ...
> }
> 
> # iOS doesn't use ffmpeg, libvpx.
> if (is_ios) {
>   media_use_ffmpeg = false
>   media_use_libvpx = false
> }
> 
> If done, this should probably be done as a followup CL.

This is very interesting. I wonder how many places this mistake is made in
chromium.

https://codereview.chromium.org/2809793002/diff/130001/remoting/client/ios/ho...
File remoting/client/ios/host_preferences.mm (right):

https://codereview.chromium.org/2809793002/diff/130001/remoting/client/ios/ho...
remoting/client/ios/host_preferences.mm:47: if (!keychainError) {
On 2017/04/12 15:27:52, sdefresne wrote:
> This is https://codereview.chromium.org/2817513004/, right? Maybe rebase your
CL
> on top of it so that this is marked as a dependent CL and does not show in
this
> one :-)

I am working on removing this file from this cl :)

[nicholss, 2017-04-12 16:20:37]

Description was changed from

==========
Adding CRD iOS app to remoting build if building for iOS.

BUG=652781
==========

to

==========
Build Chrome Remote Desktop for iOS in Chromium trybots.

This CL will default the flag for enable_webrtc to true for iOS, in the past
this was set to false for iOS builds in chromium but the remoting app needs
webrtc to communicate. This CL will also build the iOS app by default as part of
the //remoting:remoting_all target if the arch is set to iOS.

BUG=652781
==========

[nicholss, 2017-04-12 16:34:21]

nicholss@chromium.org changed reviewers:
+ dalecurtis@google.com, joedow@chromium.org

[nicholss, 2017-04-12 16:34:22]

PTAL!

Dale for media, Joe for remoting. Thanks!

[nicholss, 2017-04-12 16:34:27]

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

[commit-bot: I haz the power, 2017-04-12 16:35: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-04-12 16:46:29]

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

[commit-bot: I haz the power, 2017-04-12 16:46:30]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[nicholss, 2017-04-12 16:56:53]

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

[commit-bot: I haz the power, 2017-04-12 16:57:45]

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-04-12 17:12:53]

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

[commit-bot: I haz the power, 2017-04-12 17:12:54]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[nicholss, 2017-04-12 17:14:21]

On 2017/04/12 16:34:22, nicholss wrote:
> PTAL!
> 
> Dale for media, Joe for remoting. Thanks!

Arg... https://codereview.chromium.org/2809023003 broke a bunch of stuff for me.
I am working on fixing it up now...

Wait just a moment.

[nicholss, 2017-04-12 17:34:39]

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

[commit-bot: I haz the power, 2017-04-12 17:35:42]

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

[nicholss, 2017-04-12 18:13:32]

On 2017/04/12 17:14:21, nicholss wrote:
> On 2017/04/12 16:34:22, nicholss wrote:
> > PTAL!
> > 
> > Dale for media, Joe for remoting. Thanks!
> 
> Arg... https://codereview.chromium.org/2809023003 broke a bunch of stuff for
me.
> I am working on fixing it up now...
> 
> Wait just a moment.

Ok fixed up. Thanks, PTAL

[joedow, 2017-04-12 18:22:31]

lgtm

lgtm remoting

[DaleCurtis, 2017-04-12 18:31:59]

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

[DaleCurtis, 2017-04-12 18:32:00]

media/ lgtm, I double checked that media is never included on Chrome for iOS
today and has guards in place to prevent it from being dependent on:

https://cs.chromium.org/chromium/src/ios/build/config.gni

[commit-bot: I haz the power, 2017-04-12 20:57:25]

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

[commit-bot: I haz the power, 2017-04-12 20:57:26]

Dry run: This issue passed the CQ dry run.

[nicholss, 2017-04-12 21:05:28]

The CQ bit was checked by nicholss@chromium.org

[commit-bot: I haz the power, 2017-04-12 21:06:30]

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-04-12 23:21:13]

CQ is committing da patch.

Bot data: {"patchset_id": 190001, "attempt_start_ts": 1492031128093860,
"parent_rev": "3e727d61336a80e47949951d2d71aa506e90d774", "commit_rev":
"1ac03b1c431bb1e187147ed6ca748b6b00fa1297"}

[commit-bot: I haz the power, 2017-04-12 23:22:03]

Description was changed from

==========
Build Chrome Remote Desktop for iOS in Chromium trybots.

This CL will default the flag for enable_webrtc to true for iOS, in the past
this was set to false for iOS builds in chromium but the remoting app needs
webrtc to communicate. This CL will also build the iOS app by default as part of
the //remoting:remoting_all target if the arch is set to iOS.

BUG=652781
==========

to

==========
Build Chrome Remote Desktop for iOS in Chromium trybots.

This CL will default the flag for enable_webrtc to true for iOS, in the past
this was set to false for iOS builds in chromium but the remoting app needs
webrtc to communicate. This CL will also build the iOS app by default as part of
the //remoting:remoting_all target if the arch is set to iOS.

BUG=652781

Review-Url: https://codereview.chromium.org/2809793002
Cr-Commit-Position: refs/heads/master@{#464202}
Committed:
https://chromium.googlesource.com/chromium/src/+/1ac03b1c431bb1e187147ed6ca74...
==========

[commit-bot: I haz the power, 2017-04-12 23:22:04]

Committed patchset #11 (id:190001) as
https://chromium.googlesource.com/chromium/src/+/1ac03b1c431bb1e187147ed6ca74...