New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 774483 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug

Blocking:
issue 768876



Sign in to add a comment

Chromoting on iOS does not compile when the ios_deployment target is set to 10.0

Project Member Reported by noyau@chromium.org, Oct 13 2017

Issue description

Some of its dependencies are using deprecated API (for example OSAtomicIncrement32Barrier). Note that Chromium iOS intend to drop iOS 9 support in the M64 timeframe.  
 

Comment 1 by noyau@chromium.org, Oct 13 2017

Blocking: 768876
The dependency is //third_party/usrsctp.
Blockedon: 774504
Cc: yuweih@chromium.org nicho...@chromium.org rohitrao@chromium.org

Comment 5 by yuweih@chromium.org, Oct 13 2017

Given that the only error is deprecated API, could we just add "-Wno-deprecated-declarations" to the clang flag for now?

Comment 6 by yuweih@chromium.org, Oct 13 2017

Owner: yuweih@chromium.org
Status: Assigned (was: Untriaged)

Comment 7 by noyau@chromium.org, Oct 16 2017

I'm guessing that simply silencing the warning will probably mean that the app will crash when ran on iOS 10+ as the API is not available.

Comment 8 by yuweih@chromium.org, Oct 16 2017

Cc: noyau@chromium.org
Looks like the API is still available and works all the way up to deployment target 11.0.

    int* u = malloc(sizeof(int));
    *u = 123;
    OSAtomicAdd32Barrier(2, u);
    NSLog(@"u=%d", *u);  // u=125

In fact the usrsctp lib itself has bypassed that problem by suppressing the warning in their config.ac script. This doesn't fix our issue because we have our own build rules (which doesn't have that "-Wno-deprecated-declarations" flag).
https://github.com/sctplab/usrsctp/pull/129

I've filed a bug (https://github.com/sctplab/usrsctp/issues/177) on their repo to move to stdatomic. It has got assigned but I don't know how much time is needed for them to fix that. 

I would still prefer suppressing the warning in usrsctp for now. WDYT?
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/95d5777b7996229e3be7fec22687e0ab97cb8545

commit 95d5777b7996229e3be7fec22687e0ab97cb8545
Author: Eric Noyau <noyau@google.com>
Date: Tue Oct 17 15:31:39 2017

Disable remoting if the ios deployment target is greater than 9

Bug:  774483 
Change-Id: Ieba917e5b110230f77c54a84aca749eb5642d233
Reviewed-on: https://chromium-review.googlesource.com/718998
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Commit-Queue: Eric Noyau <noyau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509392}
[modify] https://crrev.com/95d5777b7996229e3be7fec22687e0ab97cb8545/remoting/remoting_enable.gni

WebRTC uses the same deprecated API. I've filed a bug and will solve that:

https://bugs.chromium.org/p/webrtc/issues/detail?id=8413
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/29f2a1b98aff4b72dd2fc8a33469a9b59f720765

commit 29f2a1b98aff4b72dd2fc8a33469a9b59f720765
Author: Yuwei Huang <yuweih@chromium.org>
Date: Wed Oct 18 22:50:21 2017

Suppress warning of use of deprecated OSAtomic* API in usrsctp

Chromoting is planning to bump up iOS deployment target to 10.0. However,
one of our dependency, //third_party/usrsctp has used some OSAtomic*
functions that have been deprecated on 10.0 SDK.

After some discussions with the usrsctp people, we would like to suppress
the deprecation warning on usrsctp for now because:
* Replacing the OSAtomic* functions with atomic_fetch_add is nontrivial
  and requires replacing some int32_t with _Atomic(int32_t) globally in
  their codebase, which is challenging for an engineer who isn't familiar
  with it.
* The OSAtomic* API works all the way up to deployment target 11, if
  ignoring the warnings.
* Filed a bug in their repo but they don't plan to fix that soon.
* They have already suppressed the warning in their configure.ac file.
  This is replicating their change.

Bug:  774483 
Change-Id: Ie7f65edb767cc241a8b5fe7dc6074ef37387d89f
Reviewed-on: https://chromium-review.googlesource.com/726619
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Taylor Brandstetter <deadbeef@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509905}
[modify] https://crrev.com/29f2a1b98aff4b72dd2fc8a33469a9b59f720765/third_party/usrsctp/BUILD.gn

Warnings are suppressed on usrsctp. I'm working on resolving that in WebRTC, probably by using std::atomic<int32_t>.

Track WebRTC bug at:
https://bugs.chromium.org/p/webrtc/issues/detail?id=8413
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e641ba867642f2c55c001f5af19844c9034d1868

commit e641ba867642f2c55c001f5af19844c9034d1868
Author: Yuwei Huang <yuweih@chromium.org>
Date: Thu Oct 19 23:43:47 2017

[CRD iOS] Replace deprecated openURL

This CL replaces the deprecated openURL: call with
openURL:options:completionHandler:.

Bug:  774483 
Change-Id: Idf9414bbacc9e633c98f1190a6aee89e2d0ebacd
Reviewed-on: https://chromium-review.googlesource.com/728733
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510251}
[modify] https://crrev.com/e641ba867642f2c55c001f5af19844c9034d1868/remoting/ios/app/remoting_menu_view_controller.mm

OSAtomic* use in WebRTC has been fixed. I'll send a CL to reenable remoting on deployment level 10.
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4149db1acc57acd64686f42cc95317edb579e2ce

commit 4149db1acc57acd64686f42cc95317edb579e2ce
Author: Yuwei Huang <yuweih@chromium.org>
Date: Mon Oct 23 21:15:25 2017

[CRD iOS] Reenable remoting on iOS 10 deployment target

Reenable CRD iOS on iOS 10.0 since all blockers have been resolved.

Bug:  774483 
Change-Id: Ic3c5ce933e01bfb30fd09d2d92cfc788b57b68c2
Reviewed-on: https://chromium-review.googlesource.com/734180
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510915}
[modify] https://crrev.com/4149db1acc57acd64686f42cc95317edb579e2ce/remoting/remoting_enable.gni

Status: Fixed (was: Assigned)
Blockedon: -774504

Sign in to add a comment