Removing unneeded webrtc dependencies on iOS.

Removing unneeded webrtc dependencies on iOS.

BUG=459705

[sherouk, 2015-09-09 15:47:08]

sherouk@google.com changed reviewers:
+ dpranke@google.com, kjellander@webrtc.org, sdefresne@chromium.org

[sherouk, 2015-09-09 15:47:19]

Can you review please?

[kjellander_webrtc, 2015-09-09 16:10:36]

First of all, you need to create a CL using a clone of
https://chromium.googlesource.com/external/webrtc - the directories in a
Chromium checkout are mirrors that are read-only so they cannot be committed to
and the WebRTC standalone trybots cannot try the patches.

Secondly, I don't think this is correct - most of our code actually builds  on
iOS and we don't have anything similar to these changes in our GYP files.

[sdefresne, 2015-09-09 16:45:12]

On 2015/09/09 at 16:10:36, kjellander wrote:
> First of all, you need to create a CL using a clone of
https://chromium.googlesource.com/external/webrtc - the directories in a
Chromium checkout are mirrors that are read-only so they cannot be committed to
and the WebRTC standalone trybots cannot try the patches.
> 
> Secondly, I don't think this is correct - most of our code actually builds  on
iOS and we don't have anything similar to these changes in our GYP files.

Chrome on iOS only depends on webrtc/base:rtc_base (aka
//third_party/base:rtc_base).

It looks like some of the targets in //third_party/webrtc have trouble with iOS
(at least when trying to build Chrome on iOS), as we get the following errors
running "gn gen" (once the "all_dependent_configs" issue is fixed):

$ gn gen out/Default
ERROR at //third_party/opus/BUILD.gn:241:14: Assignment had no effect.
    cflags = [ "-Wno-nonnull" ]
             ^----------------
You set the variable "cflags" here and it was unused before it went
out of scope.
See //third_party/opus/BUILD.gn:225:1: whence it was called.
test("test_opus_decode") {
^-------------------------

We didn't try to dig deeper because it would meant fixing targets that were not
used by Chrome on iOS. However, if this code is supposed to be build on iOS for
other project, we can probably go deeper and fix those too.

[kjellander_webrtc, 2015-09-09 19:31:57]

On 2015/09/09 16:45:12, sdefresne wrote:
> On 2015/09/09 at 16:10:36, kjellander wrote:
> > First of all, you need to create a CL using a clone of
> https://chromium.googlesource.com/external/webrtc - the directories in a
> Chromium checkout are mirrors that are read-only so they cannot be committed
to
> and the WebRTC standalone trybots cannot try the patches.
> > 
> > Secondly, I don't think this is correct - most of our code actually builds 
on
> iOS and we don't have anything similar to these changes in our GYP files.
> 
> Chrome on iOS only depends on webrtc/base:rtc_base (aka
> //third_party/base:rtc_base).
> 
> It looks like some of the targets in //third_party/webrtc have trouble with
iOS
> (at least when trying to build Chrome on iOS), as we get the following errors
> running "gn gen" (once the "all_dependent_configs" issue is fixed):
> 
> $ gn gen out/Default
> ERROR at //third_party/opus/BUILD.gn:241:14: Assignment had no effect.
>     cflags = [ "-Wno-nonnull" ]
>              ^----------------
> You set the variable "cflags" here and it was unused before it went
> out of scope.
> See //third_party/opus/BUILD.gn:225:1: whence it was called.
> test("test_opus_decode") {
> ^-------------------------
> 
> We didn't try to dig deeper because it would meant fixing targets that were
not
> used by Chrome on iOS. However, if this code is supposed to be build on iOS
for
> other project, we can probably go deeper and fix those too.

I can take a look at getting the iOS build working for WebRTC instead. We
haven't been doing any work on that since I haven't heard much about the
roadmap, but I guess you're working on fixing so it can actually be run now?

[Dirk Pranke, 2015-09-09 19:35:19]

> I can take a look at getting the iOS build working for WebRTC instead. We
> haven't been doing any work on that since I haven't heard much about the
> roadmap, but I guess you're working on fixing so it can actually be run now?

Yup, progress is being made!

[kjellander_webrtc, 2015-09-09 20:11:32]

On 2015/09/09 19:35:19, Dirk Pranke wrote:
> > I can take a look at getting the iOS build working for WebRTC instead. We
> > haven't been doing any work on that since I haven't heard much about the
> > roadmap, but I guess you're working on fixing so it can actually be run now?
> 
> Yup, progress is being made!

CL for WebRTC up for review at https://codereview.webrtc.org/1309663007/
A separate CL for Opus is needed too (for the repo at
https://chromium.googlesource.com/chromium/src/+/master/third_party/opus/)

[kjellander_webrtc, 2015-09-09 20:23:47]

On 2015/09/09 20:11:32, kjellander (webrtc) wrote:
> On 2015/09/09 19:35:19, Dirk Pranke wrote:
> > > I can take a look at getting the iOS build working for WebRTC instead. We
> > > haven't been doing any work on that since I haven't heard much about the
> > > roadmap, but I guess you're working on fixing so it can actually be run
now?
> > 
> > Yup, progress is being made!
> 
> CL for WebRTC up for review at https://codereview.webrtc.org/1309663007/

This is now landed. Notice that the WebRTC revision needs to be rolled into
Chromium DEPS before it actually has an effect from a Chromium perspective.

> A separate CL for Opus is needed too (for the repo at
> https://chromium.googlesource.com/chromium/src/+/master/third_party/opus/)

Actually no opus fix is needed, it was a bug in build/config/ios/rules.gni which
Dirk kindly fixed in https://codereview.chromium.org/1314413006/

[sdefresne, 2015-09-10 08:56:46]

On 2015/09/09 at 20:23:47, kjellander wrote:
> On 2015/09/09 20:11:32, kjellander (webrtc) wrote:
> > On 2015/09/09 19:35:19, Dirk Pranke wrote:
> > > > I can take a look at getting the iOS build working for WebRTC instead.
We
> > > > haven't been doing any work on that since I haven't heard much about the
> > > > roadmap, but I guess you're working on fixing so it can actually be run
now?
> > > 
> > > Yup, progress is being made!
> > 
> > CL for WebRTC up for review at https://codereview.webrtc.org/1309663007/
> 
> This is now landed. Notice that the WebRTC revision needs to be rolled into
Chromium DEPS before it actually has an effect from a Chromium perspective.

Right. I'll check whether the roll is automated or manual and try to get it
rolled if it is the later.

> > A separate CL for Opus is needed too (for the repo at
> > https://chromium.googlesource.com/chromium/src/+/master/third_party/opus/)
> 
> Actually no opus fix is needed, it was a bug in build/config/ios/rules.gni
which Dirk kindly fixed in https://codereview.chromium.org/1314413006/

Thank you all for taking care of fixing webrtc for Chrome on iOS.

[kjellander_webrtc, 2015-09-10 09:16:01]

On 2015/09/10 08:56:46, sdefresne wrote:
> On 2015/09/09 at 20:23:47, kjellander wrote:
> > On 2015/09/09 20:11:32, kjellander (webrtc) wrote:
> > > On 2015/09/09 19:35:19, Dirk Pranke wrote:
> > > > > I can take a look at getting the iOS build working for WebRTC instead.
> We
> > > > > haven't been doing any work on that since I haven't heard much about
the
> > > > > roadmap, but I guess you're working on fixing so it can actually be
run
> now?
> > > > 
> > > > Yup, progress is being made!
> > > 
> > > CL for WebRTC up for review at https://codereview.webrtc.org/1309663007/
> > 
> > This is now landed. Notice that the WebRTC revision needs to be rolled into
> Chromium DEPS before it actually has an effect from a Chromium perspective.
> 
> Right. I'll check whether the roll is automated or manual and try to get it
> rolled if it is the later.

It's manual but uses a script. However it is best our team does that. I'll
notify the sheriff responsible.

> > > A separate CL for Opus is needed too (for the repo at
> > > https://chromium.googlesource.com/chromium/src/+/master/third_party/opus/)
> > 
> > Actually no opus fix is needed, it was a bug in build/config/ios/rules.gni
> which Dirk kindly fixed in https://codereview.chromium.org/1314413006/
> 
> Thank you all for taking care of fixing webrtc for Chrome on iOS.

No problemo!

[kjellander_webrtc, 2015-09-10 09:37:03]

On 2015/09/10 09:16:01, kjellander (webrtc) wrote:
> On 2015/09/10 08:56:46, sdefresne wrote:
> > On 2015/09/09 at 20:23:47, kjellander wrote:
> > > On 2015/09/09 20:11:32, kjellander (webrtc) wrote:
> > > > On 2015/09/09 19:35:19, Dirk Pranke wrote:
> > > > > > I can take a look at getting the iOS build working for WebRTC
instead.
> > We
> > > > > > haven't been doing any work on that since I haven't heard much about
> the
> > > > > > roadmap, but I guess you're working on fixing so it can actually be
> run
> > now?
> > > > > 
> > > > > Yup, progress is being made!
> > > > 
> > > > CL for WebRTC up for review at https://codereview.webrtc.org/1309663007/
> > > 
> > > This is now landed. Notice that the WebRTC revision needs to be rolled
into
> > Chromium DEPS before it actually has an effect from a Chromium perspective.
> > 
> > Right. I'll check whether the roll is automated or manual and try to get it
> > rolled if it is the later.
> 
> It's manual but uses a script. However it is best our team does that. I'll
> notify the sheriff responsible.

A roll including this is already in progress:
https://codereview.chromium.org/1335523002
It should land shortly.

> 
> > > > A separate CL for Opus is needed too (for the repo at
> > > >
https://chromium.googlesource.com/chromium/src/+/master/third_party/opus/)
> > > 
> > > Actually no opus fix is needed, it was a bug in build/config/ios/rules.gni
> > which Dirk kindly fixed in https://codereview.chromium.org/1314413006/
> > 
> > Thank you all for taking care of fixing webrtc for Chrome on iOS.
> 
> No problemo!

[sdefresne, 2015-09-10 10:11:02]

On 2015/09/10 at 09:37:03, kjellander wrote:
> On 2015/09/10 09:16:01, kjellander (webrtc) wrote:
> > On 2015/09/10 08:56:46, sdefresne wrote:
> > > On 2015/09/09 at 20:23:47, kjellander wrote:
> > > > On 2015/09/09 20:11:32, kjellander (webrtc) wrote:
> > > > > On 2015/09/09 19:35:19, Dirk Pranke wrote:
> > > > > > > I can take a look at getting the iOS build working for WebRTC
instead.
> > > We
> > > > > > > haven't been doing any work on that since I haven't heard much
about
> > the
> > > > > > > roadmap, but I guess you're working on fixing so it can actually
be
> > run
> > > now?
> > > > > > 
> > > > > > Yup, progress is being made!
> > > > > 
> > > > > CL for WebRTC up for review at
https://codereview.webrtc.org/1309663007/
> > > > 
> > > > This is now landed. Notice that the WebRTC revision needs to be rolled
into
> > > Chromium DEPS before it actually has an effect from a Chromium
perspective.
> > > 
> > > Right. I'll check whether the roll is automated or manual and try to get
it
> > > rolled if it is the later.
> > 
> > It's manual but uses a script. However it is best our team does that. I'll
> > notify the sheriff responsible.
> 
> A roll including this is already in progress:
https://codereview.chromium.org/1335523002
> It should land shortly.
> 
> > 
> > > > > A separate CL for Opus is needed too (for the repo at
> > > > >
https://chromium.googlesource.com/chromium/src/+/master/third_party/opus/)
> > > > 
> > > > Actually no opus fix is needed, it was a bug in
build/config/ios/rules.gni
> > > which Dirk kindly fixed in https://codereview.chromium.org/1314413006/
> > > 
> > > Thank you all for taking care of fixing webrtc for Chrome on iOS.
> > 
> > No problemo!

Great!