Ensure FrameView scrollbars are updated when changing device emulation modes

Ensure FrameView scrollbars are updated when changing device emulation modes

Device emulation enables visual viewport scrollbar behavior and
we need to ensure the FrameView's scrollbars are removed when
we begin emulating, and restored when we stop emulating.

BUG=680894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2695493003
Cr-Commit-Position: refs/heads/master@{#450093}
Committed: https://chromium.googlesource.com/chromium/src/+/cda5197c6532a6ff59171f0bbcb5b34e0652b3db

[pdr., 2017-02-11 03:21:34]

Description was changed from

==========
fix scrollbars

BUG=
==========

to

==========
fix scrollbars

BUG=680894
==========

[pdr., 2017-02-11 03:23:05]

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

[commit-bot: I haz the power, 2017-02-11 03:23:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pdr., 2017-02-11 04:21:15]

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

[commit-bot: I haz the power, 2017-02-11 04:21:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pdr., 2017-02-11 04:26:23]

Description was changed from

==========
fix scrollbars

BUG=680894
==========

to

==========
Ensure FrameView scrollbars are updated when changing device emulation modes

Device emulation enables visual viewport scrollbar behavior and
we need to ensure the FrameView's scrollbars are removed when
we begin emulating, and restored when we stop emulating.

BUG=680894
==========

[pdr., 2017-02-11 05:19:24]

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

[commit-bot: I haz the power, 2017-02-11 05:50:22]

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

[commit-bot: I haz the power, 2017-02-11 05:50:23]

Dry run: This issue passed the CQ dry run.

[pdr., 2017-02-11 19:42:25]

Hey David, Does this look like the correct fix? I still need to add a test.

[bokan, 2017-02-12 22:54:06]

seems sensible (and related to the CL I pushed your way last week) though I'm
OOO until the 21st. You can have skobes@ take a closer look if it's urgent or
wait till I get back. (PS. feel free to route these kinds of issues to me).

[pdr., 2017-02-13 18:45:06]

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

[pdr., 2017-02-13 18:45:10]

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

[commit-bot: I haz the power, 2017-02-13 18:45:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pdr., 2017-02-13 18:45:38]

Skobes, PTAL

[skobes, 2017-02-13 19:02:03]

lgtm % nits

https://codereview.chromium.org/2695493003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebViewTest.cpp (right):

https://codereview.chromium.org/2695493003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:4306: "  #forceScroll
{width:1px; height:3000px; }"
You can just set height on the body, no need for the div.

https://codereview.chromium.org/2695493003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:4314: if
(RuntimeEnabledFeatures::rootLayerScrollingEnabled()) {
Use:

  EXPECT_NE(nullptr,
frameView->layoutViewportScrollableArea()->verticalScrollbar());

(here and below)

[pdr., 2017-02-13 19:23:57]

On 2017/02/13 at 19:02:03, skobes wrote:
> lgtm % nits
> 
>
https://codereview.chromium.org/2695493003/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/web/tests/WebViewTest.cpp (right):
> 
>
https://codereview.chromium.org/2695493003/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/web/tests/WebViewTest.cpp:4306: "  #forceScroll
{width:1px; height:3000px; }"
> You can just set height on the body, no need for the div.
> 
>
https://codereview.chromium.org/2695493003/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/web/tests/WebViewTest.cpp:4314: if
(RuntimeEnabledFeatures::rootLayerScrollingEnabled()) {
> Use:
> 
>   EXPECT_NE(nullptr,
frameView->layoutViewportScrollableArea()->verticalScrollbar());
> 
> (here and below)

Good catches, done!

[pdr., 2017-02-13 19:24:36]

The CQ bit was checked by pdr@chromium.org

[pdr., 2017-02-13 19:24:37]

The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/2695493003/#ps80001
(title: "address comments")

[commit-bot: I haz the power, 2017-02-13 19:25:33]

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

[pdr., 2017-02-13 19:28:46]

Description was changed from

==========
Ensure FrameView scrollbars are updated when changing device emulation modes

Device emulation enables visual viewport scrollbar behavior and
we need to ensure the FrameView's scrollbars are removed when
we begin emulating, and restored when we stop emulating.

BUG=680894
==========

to

==========
Ensure FrameView scrollbars are updated when changing device emulation modes

Device emulation enables visual viewport scrollbar behavior and
we need to ensure the FrameView's scrollbars are removed when
we begin emulating, and restored when we stop emulating.

BUG=680894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[commit-bot: I haz the power, 2017-02-13 21:23:28]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487013876629770,
"parent_rev": "3e83267e6295ccac0663db8efbf44b3fbfaad272", "commit_rev":
"cda5197c6532a6ff59171f0bbcb5b34e0652b3db"}

[commit-bot: I haz the power, 2017-02-13 21:24:06]

Description was changed from

==========
Ensure FrameView scrollbars are updated when changing device emulation modes

Device emulation enables visual viewport scrollbar behavior and
we need to ensure the FrameView's scrollbars are removed when
we begin emulating, and restored when we stop emulating.

BUG=680894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Ensure FrameView scrollbars are updated when changing device emulation modes

Device emulation enables visual viewport scrollbar behavior and
we need to ensure the FrameView's scrollbars are removed when
we begin emulating, and restored when we stop emulating.

BUG=680894
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[commit-bot: I haz the power, 2017-02-13 21:24:07]

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