Make scrollWidth and scrollHeight match other DOM measurement APIs

Make scrollWidth and scrollHeight match other DOM measurement APIs

Use the same logic for scrollWidth and scrollHeight that's used for
offsetWidth and offsetHeight to avoid mis-reporting scrolling to
JavaScript. This has the sad effect of exposing a dependence on
truncating and scaling values in the WebView that's being preserved
with this change to avoid broader implications of this patch.

BUG=447902
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193218

[leviw_travelin_and_unemployed, 2015-03-13 20:58:40]

leviw@chromium.org changed reviewers:
+ eae@chromium.org

[eae, 2015-03-13 21:16:19]

eae@chromium.org changed reviewers:
+ rbyers@chromium.org

[eae, 2015-03-13 21:16:20]

LGTM

[leviw_travelin_and_unemployed, 2015-03-13 21:18:57]

The CQ bit was checked by leviw@chromium.org

[commit-bot: I haz the power, 2015-03-13 21:19:15]

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

[commit-bot: I haz the power, 2015-03-14 00:16:10]

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

[commit-bot: I haz the power, 2015-03-14 00:16:11]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/47369)

[leviw_travelin_and_unemployed, 2015-03-25 21:13:55]

PTAL... To recreate old behavior, we have to continue to truncate->scale. This
is shitty, but at a minimum scopes this change to JS only, not Chromium.

[eae, 2015-03-25 21:41:05]

LLGTM (looks less-good to me), with nit.

LGTM

https://codereview.chromium.org/1007013002/diff/40001/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1007013002/diff/40001/Source/web/WebViewImpl....
Source/web/WebViewImpl.cpp:3374: // int heightScaled =
static_cast<int>(document->documentElement()->scrollHeight() *
zoomLevelToZoomFactor(zoomLevel()));
Remove commented add code and please add a comment explaining why you truncate.

[Rick Byers, 2015-03-25 21:56:15]

LGTM (with eae's nit)

[Rick Byers, 2015-03-25 21:57:33]

On 2015/03/25 21:56:15, Rick Byers wrote:
> LGTM (with eae's nit)

BTW, the CL title made me think this was going to be a fix to
http://crbug.com/469835 (which could also be described this way).  Maybe add
something about truncation / rounding to the description?

[leviw_travelin_and_unemployed, 2015-03-25 22:13:54]

Was planning on adding a comment to the description after the ugliness...

I'm sad to have to make that change in WebViewImpl :(

[leviw_travelin_and_unemployed, 2015-03-25 22:14:45]

On 2015/03/25 at 21:41:05, eae wrote:
> LLGTM (looks less-good to me), with nit.
> 
> LGTM
> 
>
https://codereview.chromium.org/1007013002/diff/40001/Source/web/WebViewImpl.cpp
> File Source/web/WebViewImpl.cpp (right):
> 
>
https://codereview.chromium.org/1007013002/diff/40001/Source/web/WebViewImpl....
> Source/web/WebViewImpl.cpp:3374: // int heightScaled =
static_cast<int>(document->documentElement()->scrollHeight() *
zoomLevelToZoomFactor(zoomLevel()));
> Remove commented add code and please add a comment explaining why you
truncate.

Also, D'oh! I forgot to hit "save"! I had a comment and no commented out code :p

[leviw_travelin_and_unemployed, 2015-03-26 18:58:19]

The CQ bit was checked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-03-26 18:58:19]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org,
rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1007013002/#ps60001
(title: "Whoops")

[commit-bot: I haz the power, 2015-03-26 18:58:37]

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

[commit-bot: I haz the power, 2015-03-26 19:06:22]

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

[commit-bot: I haz the power, 2015-03-26 19:06:23]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/2...)

[leviw_travelin_and_unemployed, 2015-03-26 19:08:15]

leviw@chromium.org changed reviewers:
+ pdr@chromium.org

[leviw_travelin_and_unemployed, 2015-03-26 19:08:16]

+pdr for Source/web

[pdr., 2015-03-26 20:19:36]

LGTM

https://codereview.chromium.org/1007013002/diff/60001/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1007013002/diff/60001/Source/web/WebViewImpl....
Source/web/WebViewImpl.cpp:3373: // We truncate then zoom the height instead of
rounding to match the width behavior.
Can you add a fixme and file a separate bug for this?

[leviw_travelin_and_unemployed, 2015-04-03 21:37:37]

So... Moar changes in LayoutTextControlSingleLine. And more news on the changes
to WebViewImpl -- namely we don't want the changes. I didn't remove the change,
but instead will land it in a subsequent patch if folks are okay with that to
avoid conflating too many changes.

https://code.google.com/p/chromium/issues/detail?id=471336 covers the issue we
want the WebViewImpl changes removed for.

[pdr., 2015-04-03 21:56:53]

On 2015/04/03 at 21:37:37, leviw wrote:
> So... Moar changes in LayoutTextControlSingleLine. And more news on the
changes to WebViewImpl -- namely we don't want the changes. I didn't remove the
change, but instead will land it in a subsequent patch if folks are okay with
that to avoid conflating too many changes.
> 
> https://code.google.com/p/chromium/issues/detail?id=471336 covers the issue we
want the WebViewImpl changes removed for.

This is gross but pragmatic. LGTM

[leviw_travelin_and_unemployed, 2015-04-03 22:00:40]

The CQ bit was checked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-04-03 22:00:41]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org,
rbyers@chromium.org, pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/1007013002/#ps100001
(title: "Add link to crbug for WebViewImpl change")

[commit-bot: I haz the power, 2015-04-03 22:00:56]

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

[commit-bot: I haz the power, 2015-04-04 01:30:31]

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

[commit-bot: I haz the power, 2015-04-04 01:30:32]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/50490)

[leviw_travelin_and_unemployed, 2015-04-06 20:37:15]

The CQ bit was checked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-04-06 20:37:16]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
eae@chromium.org, rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1007013002/#ps120001
(title: "Add additional test expectation")

[commit-bot: I haz the power, 2015-04-06 20:37:26]

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

[commit-bot: I haz the power, 2015-04-06 21:56:57]

Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=193218