[DevTools] Try to adjust device mode scale to avoid rounding errors.

[DevTools] Try to adjust device mode scale to avoid rounding errors.

If not possible, fallback to faithful emulation with correct size.

BUG=599511
Committed: https://crrev.com/128414e543648a361e5791cde27e11f6d399d19a
Cr-Commit-Position: refs/heads/master@{#400764}

[dgozman, 2016-06-01 00:48:41]

dgozman@chromium.org changed reviewers:
+ lushnikov@chromium.org

[dgozman, 2016-06-01 00:48:41]

Take a look please.

[lushnikov, 2016-06-02 03:40:58]

https://codereview.chromium.org/2020083004/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeModel.js
(right):

https://codereview.chromium.org/2020083004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeModel.js:445:
if (this._type !== WebInspector.DeviceModeModel.Type.Device || !this._mode)
how could this._type === WebInspector.DeviceModeModel.Type.Device, but
!this.mode?

AFAIU this._type === Device implies some value for this._mode.

https://codereview.chromium.org/2020083004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeModel.js:513:
while (sharpScale > scale * 0.7) {
where does 0.7 come from?

https://codereview.chromium.org/2020083004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeModel.js:516:
sharp = sharp && this._isInteger((screenWidth - insetsWidth) * sharpScale /
100);
i don't follow this logic. Can you please explain? 

(i believe the real question is: what are insets and why do they affect scale?)

https://codereview.chromium.org/2020083004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeModel.js:530:
_isInteger: function(n)
There is a built-in: Number.isInteger(n)

[dgozman, 2016-06-03 17:47:45]

I'll address nits later. See replies.

https://codereview.chromium.org/2020083004/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeModel.js
(right):

https://codereview.chromium.org/2020083004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeModel.js:445:
if (this._type !== WebInspector.DeviceModeModel.Type.Device || !this._mode)
On 2016/06/02 03:40:58, lushnikov wrote:
> how could this._type === WebInspector.DeviceModeModel.Type.Device, but
> !this.mode?
> 
> AFAIU this._type === Device implies some value for this._mode.

Correct.

https://codereview.chromium.org/2020083004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeModel.js:513:
while (sharpScale > scale * 0.7) {
On 2016/06/02 03:40:58, lushnikov wrote:
> where does 0.7 come from?

Arbitrary number to not make scale too small. For example, when the real fit
scale if 0.49, we don't want to go down to 0.1.

https://codereview.chromium.org/2020083004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeModel.js:516:
sharp = sharp && this._isInteger((screenWidth - insetsWidth) * sharpScale /
100);
On 2016/06/02 03:40:58, lushnikov wrote:
> i don't follow this logic. Can you please explain? 
> 
> (i believe the real question is: what are insets and why do they affect
scale?)

We try to scale so that no rounding error will occur later. The actual emulated
size is (screenSize - insets), and it should stay integer.

[lushnikov, 2016-06-16 10:28:38]

lgtm

[dgozman, 2016-06-20 18:59:42]

https://codereview.chromium.org/2020083004/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeModel.js
(right):

https://codereview.chromium.org/2020083004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeModel.js:530:
_isInteger: function(n)
On 2016/06/02 03:40:58, lushnikov_ooo wrote:
> There is a built-in: Number.isInteger(n)

Done.

[dgozman, 2016-06-20 19:00:39]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2016-06-20 19:00:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
lushnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2020083004/#ps20001
(title: "fixed nits")

[commit-bot: I haz the power, 2016-06-20 19:01:47]

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

[commit-bot: I haz the power, 2016-06-20 20:26:23]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-20 20:28:44]

Description was changed from

==========
[DevTools] Try to adjust device mode scale to avoid rounding errors.

If not possible, fallback to faithful emulation with correct size.

BUG=599511
==========

to

==========
[DevTools] Try to adjust device mode scale to avoid rounding errors.

If not possible, fallback to faithful emulation with correct size.

BUG=599511

Committed: https://crrev.com/128414e543648a361e5791cde27e11f6d399d19a
Cr-Commit-Position: refs/heads/master@{#400764}
==========

[commit-bot: I haz the power, 2016-06-20 20:28:46]

Patchset 2 (id:??) landed as
https://crrev.com/128414e543648a361e5791cde27e11f6d399d19a
Cr-Commit-Position: refs/heads/master@{#400764}