[CRD iOS] Refactor an interface for RemotingAuthorization

[CRD iOS] Refactor an interface for RemotingAuthorization

This CL refactors an interface from RemotingAuthentication to allow private
implementation to authenticate using SSO. It also includes these changes:

* Change the bundle ID in branding_Chrome to com.google.ChromeRemoteDesktop to
  match the whitelisted ID for SSO.
* Disallow host refreshing when the current refresh is still in progress, since
  the new codepath may trigger the call to refresh host list twice.

BUG=734851
Review-Url: https://codereview.chromium.org/2949713002
Cr-Commit-Position: refs/heads/master@{#481389}
Committed: https://chromium.googlesource.com/chromium/src/+/c5bfa5e20bf9e0d983a6538d66ea36c7a98f2d72

[Yuwei, 2017-06-21 19:54:50]

Description was changed from

==========
[CRD iOS] Refactor an interface for RemotingAuthorization

BUG=734851
==========

to

==========
[CRD iOS] Refactor an interface for RemotingAuthorization

This CL refactors an interface from RemotingAuthentication to allow private
implementation to authenticate using SSO. It also includes these changes:

* Change the bundle ID in branding_Chrome to com.google.ChromeRemoteDesktop to
  match the whitelisted ID for SSO.
* Disallow host refreshing when the current refresh is still in progress.

BUG=734851
==========

[Yuwei, 2017-06-21 19:57:29]

Description was changed from

==========
[CRD iOS] Refactor an interface for RemotingAuthorization

This CL refactors an interface from RemotingAuthentication to allow private
implementation to authenticate using SSO. It also includes these changes:

* Change the bundle ID in branding_Chrome to com.google.ChromeRemoteDesktop to
  match the whitelisted ID for SSO.
* Disallow host refreshing when the current refresh is still in progress.

BUG=734851
==========

to

==========
[CRD iOS] Refactor an interface for RemotingAuthorization

This CL refactors an interface from RemotingAuthentication to allow private
implementation to authenticate using SSO. It also includes these changes:

* Change the bundle ID in branding_Chrome to com.google.ChromeRemoteDesktop to
  match the whitelisted ID for SSO.
* Disallow host refreshing when the current refresh is still in progress, since
  the new codepath may trigger the call to refresh host list twice.

BUG=734851
==========

[Yuwei, 2017-06-21 19:57:45]

yuweih@chromium.org changed reviewers:
+ nicholss@chromium.org

[Yuwei, 2017-06-21 19:57:45]

PTAL thanks!

https://codereview.chromium.org/2949713002/diff/60001/remoting/branding_Chrome
File remoting/branding_Chrome (right):

https://codereview.chromium.org/2949713002/diff/60001/remoting/branding_Chrom...
remoting/branding_Chrome:4: IOS_BUNDLE_ID=com.google.ChromeRemoteDesktop
I think we should also change the bundle ID in branding_Chromium to
com.google.ChromeRemoteDesktop.dev?

[Yuwei, 2017-06-21 20:34:11]

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

[commit-bot: I haz the power, 2017-06-21 20:34:36]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nicholss, 2017-06-21 21:31:06]

It looks good in general but the added below comment applies. Thoughts?

https://codereview.chromium.org/2949713002/diff/60001/remoting/ios/app/app_de...
File remoting/ios/app/app_delegate.h (right):

https://codereview.chromium.org/2949713002/diff/60001/remoting/ios/app/app_de...
remoting/ios/app/app_delegate.h:10: #import
"remoting/ios/facade/remoting_authentication.h"
Why is this added ?

https://codereview.chromium.org/2949713002/diff/60001/remoting/ios/app/app_de...
File remoting/ios/app/app_delegate.mm (right):

https://codereview.chromium.org/2949713002/diff/60001/remoting/ios/app/app_de...
remoting/ios/app/app_delegate.mm:36:
setAuthentication:_appViewController.authentication];
This feels backwards to me. The view controller should not setup the
authentication and then set the service, the service should have this set and
then the app view controller should always use the service's authentication
object. I realized you are doing this because this is the hook we have to set
differences between chromium and private but still seems wrong. 

Maybe we do need a private vs chromium main.mm to help us inject things into the
singletons.

https://codereview.chromium.org/2949713002/diff/60001/remoting/ios/app/app_de...
remoting/ios/app/app_delegate.mm:47: if (![_appViewController.authentication
Could it be cleaner to wrap this method in something that means #ifdef
chromium_build rather than typechecking the auth provider?

[commit-bot: I haz the power, 2017-06-21 21:54:22]

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

[commit-bot: I haz the power, 2017-06-21 21:54:22]

Dry run: This issue passed the CQ dry run.

[Yuwei, 2017-06-21 22:39:26]

Patchset #5 (id:80001) has been deleted

[Yuwei, 2017-06-21 22:44:16]

PTAL thanks!

https://codereview.chromium.org/2949713002/diff/60001/remoting/ios/app/app_de...
File remoting/ios/app/app_delegate.h (right):

https://codereview.chromium.org/2949713002/diff/60001/remoting/ios/app/app_de...
remoting/ios/app/app_delegate.h:10: #import
"remoting/ios/facade/remoting_authentication.h"
On 2017/06/21 21:31:06, nicholss wrote:
> Why is this added ?

Deleted. Just some remnant I forgot to clean up...

https://codereview.chromium.org/2949713002/diff/60001/remoting/ios/app/app_de...
File remoting/ios/app/app_delegate.mm (right):

https://codereview.chromium.org/2949713002/diff/60001/remoting/ios/app/app_de...
remoting/ios/app/app_delegate.mm:36:
setAuthentication:_appViewController.authentication];
On 2017/06/21 21:31:06, nicholss wrote:
> This feels backwards to me. The view controller should not setup the
> authentication and then set the service, the service should have this set and
> then the app view controller should always use the service's authentication
> object. I realized you are doing this because this is the hook we have to set
> differences between chromium and private but still seems wrong. 
> 
> Maybe we do need a private vs chromium main.mm to help us inject things into
the
> singletons.

Done. It's now injected in the main function.

https://codereview.chromium.org/2949713002/diff/60001/remoting/ios/app/app_de...
remoting/ios/app/app_delegate.mm:47: if (![_appViewController.authentication
On 2017/06/21 21:31:06, nicholss wrote:
> Could it be cleaner to wrap this method in something that means #ifdef
> chromium_build rather than typechecking the auth provider?

Done. Use marco here. I put the DCHECK here just to be safe.

https://codereview.chromium.org/2949713002/diff/100001/remoting/branding_Chro...
File remoting/branding_Chromium (right):

https://codereview.chromium.org/2949713002/diff/100001/remoting/branding_Chro...
remoting/branding_Chromium:4: IOS_BUNDLE_ID=com.google.ChromeRemoteDesktop.dev
Note that you'll need to manually remove the old org.chromium.Chromium app :-/

[nicholss, 2017-06-21 22:53:22]

lgtm 

That looks much nicer. Thanks for reworking it.

[Yuwei, 2017-06-21 23:04:13]

On 2017/06/21 22:53:22, nicholss wrote:
> lgtm 
> 
> That looks much nicer. Thanks for reworking it.

Thanks! Checking-in

[Yuwei, 2017-06-21 23:04:16]

The CQ bit was checked by yuweih@chromium.org

[commit-bot: I haz the power, 2017-06-21 23:04:47]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Yuwei, 2017-06-22 00:21:52]

The CQ bit was unchecked by yuweih@chromium.org

[Yuwei, 2017-06-22 00:35:16]

The CQ bit was checked by yuweih@chromium.org

[commit-bot: I haz the power, 2017-06-22 00:35:31]

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-06-22 01:14:47]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1498091716884840,
"parent_rev": "6ea62d807419383be94e6e1230f148400aaa8bf1", "commit_rev":
"c5bfa5e20bf9e0d983a6538d66ea36c7a98f2d72"}

[commit-bot: I haz the power, 2017-06-22 01:15:01]

Description was changed from

==========
[CRD iOS] Refactor an interface for RemotingAuthorization

This CL refactors an interface from RemotingAuthentication to allow private
implementation to authenticate using SSO. It also includes these changes:

* Change the bundle ID in branding_Chrome to com.google.ChromeRemoteDesktop to
  match the whitelisted ID for SSO.
* Disallow host refreshing when the current refresh is still in progress, since
  the new codepath may trigger the call to refresh host list twice.

BUG=734851
==========

to

==========
[CRD iOS] Refactor an interface for RemotingAuthorization

This CL refactors an interface from RemotingAuthentication to allow private
implementation to authenticate using SSO. It also includes these changes:

* Change the bundle ID in branding_Chrome to com.google.ChromeRemoteDesktop to
  match the whitelisted ID for SSO.
* Disallow host refreshing when the current refresh is still in progress, since
  the new codepath may trigger the call to refresh host list twice.

BUG=734851

Review-Url: https://codereview.chromium.org/2949713002
Cr-Commit-Position: refs/heads/master@{#481389}
Committed:
https://chromium.googlesource.com/chromium/src/+/c5bfa5e20bf9e0d983a6538d66ea...
==========

[commit-bot: I haz the power, 2017-06-22 01:15:03]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/c5bfa5e20bf9e0d983a6538d66ea...