Pass settings model to ARDAppClient instead of individual settings.

Pass settings model to ARDAppClient instead of individual settings.

Moves settings model and related classes to code common for both iOS
and Mac.

BUG=webrtc:7177, webrtc:6494
Review-Url: https://codereview.webrtc.org/2770113004
Cr-Commit-Position: refs/heads/master@{#17408}
Committed: https://chromium.googlesource.com/external/webrtc/+/c4adacfb15cfd4269c7ca0f0857ddb9bef520182

[sakal, 2017-03-24 12:15:15]

sakal@webrtc.org changed reviewers:
+ denicija@webrtc.org, magjed@webrtc.org

[sakal, 2017-03-24 12:15:16]

PTAL

[daniela-webrtc, 2017-03-26 15:49:49]

Nicely done. This will be great once we migrate the audio settings as well. LGTM
with few comments that can be addressed if you see fit.

https://codereview.webrtc.org/2770113004/diff/1/webrtc/examples/objc/AppRTCMo...
File webrtc/examples/objc/AppRTCMobile/ARDAppClient.h (right):

https://codereview.webrtc.org/2770113004/diff/1/webrtc/examples/objc/AppRTCMo...
webrtc/examples/objc/AppRTCMobile/ARDAppClient.h:74:
usingSettings:(ARDSettingsModel *)settings
Nit: `using` is implied so you can remove the prefix. But up to you.

https://codereview.webrtc.org/2770113004/diff/1/webrtc/examples/objc/AppRTCMo...
File webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m (right):

https://codereview.webrtc.org/2770113004/diff/1/webrtc/examples/objc/AppRTCMo...
webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m:374:
usingSettings:[[ARDSettingsModel alloc] init]
Maybe passing nil here for the settings parameter would be better as it better
conveys the no-op nature of the settings here

https://codereview.webrtc.org/2770113004/diff/20001/webrtc/examples/objc/AppR...
File webrtc/examples/objc/AppRTCMobile/ARDAppClient.h (right):

https://codereview.webrtc.org/2770113004/diff/20001/webrtc/examples/objc/AppR...
webrtc/examples/objc/AppRTCMobile/ARDAppClient.h:17: 
Usually in header files (.h)

@class ARDSettingsModel 

and import in implementation files (.m/mm)

https://codereview.webrtc.org/2770113004/diff/20001/webrtc/examples/objc/AppR...
webrtc/examples/objc/AppRTCMobile/ARDAppClient.h:74:
usingSettings:(ARDSettingsModel *)settings
Document this parameter as well.

[magjed_webrtc, 2017-03-27 07:58:23]

lgtm

[sakal, 2017-03-27 13:34:24]

https://codereview.webrtc.org/2770113004/diff/1/webrtc/examples/objc/AppRTCMo...
File webrtc/examples/objc/AppRTCMobile/ARDAppClient.h (right):

https://codereview.webrtc.org/2770113004/diff/1/webrtc/examples/objc/AppRTCMo...
webrtc/examples/objc/AppRTCMobile/ARDAppClient.h:74:
usingSettings:(ARDSettingsModel *)settings
On 2017/03/26 15:49:48, daniela-webrtc wrote:
> Nit: `using` is implied so you can remove the prefix. But up to you.

Done.

https://codereview.webrtc.org/2770113004/diff/1/webrtc/examples/objc/AppRTCMo...
File webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m (right):

https://codereview.webrtc.org/2770113004/diff/1/webrtc/examples/objc/AppRTCMo...
webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m:374:
usingSettings:[[ARDSettingsModel alloc] init]
On 2017/03/26 15:49:48, daniela-webrtc wrote:
> Maybe passing nil here for the settings parameter would be better as it better
> conveys the no-op nature of the settings here

The reason I do it this way is that I want to use the default values defined by
ARDSettingsModel.

https://codereview.webrtc.org/2770113004/diff/20001/webrtc/examples/objc/AppR...
File webrtc/examples/objc/AppRTCMobile/ARDAppClient.h (right):

https://codereview.webrtc.org/2770113004/diff/20001/webrtc/examples/objc/AppR...
webrtc/examples/objc/AppRTCMobile/ARDAppClient.h:17: 
On 2017/03/26 15:49:49, daniela-webrtc wrote:
> Usually in header files (.h)
> 
> @class ARDSettingsModel 
> 
> and import in implementation files (.m/mm)

Done.

https://codereview.webrtc.org/2770113004/diff/20001/webrtc/examples/objc/AppR...
webrtc/examples/objc/AppRTCMobile/ARDAppClient.h:74:
usingSettings:(ARDSettingsModel *)settings
On 2017/03/26 15:49:49, daniela-webrtc wrote:
> Document this parameter as well.

Done.

[sakal, 2017-03-27 13:34:54]

The CQ bit was checked by sakal@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 13:35:03]

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

[commit-bot: I haz the power, 2017-03-27 13:37:50]

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

[commit-bot: I haz the power, 2017-03-27 13:37:51]

Dry run: Try jobs failed on following builders:
  ios64_sim_ios10_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios10_dbg/bui...)
  ios64_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios9_dbg/buil...)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/18953)

[daniela-webrtc, 2017-03-27 13:49:07]

LGTM

https://codereview.webrtc.org/2770113004/diff/1/webrtc/examples/objc/AppRTCMo...
File webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m (right):

https://codereview.webrtc.org/2770113004/diff/1/webrtc/examples/objc/AppRTCMo...
webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m:374:
usingSettings:[[ARDSettingsModel alloc] init]
On 2017/03/27 13:34:24, sakal wrote:
> On 2017/03/26 15:49:48, daniela-webrtc wrote:
> > Maybe passing nil here for the settings parameter would be better as it
better
> > conveys the no-op nature of the settings here
> 
> The reason I do it this way is that I want to use the default values defined
by
> ARDSettingsModel.

Acknowledged.

[sakal, 2017-03-27 15:04:59]

Patchset #4 (id:60001) has been deleted

[sakal, 2017-03-27 15:05:13]

The CQ bit was checked by sakal@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 15:05:22]

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

[commit-bot: I haz the power, 2017-03-27 15:28:30]

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

[commit-bot: I haz the power, 2017-03-27 15:28:31]

Dry run: This issue passed the CQ dry run.

[sakal, 2017-03-28 07:50:11]

The CQ bit was checked by sakal@webrtc.org

[sakal, 2017-03-28 07:50:11]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
denicija@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2770113004/#ps80001 (title:
"Fix test.")

[commit-bot: I haz the power, 2017-03-28 07:50:16]

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

[commit-bot: I haz the power, 2017-03-28 08:22:49]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490687411134470,
"parent_rev": "bcbaf746439291249b5688b239e6346caee90368", "commit_rev":
"c4adacfb15cfd4269c7ca0f0857ddb9bef520182"}

[commit-bot: I haz the power, 2017-03-28 08:22:52]

Description was changed from

==========
Pass settings model to ARDAppClient instead of individual settings.

Moves settings model and related classes to code common for both iOS
and Mac.

BUG=webrtc:7177,webrtc:6494
==========

to

==========
Pass settings model to ARDAppClient instead of individual settings.

Moves settings model and related classes to code common for both iOS
and Mac.

BUG=webrtc:7177,webrtc:6494

Review-Url: https://codereview.webrtc.org/2770113004
Cr-Commit-Position: refs/heads/master@{#17408}
Committed:
https://chromium.googlesource.com/external/webrtc/+/c4adacfb15cfd4269c7ca0f08...
==========

[commit-bot: I haz the power, 2017-03-28 08:22:53]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/external/webrtc/+/c4adacfb15cfd4269c7ca0f08...