Disable scrollbars on the root scroller when using visual viewport scrollbars.

Disable scrollbars on the root scroller when using visual viewport scrollbars.

On Android, the VisualViewport supplies the scrollbars for the root FrameView
and the FrameView blocks its scrollbars from being created. When setting a
different element is set as the document.rootScroller, this scrollbar
suppression behavior needs to occur as well to prevent two sets of scrollbars
from appearing.

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

Committed: https://crrev.com/f5a4f9d38de7403d172ad20f78c462048eae37bc
Cr-Commit-Position: refs/heads/master@{#432965}

[bokan, 2016-11-15 01:45:53]

Description was changed from

==========
Disable scrollbars on the root scroller when using visual viewport scrollbars.

On Android, the VisualViewport supplies the scrollbars for the root FrameView
and the FrameView blocks its scrollbars from being created. When setting a
different element is set as the document.rootScroller, this scrollbar
suppression behavior needs to occur as well to prevent two sets of scrollbars
from appearing.

BUG=505516
==========

to

==========
Disable scrollbars on the root scroller when using visual viewport scrollbars.

On Android, the VisualViewport supplies the scrollbars for the root FrameView
and the FrameView blocks its scrollbars from being created. When setting a
different element is set as the document.rootScroller, this scrollbar
suppression behavior needs to occur as well to prevent two sets of scrollbars
from appearing.

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

[bokan, 2016-11-15 01:50:21]

Description was changed from

==========
Disable scrollbars on the root scroller when using visual viewport scrollbars.

On Android, the VisualViewport supplies the scrollbars for the root FrameView
and the FrameView blocks its scrollbars from being created. When setting a
different element is set as the document.rootScroller, this scrollbar
suppression behavior needs to occur as well to prevent two sets of scrollbars
from appearing.

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

to

==========
Disable scrollbars on the root scroller when using visual viewport scrollbars.

On Android, the VisualViewport supplies the scrollbars for the root FrameView
and the FrameView blocks its scrollbars from being created. When setting a
different element is set as the document.rootScroller, this scrollbar
suppression behavior needs to occur as well to prevent two sets of scrollbars
from appearing.

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

[bokan, 2016-11-15 01:50:21]

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

[bokan, 2016-11-15 01:50:30]

ptal.

[bokan, 2016-11-15 12:46:15]

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

[commit-bot: I haz the power, 2016-11-15 12:46:30]

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, 2016-11-15 13:24:40]

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

[commit-bot: I haz the power, 2016-11-15 13:24:41]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[bokan, 2016-11-15 19:48:21]

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

[commit-bot: I haz the power, 2016-11-15 19:48:58]

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, 2016-11-15 20:00:48]

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

[commit-bot: I haz the power, 2016-11-15 20:00:49]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[skobes, 2016-11-15 20:18:44]

lgtm w/ nits

https://codereview.chromium.org/2501723003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.h (right):

https://codereview.chromium.org/2501723003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.h:919: bool
visualViewportSuppliesScrollbars();
Why can't this be const anymore?

https://codereview.chromium.org/2501723003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/2501723003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:1757:
layoutBox()->document().frameHost()->globalRootScrollerController();
You could just do frame->host() here.

[bokan, 2016-11-15 22:04:09]

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

[bokan, 2016-11-15 22:04:10]

+jbroman@ for platform OWNER

https://codereview.chromium.org/2501723003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.h (right):

https://codereview.chromium.org/2501723003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.h:919: bool
visualViewportSuppliesScrollbars();
On 2016/11/15 20:18:43, skobes wrote:
> Why can't this be const anymore?

Because of the check: root scroller == layoutViewportScrollableArea(),
layoutViewportScrollableArea can't be const because if RLS is disabled it
returns `this`. 

The solution would be to add a const overload of layoutViewportScrollableArea()
but we've decided long ago that the ship has sailed on const-correctness.

https://codereview.chromium.org/2501723003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right):

https://codereview.chromium.org/2501723003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:1757:
layoutBox()->document().frameHost()->globalRootScrollerController();
On 2016/11/15 20:18:44, skobes wrote:
> You could just do frame->host() here.

Done.

[jbroman, 2016-11-15 22:08:59]

platform rs lgtm

[bokan, 2016-11-15 22:11:45]

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

[commit-bot: I haz the power, 2016-11-15 22:12:29]

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, 2016-11-15 22:46:55]

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

[commit-bot: I haz the power, 2016-11-15 22:46:56]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[bokan, 2016-11-16 15:13:11]

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

[commit-bot: I haz the power, 2016-11-16 15:13:33]

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, 2016-11-16 16:50:03]

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

[commit-bot: I haz the power, 2016-11-16 16:50:04]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[bokan, 2016-11-16 23:01:20]

The CQ bit was checked by bokan@chromium.org

[bokan, 2016-11-16 23:01:20]

The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org,
jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/2501723003/#ps100001
(title: "Unset upstream")

[commit-bot: I haz the power, 2016-11-16 23:03:29]

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, 2016-11-16 23:50:31]

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

[commit-bot: I haz the power, 2016-11-16 23:50:32]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[bokan, 2016-11-17 00:39:49]

The CQ bit was checked by bokan@chromium.org

[bokan, 2016-11-17 00:39:49]

The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org,
jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/2501723003/#ps120001
(title: "Fix build after bad rebase")

[commit-bot: I haz the power, 2016-11-17 00:41:20]

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, 2016-11-17 02:44:37]

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

[commit-bot: I haz the power, 2016-11-17 02:44:38]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_compile_dbg on master.tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)
  android_cronet on master.tryserver.chromium.android (JOB_TIMED_OUT, no build
URL)
  cast_shell_android on master.tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)

[bokan, 2016-11-17 13:29:03]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2016-11-17 13:29:22]

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, 2016-11-17 14:18:35]

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

[commit-bot: I haz the power, 2016-11-17 14:18:38]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[bokan, 2016-11-17 17:21:56]

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

[bokan, 2016-11-17 17:27:49]

victorhsieh@: Adding you from blame in the failing test.

My change is causing the OutOfProcessPPAPITest.Graphics2D test to fail. This
patch might change some paint invalidation timing in Blink, particularly around
overlay scrollbars. I suspect that the problem is that the invalidation was
forcing a flush to happen. Adding a flush to happen before the draw command
seems to fix this but I have no idea how this test works or anything around
PPAPI so it'd be reassuring if you could confirm this is the right way to fix
it. Thanks!

[bokan, 2016-11-17 17:27:57]

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

[commit-bot: I haz the power, 2016-11-17 17:28:32]

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

[victorhsieh, 2016-11-17 17:37:13]

On 2016/11/17 17:27:49, bokan wrote:
> victorhsieh@: Adding you from blame in the failing test.
> 
> My change is causing the OutOfProcessPPAPITest.Graphics2D test to fail. This
> patch might change some paint invalidation timing in Blink, particularly
around
> overlay scrollbars. I suspect that the problem is that the invalidation was
> forcing a flush to happen. Adding a flush to happen before the draw command
> seems to fix this but I have no idea how this test works or anything around
> PPAPI so it'd be reassuring if you could confirm this is the right way to fix
> it. Thanks!

Sorry, I do remember I did something to Graphics2D offscreen, but as I only
worked on PPAPI for a short period, I don't remember the details :(

I'm not sure if bbudge@ is still the maintainer, but he might be help to chime
in.

[bbudge, 2016-11-17 18:07:36]

bbudge@chromium.org changed reviewers:
+ bbudge@chromium.org

[bbudge, 2016-11-17 18:07:37]

ppapi/tests lgtm

[commit-bot: I haz the power, 2016-11-17 18:53:02]

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

[commit-bot: I haz the power, 2016-11-17 18:53:03]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[bokan, 2016-11-17 18:53:34]

The CQ bit was checked by bokan@chromium.org

[bokan, 2016-11-17 18:53:34]

The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org,
jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/2501723003/#ps140001
(title: "Fix (?) Graphics2D test")

[commit-bot: I haz the power, 2016-11-17 18:54:29]

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, 2016-11-17 19:31:00]

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

[commit-bot: I haz the power, 2016-11-17 19:31:01]

Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[bokan, 2016-11-17 20:07:38]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2016-11-17 20:08:53]

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, 2016-11-17 21:36:50]

Description was changed from

==========
Disable scrollbars on the root scroller when using visual viewport scrollbars.

On Android, the VisualViewport supplies the scrollbars for the root FrameView
and the FrameView blocks its scrollbars from being created. When setting a
different element is set as the document.rootScroller, this scrollbar
suppression behavior needs to occur as well to prevent two sets of scrollbars
from appearing.

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

to

==========
Disable scrollbars on the root scroller when using visual viewport scrollbars.

On Android, the VisualViewport supplies the scrollbars for the root FrameView
and the FrameView blocks its scrollbars from being created. When setting a
different element is set as the document.rootScroller, this scrollbar
suppression behavior needs to occur as well to prevent two sets of scrollbars
from appearing.

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

[commit-bot: I haz the power, 2016-11-17 21:36:51]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-11-17 21:40:54]

Description was changed from

==========
Disable scrollbars on the root scroller when using visual viewport scrollbars.

On Android, the VisualViewport supplies the scrollbars for the root FrameView
and the FrameView blocks its scrollbars from being created. When setting a
different element is set as the document.rootScroller, this scrollbar
suppression behavior needs to occur as well to prevent two sets of scrollbars
from appearing.

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

to

==========
Disable scrollbars on the root scroller when using visual viewport scrollbars.

On Android, the VisualViewport supplies the scrollbars for the root FrameView
and the FrameView blocks its scrollbars from being created. When setting a
different element is set as the document.rootScroller, this scrollbar
suppression behavior needs to occur as well to prevent two sets of scrollbars
from appearing.

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

Committed: https://crrev.com/f5a4f9d38de7403d172ad20f78c462048eae37bc
Cr-Commit-Position: refs/heads/master@{#432965}
==========

[commit-bot: I haz the power, 2016-11-17 21:40:56]

Patchset 8 (id:??) landed as
https://crrev.com/f5a4f9d38de7403d172ad20f78c462048eae37bc
Cr-Commit-Position: refs/heads/master@{#432965}