Provide HostView with data required for creating a new and session, and render for CRD iOS.

This CL is about plumbing the required information from the HostInfo objects and passing that to the session creation methods. It also cleans up responsibilities for which object holds and creates the rendering and client connections, pushing that into the client session rather than the view (from another CL, integrated here).

Also integrating with new rendering for CRD iOS for the gl rendering client.

Also making the FAB smaller on host view.

Moving https://codereview.chromium.org/2828113002/ to this cl.

BUG=671692
Review-Url: https://codereview.chromium.org/2828113002
Cr-Commit-Position: refs/heads/master@{#466797}
Committed: https://chromium.googlesource.com/chromium/src/+/a204e74f7bbea5269fd0b8ff221d5ea42f524552

[nicholss, 2017-04-19 23:50:10]

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

[commit-bot: I haz the power, 2017-04-19 23:51:02]

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-19 23:57:24]

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

[commit-bot: I haz the power, 2017-04-19 23:57:25]

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-...)
  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-21 16:59:06]

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

[commit-bot: I haz the power, 2017-04-21 16:59:25]

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-21 17:05:05]

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

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

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  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-21 21:23:12]

On 2017/04/21 17:05:05, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
>   ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
>   ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

This cl is dependent on https://codereview.chromium.org/2827163004/

[nicholss, 2017-04-24 15:49:12]

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

[commit-bot: I haz the power, 2017-04-24 15:49:59]

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-24 15:53:38]

Description was changed from

==========
Integrating with new rendering and session for CRD iOS.

BUG=671692
==========

to

==========
This CL is about plumbing the required information from the HostInfo objects and
passing that to the session creation methods. It also cleans up responsibilities
for which object holds and creates the rendering and client connections, pushing
that into the client session rather than the view (from another CL, integrated
here).

BUG=671692
==========

[nicholss, 2017-04-24 15:53:38]

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

[commit-bot: I haz the power, 2017-04-24 15:55:08]

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

[commit-bot: I haz the power, 2017-04-24 15:55:09]

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-...)
  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-24 16:06:37]

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

[commit-bot: I haz the power, 2017-04-24 16:07:05]

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-24 16:12:55]

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

[commit-bot: I haz the power, 2017-04-24 16:12:56]

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-...)
  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-24 16:35:23]

Description was changed from

==========
This CL is about plumbing the required information from the HostInfo objects and
passing that to the session creation methods. It also cleans up responsibilities
for which object holds and creates the rendering and client connections, pushing
that into the client session rather than the view (from another CL, integrated
here).

BUG=671692
==========

to

==========
This CL is about plumbing the required information from the HostInfo objects and
passing that to the session creation methods. It also cleans up responsibilities
for which object holds and creates the rendering and client connections, pushing
that into the client session rather than the view (from another CL, integrated
here).

Also integrating with new rendering for CRD iOS for the gl rendering client.

Moving https://codereview.chromium.org/2828113002/ to this cl.

BUG=671692
==========

[nicholss, 2017-04-24 16:35:23]

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

[nicholss, 2017-04-24 16:38:45]

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

[commit-bot: I haz the power, 2017-04-24 16:39:13]

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-24 16:41:06]

I had to combine https://codereview.chromium.org/2828113002 and this CL into one
to avoid circular change dependencies in this few components. They all
interconnect, so I mushed them together here.

PTAL, thanks! the GL and Service code did not change.

[commit-bot: I haz the power, 2017-04-24 16:44:52]

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

[commit-bot: I haz the power, 2017-04-24 16:44:53]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  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-24 17:00:16]

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

[commit-bot: I haz the power, 2017-04-24 17:00:48]

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-24 18:03:59]

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

[commit-bot: I haz the power, 2017-04-24 18:04:00]

Dry run: This issue passed the CQ dry run.

[Yuwei, 2017-04-24 19:27:39]

https://codereview.chromium.org/2828113002/diff/80001/remoting/client/ios/app...
File remoting/client/ios/app/remoting_view_controller.mm (right):

https://codereview.chromium.org/2828113002/diff/80001/remoting/client/ios/app...
remoting/client/ios/app/remoting_view_controller.mm:138: completion:(void
(^)())completionBlock {
Is completionBlock not used here?

https://codereview.chromium.org/2828113002/diff/80001/remoting/client/ios/dis...
File remoting/client/ios/display/gl_display_handler.mm (right):

https://codereview.chromium.org/2828113002/diff/80001/remoting/client/ios/dis...
remoting/client/ios/display/gl_display_handler.mm:118: std::array<float, 9>
matrix = {{1, 0, 0,
I think you should sync with ToT?

[nicholss, 2017-04-24 19:51:09]

On 2017/04/24 16:41:06, nicholss wrote:
> I had to combine https://codereview.chromium.org/2828113002 and this CL into
one
> to avoid circular change dependencies in this few components. They all
> interconnect, so I mushed them together here.
> 
> PTAL, thanks! the GL and Service code did not change.

Typo, I mean I had to combine https://codereview.chromium.org/2825403004/ with
this CL.

[nicholss, 2017-04-24 20:41:32]

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

[nicholss, 2017-04-24 20:41:52]

https://codereview.chromium.org/2828113002/diff/80001/remoting/client/ios/app...
File remoting/client/ios/app/remoting_view_controller.mm (right):

https://codereview.chromium.org/2828113002/diff/80001/remoting/client/ios/app...
remoting/client/ios/app/remoting_view_controller.mm:138: completion:(void
(^)())completionBlock {
On 2017/04/24 19:27:39, Yuwei wrote:
> Is completionBlock not used here?

it is suppose to be! Fixed.

https://codereview.chromium.org/2828113002/diff/80001/remoting/client/ios/dis...
File remoting/client/ios/display/gl_display_handler.mm (right):

https://codereview.chromium.org/2828113002/diff/80001/remoting/client/ios/dis...
remoting/client/ios/display/gl_display_handler.mm:118: std::array<float, 9>
matrix = {{1, 0, 0,
On 2017/04/24 19:27:39, Yuwei wrote:
> I think you should sync with ToT?

I had to combine this cl with https://codereview.chromium.org/2825403004/

[commit-bot: I haz the power, 2017-04-24 20:42:40]

Dry run: CQ is trying da patch.

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

[Yuwei, 2017-04-24 20:58:07]

LGTM

[Jamie, 2017-04-24 21:54:11]

LGTM with nits.

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
File remoting/client/ios/app/host_view_controller.mm (right):

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
remoting/client/ios/app/host_view_controller.mm:14: static const CGFloat
kFabInset = 15.f;
This doesn't look related to the CL description. If it's deliberate, please add
a line to the description, otherwise it looks like something you accidentally
committed while playing with something unrelated. There's other FAB-related code
further down; if it's all related, consider putting it in a separate CL.

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
File remoting/client/ios/app/remoting_view_controller.mm (right):

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
remoting/client/ios/app/remoting_view_controller.mm:142:
callbackWithAccessToken:base::BindBlockArc(^(
Can you pull this block out into a variable? It's hard to read like this,
particularly given ObjC's strange function call syntax.

[nicholss, 2017-04-24 22:03:07]

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
File remoting/client/ios/app/host_view_controller.mm (right):

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
remoting/client/ios/app/host_view_controller.mm:14: static const CGFloat
kFabInset = 15.f;
On 2017/04/24 21:54:11, Jamie wrote:
> This doesn't look related to the CL description. If it's deliberate, please
add
> a line to the description, otherwise it looks like something you accidentally
> committed while playing with something unrelated. There's other FAB-related
code
> further down; if it's all related, consider putting it in a separate CL.

The host view has a FAB on it, in seeing the rendered desktop I thought the
existing button was too large so I made it smaller is all.

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
File remoting/client/ios/app/remoting_view_controller.mm (right):

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
remoting/client/ios/app/remoting_view_controller.mm:142:
callbackWithAccessToken:base::BindBlockArc(^(
On 2017/04/24 21:54:11, Jamie wrote:
> Can you pull this block out into a variable? It's hard to read like this,
> particularly given ObjC's strange function call syntax.

The blind blocks are suppose to be used inline like this, it uses cell from the
parent method context. I don't think it would help readability to define it and
then use it rather than just use it in place.

[Jamie, 2017-04-24 22:07:42]

lgtm

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
File remoting/client/ios/app/host_view_controller.mm (right):

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
remoting/client/ios/app/host_view_controller.mm:14: static const CGFloat
kFabInset = 15.f;
On 2017/04/24 22:03:07, nicholss wrote:
> On 2017/04/24 21:54:11, Jamie wrote:
> > This doesn't look related to the CL description. If it's deliberate, please
> add
> > a line to the description, otherwise it looks like something you
accidentally
> > committed while playing with something unrelated. There's other FAB-related
> code
> > further down; if it's all related, consider putting it in a separate CL.
> 
> The host view has a FAB on it, in seeing the rendered desktop I thought the
> existing button was too large so I made it smaller is all.

It should still at least be called out in the CL description, if only so your
reviewer doesn't spend time trying to work out how this line contributes to what
the CL is supposed to do.

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
File remoting/client/ios/app/remoting_view_controller.mm (right):

https://codereview.chromium.org/2828113002/diff/100001/remoting/client/ios/ap...
remoting/client/ios/app/remoting_view_controller.mm:142:
callbackWithAccessToken:base::BindBlockArc(^(
On 2017/04/24 22:03:07, nicholss wrote:
> On 2017/04/24 21:54:11, Jamie wrote:
> > Can you pull this block out into a variable? It's hard to read like this,
> > particularly given ObjC's strange function call syntax.
> 
> The blind blocks are suppose to be used inline like this, it uses cell from
the
> parent method context. I don't think it would help readability to define it
and
> then use it rather than just use it in place.

One advantage is that the name of the variable makes it clear what the callback
is supposed to do, but I don't feel strongly about it.

[nicholss, 2017-04-24 22:09:31]

Description was changed from

==========
This CL is about plumbing the required information from the HostInfo objects and
passing that to the session creation methods. It also cleans up responsibilities
for which object holds and creates the rendering and client connections, pushing
that into the client session rather than the view (from another CL, integrated
here).

Also integrating with new rendering for CRD iOS for the gl rendering client.

Moving https://codereview.chromium.org/2828113002/ to this cl.

BUG=671692
==========

to

==========
This CL is about plumbing the required information from the HostInfo objects and
passing that to the session creation methods. It also cleans up responsibilities
for which object holds and creates the rendering and client connections, pushing
that into the client session rather than the view (from another CL, integrated
here).

Also integrating with new rendering for CRD iOS for the gl rendering client.

Also making the FAB smaller on host view.

Moving https://codereview.chromium.org/2828113002/ to this cl.

BUG=671692
==========

[commit-bot: I haz the power, 2017-04-24 22:13:52]

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

[commit-bot: I haz the power, 2017-04-24 22:13:54]

Dry run: This issue passed the CQ dry run.

[nicholss, 2017-04-24 22:21:30]

The CQ bit was checked by nicholss@chromium.org

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

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-24 22:28:54]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493072490589660,
"parent_rev": "b374042e98055fbf1a709c451d76ca81679fe988", "commit_rev":
"a204e74f7bbea5269fd0b8ff221d5ea42f524552"}

[commit-bot: I haz the power, 2017-04-24 22:29:04]

Description was changed from

==========
This CL is about plumbing the required information from the HostInfo objects and
passing that to the session creation methods. It also cleans up responsibilities
for which object holds and creates the rendering and client connections, pushing
that into the client session rather than the view (from another CL, integrated
here).

Also integrating with new rendering for CRD iOS for the gl rendering client.

Also making the FAB smaller on host view.

Moving https://codereview.chromium.org/2828113002/ to this cl.

BUG=671692
==========

to

==========
This CL is about plumbing the required information from the HostInfo objects and
passing that to the session creation methods. It also cleans up responsibilities
for which object holds and creates the rendering and client connections, pushing
that into the client session rather than the view (from another CL, integrated
here).

Also integrating with new rendering for CRD iOS for the gl rendering client.

Also making the FAB smaller on host view.

Moving https://codereview.chromium.org/2828113002/ to this cl.

BUG=671692

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

[commit-bot: I haz the power, 2017-04-24 22:29:06]

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