Commit eb6e9c67 authored by David Bokan's avatar David Bokan Committed by Commit Bot

Prevent reentrancy crash in ScrollAnchorAdjustment

CHECKs proved this to be the cause of the crash in the bug. While
there are some suggested fixes in the bug that are preferable, this
patch patch simply avoids reentering PerformScrollAnchoringAdjustments
which will prevent the crash.

Bug: 745686
Change-Id: Ic2a156cb344b6b32e5aa4d5c3164421a033c757e
Reviewed-on: https://chromium-review.googlesource.com/657600Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501549}
parent bc49021b
...@@ -209,7 +209,6 @@ LocalFrameView::LocalFrameView(LocalFrame& frame, IntRect frame_rect) ...@@ -209,7 +209,6 @@ LocalFrameView::LocalFrameView(LocalFrame& frame, IntRect frame_rect)
DocumentLifecycle::kUninitialized), DocumentLifecycle::kUninitialized),
past_layout_lifecycle_update_(false), past_layout_lifecycle_update_(false),
scroll_anchor_(this), scroll_anchor_(this),
in_perform_scroll_anchoring_adjustments_(false),
scrollbar_manager_(*this), scrollbar_manager_(*this),
needs_scrollbars_update_(false), needs_scrollbars_update_(false),
suppress_adjust_view_size_(false), suppress_adjust_view_size_(false),
...@@ -3276,20 +3275,18 @@ void LocalFrameView::EnqueueScrollAnchoringAdjustment( ...@@ -3276,20 +3275,18 @@ void LocalFrameView::EnqueueScrollAnchoringAdjustment(
} }
void LocalFrameView::PerformScrollAnchoringAdjustments() { void LocalFrameView::PerformScrollAnchoringAdjustments() {
// TODO(bokan): Temporary to get more information about crash in // Adjust() will cause a scroll which could end up causing a layout and
// crbug.com/745686. // reentering this method. Copy and clear the queue so we don't modify it
CHECK(!in_perform_scroll_anchoring_adjustments_); // during iteration.
in_perform_scroll_anchoring_adjustments_ = true; AnchoringAdjustmentQueue queue_copy = anchoring_adjustment_queue_;
anchoring_adjustment_queue_.clear();
for (WeakMember<ScrollableArea>& scroller : anchoring_adjustment_queue_) { for (WeakMember<ScrollableArea>& scroller : queue_copy) {
if (scroller) { if (scroller) {
DCHECK(scroller->GetScrollAnchor()); DCHECK(scroller->GetScrollAnchor());
scroller->GetScrollAnchor()->Adjust(); scroller->GetScrollAnchor()->Adjust();
} }
} }
anchoring_adjustment_queue_.clear();
in_perform_scroll_anchoring_adjustments_ = false;
} }
void LocalFrameView::PrePaint() { void LocalFrameView::PrePaint() {
......
...@@ -1219,10 +1219,6 @@ class CORE_EXPORT LocalFrameView final ...@@ -1219,10 +1219,6 @@ class CORE_EXPORT LocalFrameView final
HeapLinkedHashSet<WeakMember<ScrollableArea>>; HeapLinkedHashSet<WeakMember<ScrollableArea>>;
AnchoringAdjustmentQueue anchoring_adjustment_queue_; AnchoringAdjustmentQueue anchoring_adjustment_queue_;
// TODO(bokan): Temporary to get more information about crash in
// crbug.com/745686.
bool in_perform_scroll_anchoring_adjustments_;
// ScrollbarManager holds the Scrollbar instances. // ScrollbarManager holds the Scrollbar instances.
ScrollbarManager scrollbar_manager_; ScrollbarManager scrollbar_manager_;
......
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