Add a per-host desktopScale option, available via the JS console.

Add a per-host desktopScale option, available via the JS console.

This CL adds a setDesktopScale() method to ClientSession, allowing the JS console to be used to easily apply a new desktop scale factor, to be applied to the current session and all subsequent sessions to the current host from this client. If the connected host supports dynamic resize then desktopZoom changes will take effect immediately.

For example, a user with a Chromebook Pixel can set desktopScale to 2.0 to have a low-DPI host set to match the Pixel's physical resolution 1:1, rather than being set to match the logical resolution and up-scaled.

BUG=135089, 406100
TEST=Connect to Linux host w/ resize-to-client on & scale-to-fit *off*. Open Dev Console for app's main.html and run e.g. remoting.clientSession.setDesktopScale(scale) for different values of |scale|, and verify that: (1) |scale==0.5| gives a low-res desktop that is up-scaled to fill the client area. (2) on low-DPI devices, |scale==2.0| gives a normal-res desktop that is bigger than the client area. (3) on Pixel |scale==2.0| gives a desktop sized 1:1 to the device's native display resolution. (4) on Pixel |scale==3.0| gives a desktop scaled 1:1 to the device's native display resolution, and sized to be larger than the client area.

Committed: https://crrev.com/2553f80061f805ea47ac23b6978af7bcadee3265
Cr-Commit-Position: refs/heads/master@{#308490}

[Wez, 2014-12-14 06:51:39]

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

[Wez, 2014-12-14 06:51:40]

PTAL

[Jamie, 2014-12-15 18:39:06]

Please also add bug 406100, since I think this is implementing what's described
there.

https://codereview.chromium.org/761673008/diff/20001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_session.js (right):

https://codereview.chromium.org/761673008/diff/20001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_session.js:790: // Apply the new desktop zoom, if
resize to client is enabled.
Nit: resize-to-client

https://codereview.chromium.org/761673008/diff/20001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_session.js:1660: window.devicePixelRatio);
Don't you also need to apply the inverse scale to the plugin element? This will
only work if scale-to-fit is enabled, I think.

[Wez, 2014-12-15 21:49:42]

https://codereview.chromium.org/761673008/diff/20001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_session.js (right):

https://codereview.chromium.org/761673008/diff/20001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_session.js:790: // Apply the new desktop zoom, if
resize to client is enabled.
On 2014/12/15 18:39:06, Jamie wrote:
> Nit: resize-to-client

Done.

https://codereview.chromium.org/761673008/diff/20001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_session.js:1660: window.devicePixelRatio);
On 2014/12/15 18:39:06, Jamie wrote:
> Don't you also need to apply the inverse scale to the plugin element? This
will
> only work if scale-to-fit is enabled, I think.

That's a good question.

I've added code in updateDimensions() to include the desktopScale in the
hostPixelRatio, which will mean taking it into account, up to the limit of the
devicePixelRatio, i.e. we won't down-scale smaller than 1:1 host:device pixels.

[Jamie, 2014-12-15 23:18:55]

LGTM. If this CL fixes bug 406100, could you assign it to yourself and close it?
If not, please add comments to that bug indicating what the remaining work-items
are.

Also, if you have any thoughts on how to test this, let me know (though, as a
console-only feature, I don't think it's higher than a P2 for testing).

https://codereview.chromium.org/761673008/diff/40001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_session.js (right):

https://codereview.chromium.org/761673008/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_session.js:791: function(desktopScale) {
Nit: Looks like this will fit on the previous line.

[Wez, 2014-12-15 23:57:43]

On 2014/12/15 23:18:55, Jamie wrote:
> LGTM. If this CL fixes bug 406100, could you assign it to yourself and close
it?
> If not, please add comments to that bug indicating what the remaining
work-items
> are.
> 
> Also, if you have any thoughts on how to test this, let me know (though, as a
> console-only feature, I don't think it's higher than a P2 for testing).

I've added a TEST line to the CL description; let me know if you think that's
(in)sufficient.
 
>
https://codereview.chromium.org/761673008/diff/40001/remoting/webapp/crd/js/c...
> File remoting/webapp/crd/js/client_session.js (right):
> 
>
https://codereview.chromium.org/761673008/diff/40001/remoting/webapp/crd/js/c...
> remoting/webapp/crd/js/client_session.js:791: function(desktopScale) {
> Nit: Looks like this will fit on the previous line.

Thanks for spotting that.

[Wez, 2014-12-15 23:58:12]

The CQ bit was checked by wez@chromium.org

[commit-bot: I haz the power, 2014-12-15 23:59:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/761673008/60001

[Jamie, 2014-12-15 23:59:33]

On 2014/12/15 23:57:43, Wez wrote:
> On 2014/12/15 23:18:55, Jamie wrote:
> > LGTM. If this CL fixes bug 406100, could you assign it to yourself and close
> it?
> > If not, please add comments to that bug indicating what the remaining
> work-items
> > are.
> > 
> > Also, if you have any thoughts on how to test this, let me know (though, as
a
> > console-only feature, I don't think it's higher than a P2 for testing).
> 
> I've added a TEST line to the CL description; let me know if you think that's
> (in)sufficient.

I was thinking more along the lines of automated tests, but I can't think how
we'd come up with a robust test for this.

[Wez, 2014-12-16 00:07:04]

On 2014/12/15 23:59:33, Jamie wrote:
> On 2014/12/15 23:57:43, Wez wrote:
> > On 2014/12/15 23:18:55, Jamie wrote:
> > > LGTM. If this CL fixes bug 406100, could you assign it to yourself and
close
> > it?
> > > If not, please add comments to that bug indicating what the remaining
> > work-items
> > > are.
> > > 
> > > Also, if you have any thoughts on how to test this, let me know (though,
as
> a
> > > console-only feature, I don't think it's higher than a P2 for testing).
> > 
> > I've added a TEST line to the CL description; let me know if you think
that's
> > (in)sufficient.
> 
> I was thinking more along the lines of automated tests, but I can't think how
> we'd come up with a robust test for this.

I think we can come up w/ some simple automated tests that just check set the
desktop scale factor & client dimensions to specific values and then just check
that the plugin is set to the right dimensions by the app. WDYT?

[Jamie, 2014-12-16 00:14:37]

On 2014/12/16 00:07:04, Wez wrote:
> On 2014/12/15 23:59:33, Jamie wrote:
> > On 2014/12/15 23:57:43, Wez wrote:
> > > On 2014/12/15 23:18:55, Jamie wrote:
> > > > LGTM. If this CL fixes bug 406100, could you assign it to yourself and
> close
> > > it?
> > > > If not, please add comments to that bug indicating what the remaining
> > > work-items
> > > > are.
> > > > 
> > > > Also, if you have any thoughts on how to test this, let me know (though,
> as
> > a
> > > > console-only feature, I don't think it's higher than a P2 for testing).
> > > 
> > > I've added a TEST line to the CL description; let me know if you think
> that's
> > > (in)sufficient.
> > 
> > I was thinking more along the lines of automated tests, but I can't think
how
> > we'd come up with a robust test for this.
> 
> I think we can come up w/ some simple automated tests that just check set the
> desktop scale factor & client dimensions to specific values and then just
check
> that the plugin is set to the right dimensions by the app. WDYT?

The scale factor won't affect the size of the plugin, will it? I think we'd need
to intercept the resize messages being sent to the host. That should be
relatively straightforward once I've finished mocking the host-list.

[Wez, 2014-12-16 00:40:05]

You're right - we actually want to test both the size of the plugin *and*
the contents of the resize messages.

On 15 December 2014 at 16:14, <jamiewalch@chromium.org> wrote:
>
> On 2014/12/16 00:07:04, Wez wrote:
>
>> On 2014/12/15 23:59:33, Jamie wrote:
>> > On 2014/12/15 23:57:43, Wez wrote:
>> > > On 2014/12/15 23:18:55, Jamie wrote:
>> > > > LGTM. If this CL fixes bug 406100, could you assign it to yourself
>> and
>> close
>> > > it?
>> > > > If not, please add comments to that bug indicating what the
>> remaining
>> > > work-items
>> > > > are.
>> > > >
>> > > > Also, if you have any thoughts on how to test this, let me know
>> (though,
>> as
>> > a
>> > > > console-only feature, I don't think it's higher than a P2 for
>> testing).
>> > >
>> > > I've added a TEST line to the CL description; let me know if you think
>> that's
>> > > (in)sufficient.
>> >
>> > I was thinking more along the lines of automated tests, but I can't
>> think
>>
> how
>
>> > we'd come up with a robust test for this.
>>
>
>  I think we can come up w/ some simple automated tests that just check set
>> the
>> desktop scale factor & client dimensions to specific values and then just
>>
> check
>
>> that the plugin is set to the right dimensions by the app. WDYT?
>>
>
> The scale factor won't affect the size of the plugin, will it? I think
> we'd need
> to intercept the resize messages being sent to the host. That should be
> relatively straightforward once I've finished mocking the host-list.
>
> https://codereview.chromium.org/761673008/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-12-16 01:18:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-12-16 01:19:17]

Patchset 4 (id:??) landed as
https://crrev.com/2553f80061f805ea47ac23b6978af7bcadee3265
Cr-Commit-Position: refs/heads/master@{#308490}