New issue
Advanced search Search tips

Issue 758028 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Divine test infrastructure for integration tests of blink and the compositor

Project Member Reported by pdr@chromium.org, Aug 22 2017

Issue description

It's very difficult, and sometimes not possible, to test interactions between blink and the compositor. Here are some recent usecases that we would have liked to test:
1) In https://chromium-review.googlesource.com/c/chromium/src/+/627022 we needed to test an impl-generated scroll callback into blink. There is no way to change the arguments passed to ProxyMain::BeginMainFrame (begin_main_frame_state->scroll_info in this case) and see how blink's scrollable areas get called back.
2) There are few-to-no end-to-end composited scrolling tests that have an impl-side scroll event trigger a scroll offset change in blink.
3) There are few-to-no tests of url bar hiding and how it affects blink.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f212abdd8fefeea395d04f69fd140a299ed73bb

commit 8f212abdd8fefeea395d04f69fd140a299ed73bb
Author: Philip Rogers <pdr@chromium.org>
Date: Mon Aug 28 21:24:30 2017

Add a SPV2 test of scrollable area changes that affect cc::Layers

This patch adds a test with slimming paint V2 enabled where a scrollable
area is deleted and impl-side DidScroll callbacks to not crash. Before
[1], the DidScroll callback was on ScrollableArea and deleting a
ScrollableArea without updating compositing could lead to a crash if
an impl-side scroll tried to notify a deleted ScrollableArea.

[1] https://chromium.googlesource.com/chromium/src/+/e7b75b84514adffc3f0a494e82c7de22708b0ec6

Bug:  758028 ,  758360 , 747719
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I61489c5138c0b54b55d841f9ab9ff61fe5b9b3eb
Reviewed-on: https://chromium-review.googlesource.com/636056
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497879}
[modify] https://crrev.com/8f212abdd8fefeea395d04f69fd140a299ed73bb/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/8f212abdd8fefeea395d04f69fd140a299ed73bb/third_party/WebKit/Source/core/frame/LocalFrameView.h
[modify] https://crrev.com/8f212abdd8fefeea395d04f69fd140a299ed73bb/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/8f212abdd8fefeea395d04f69fd140a299ed73bb/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h

Comment 2 by pdr@chromium.org, Aug 28 2017

Cc: bokan@chromium.org
Status: Fixed (was: Assigned)
David, I think you may be able to use similar code to unittest root scroller and/or url bar hiding features. WebFrameTest is pretty much a test of the main frame side of both blink and the compositor (i.e., cc::Layers but not cc::LayerImpls).
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24a13a10bede5ea482343990bd14b72ca1d742c3

commit 24a13a10bede5ea482343990bd14b72ca1d742c3
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Aug 28 22:22:40 2017

Speculative revert "Add a SPV2 test of scrollable area changes that affect cc::Layers"

This reverts commit 8f212abdd8fefeea395d04f69fd140a299ed73bb.

Reason for revert: 
Breaks build:
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win%20x64%20Builder%20%28dbg%29/builds/112482

Original change's description:
> Add a SPV2 test of scrollable area changes that affect cc::Layers
> 
> This patch adds a test with slimming paint V2 enabled where a scrollable
> area is deleted and impl-side DidScroll callbacks to not crash. Before
> [1], the DidScroll callback was on ScrollableArea and deleting a
> ScrollableArea without updating compositing could lead to a crash if
> an impl-side scroll tried to notify a deleted ScrollableArea.
> 
> [1] https://chromium.googlesource.com/chromium/src/+/e7b75b84514adffc3f0a494e82c7de22708b0ec6
> 
> Bug:  758028 ,  758360 , 747719
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: I61489c5138c0b54b55d841f9ab9ff61fe5b9b3eb
> Reviewed-on: https://chromium-review.googlesource.com/636056
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Commit-Queue: Philip Rogers <pdr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#497879}

TBR=pdr@chromium.org,chrishtr@chromium.org

Change-Id: Ib72b249e2f81f42acda9d28263e37413fbae9fe1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  758028 ,  758360 , 747719
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/639166
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497899}
[modify] https://crrev.com/24a13a10bede5ea482343990bd14b72ca1d742c3/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/24a13a10bede5ea482343990bd14b72ca1d742c3/third_party/WebKit/Source/core/frame/LocalFrameView.h
[modify] https://crrev.com/24a13a10bede5ea482343990bd14b72ca1d742c3/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/24a13a10bede5ea482343990bd14b72ca1d742c3/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dfcf5438f0c1282eba7b4798ea30da5528bde235

commit dfcf5438f0c1282eba7b4798ea30da5528bde235
Author: pdr <pdr@chromium.org>
Date: Tue Aug 29 03:52:33 2017

[Reland] Add a SPV2 test of scrollable area changes that affect cc::Layers

This patch adds a test with slimming paint V2 enabled where a scrollable
area is deleted and impl-side DidScroll callbacks to not crash. Before
[1], the DidScroll callback was on ScrollableArea and deleting a
ScrollableArea without updating compositing could lead to a crash if
an impl-side scroll tried to notify a deleted ScrollableArea.

This patch is a reland of [2] which was rolled out in [3] for a compile
failure on Windows. This patch is equal to [2] except a PLATFORM_EXPORT
has been added in PaintArtifactCompositor.h.

[1] https://chromium.googlesource.com/chromium/src/+/e7b75b84514adffc3f0a494e82c7de22708b0ec6
[2] https://chromium.googlesource.com/chromium/src/+/8f212abdd8fefeea395d04f69fd140a299ed73bb
[3] https://chromium.googlesource.com/chromium/src/+/24a13a10bede5ea482343990bd14b72ca1d742c3

Bug:  758028 ,  758360 , 747719
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I15b92abada84eb84eed18d4f9fa2f0d093f90e33
Reviewed-on: https://chromium-review.googlesource.com/639750
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498015}
[modify] https://crrev.com/dfcf5438f0c1282eba7b4798ea30da5528bde235/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/dfcf5438f0c1282eba7b4798ea30da5528bde235/third_party/WebKit/Source/core/frame/LocalFrameView.h
[modify] https://crrev.com/dfcf5438f0c1282eba7b4798ea30da5528bde235/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/dfcf5438f0c1282eba7b4798ea30da5528bde235/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h

Comment 5 by bokan@chromium.org, Sep 5 2017

Cool! Though for URL bar and root scroller, there's a special paths through WebViewImpl::ResizeWithBrowserControls and WebViewImpl::ApplyViewportDeltas which I've been using in unit tests. I've also added some methods to Internals to let layout tests manipulate the URL bar which has helped greatly in improving test coverage. I now run a small suite in 
LayoutTests/virtual/android/fast/rootscroller/ and am working on extending that to more general viewport tests.

Sign in to add a comment