Commit 943b8b0f authored by Mason Freed's avatar Mason Freed Committed by Commit Bot

Check the frame tree for each frame before compositing.

This fixes several clusterfuzz issues, which are caused by frames
being freed in between calls to CompositeWithRaster(). With this CL,
a fresh list of frames is requested and checked for the next one,
to avoid using deleted frames. Note that with this CL:
  https://chromium-review.googlesource.com/c/chromium/src/+/1252141
those calls are asynchronous. Also note that this CL:
  https://chromium-review.googlesource.com/c/chromium/src/+/1213864
caused this code to get called much more frequently, for all layout
tests.

Here are the clusterfuzz issues:
https://clusterfuzz.com/v2/testcase-detail/5701500434907136
https://clusterfuzz.com/v2/testcase-detail/4929420383748096
https://clusterfuzz.com/v2/testcase-detail/4996950557196288

Bug: 900087, 899465, 899450
Change-Id: I70fc7c723b2118f4796289fe9b7272c42b6e50e5
Reviewed-on: https://chromium-review.googlesource.com/c/1308038Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarvmpstr <vmpstr@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604274}
parent 6271fcdc
......@@ -670,6 +670,13 @@ void BlinkTestController::CompositeAllFramesThen(
void BlinkTestController::CompositeNodeQueueThen(
base::OnceCallback<void()> callback) {
// Frames can get freed somewhere else while a CompositeWithRaster is taking
// place. Therefore, we need to double-check that this frame pointer is
// still valid before using it. To do that, grab the list of all frames
// again, and make sure it contains the one we're about to composite.
// See crbug.com/899465 for an example of this problem.
std::vector<RenderFrameHost*> current_frames(
main_window_->web_contents()->GetAllFrames());
RenderFrameHost* next_node_host;
do {
if (composite_all_frames_node_queue_.empty()) {
......@@ -680,6 +687,10 @@ void BlinkTestController::CompositeNodeQueueThen(
next_node_host =
composite_all_frames_node_queue_.front()->render_frame_host;
composite_all_frames_node_queue_.pop();
if (std::find(current_frames.begin(), current_frames.end(),
next_node_host) == current_frames.end()) {
next_node_host = nullptr; // This one is now gone
}
} while (!next_node_host || !next_node_host->IsRenderFrameLive());
GetLayoutTestControlPtr(next_node_host)
->CompositeWithRaster(
......
......@@ -113,6 +113,20 @@ crbug.com/891427 virtual/threaded/animations/invisible-composited-animations-pre
# One more from Findit: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/220243
crbug.com/891427 rootscroller/set-root-scroller.html [ Pass Failure Timeout Crash ]
# These 6 are special. They are tied to both of these CLs:
# - https://chromium-review.googlesource.com/c/chromium/src/+/1213864
# - https://chromium-review.googlesource.com/c/chromium/src/+/1308038
# and they only seem (really) flaky when the second CL is in place. But I believe
# they are related to timing and to the same issues as above.
# Here is an example: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/221340
crbug.com/891427 fast/block/float/4145535Crash.html [ Pass Failure Timeout Crash ]
crbug.com/891427 virtual/layout_ng/fast/block/float/4145535Crash.html [ Pass Failure Timeout Crash ]
crbug.com/891427 http/tests/webfont/font-display-intervention.html [ Pass Failure Timeout Crash ]
crbug.com/891427 paint/invalidation/scroll/fixed-img-src-change-after-scroll.html [ Pass Failure Timeout Crash ]
crbug.com/891427 virtual/prefer_compositing_to_lcd_text/scrollbars/custom-scrollbar-changing-style-relayout-body-scrollablearea.html [ Pass Failure Timeout Crash ]
crbug.com/891427 virtual/prefer_compositing_to_lcd_text/scrollbars/custom-scrollbar-changing-style-relayout-div-body-scrollablearea.html [ Pass Failure Timeout Crash ]
# Remove these when --enable-display-compositor-pixel-dump becomes the
# "default" setting. These will all need to be re-baselined at that point.
# All differences should be related to the overflow menu being (now
......@@ -2845,7 +2859,8 @@ crbug.com/543369 [ Win Mac Linux ] fast/forms/select-popup/popup-menu-appearance
crbug.com/548765 http/tests/devtools/console-fetch-logging.js [ Failure Pass ]
crbug.com/564109 [ Win ] http/tests/webfont/font-display-intervention.html [ Pass Failure Timeout ]
### See crbug.com/891427 comment near the top of this file:
###crbug.com/564109 [ Win ] http/tests/webfont/font-display-intervention.html [ Pass Failure Timeout ]
crbug.com/399951 http/tests/mime/javascript-mimetype-usecounters.html [ Pass Failure ]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment