Diff
Modified: releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog (218582 => 218583)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-06-20 10:29:53 UTC (rev 218582)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-06-20 10:34:31 UTC (rev 218583)
@@ -1,3 +1,13 @@
+2017-06-17 Antti Koivisto <[email protected]>
+
+ Crash due to infinite recursion via FrameSelection::updateAppearanceAfterLayout
+ https://bugs.webkit.org/show_bug.cgi?id=173468
+
+ Reviewed by Ryosuke Niwa.
+
+ * editing/selection/updateAppearanceAfterLayout-recursion-expected.txt: Added.
+ * editing/selection/updateAppearanceAfterLayout-recursion.html: Added.
+
2017-06-17 Ryosuke Niwa <[email protected]>
REGRESSION(r209495): materiauxlaverdure.com fails to load
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/editing/selection/updateAppearanceAfterLayout-recursion-expected.txt (0 => 218583)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/editing/selection/updateAppearanceAfterLayout-recursion-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/editing/selection/updateAppearanceAfterLayout-recursion-expected.txt 2017-06-20 10:34:31 UTC (rev 218583)
@@ -0,0 +1,2 @@
+edit
+target
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/editing/selection/updateAppearanceAfterLayout-recursion.html (0 => 218583)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/editing/selection/updateAppearanceAfterLayout-recursion.html (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/editing/selection/updateAppearanceAfterLayout-recursion.html 2017-06-20 10:34:31 UTC (rev 218583)
@@ -0,0 +1,17 @@
+<script>
+if (window.testRunner) {
+ internals.settings.setTelephoneNumberParsingEnabled(true);
+ testRunner.dumpAsText();
+}
+</script>
+<div id=edit>edit</div>
+<div id=target>target</div>
+<script>
+window.getSelection().selectAllChildren(edit);
+target.offsetWidth;
+const link = document.createElement("link");
+link.setAttribute("rel", "stylesheet");
+link.setAttribute("href", "data:text/css,"+Math.random());
+document.head.appendChild(link);
+location.hash = "#target";
+</script>
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog (218582 => 218583)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-06-20 10:29:53 UTC (rev 218582)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-06-20 10:34:31 UTC (rev 218583)
@@ -1,3 +1,34 @@
+2017-06-17 Antti Koivisto <[email protected]>
+
+ Crash due to infinite recursion via FrameSelection::updateAppearanceAfterLayout
+ https://bugs.webkit.org/show_bug.cgi?id=173468
+
+ Reviewed by Ryosuke Niwa.
+
+ Test: editing/selection/updateAppearanceAfterLayout-recursion.html
+
+ Calling FrameSelection::updateAppearanceAfterLayout() from Document::resolveStyle is unsafe
+ because it may cause another call to resolveStyle. We have some cases where the style
+ is still unclean when updateAppearanceAfterLayout() is called. This can lead to infinite
+ recursion.
+
+ The test case is not the common stack seen in CrashTracer (couldn't quit replicate it) but
+ the updateAppearanceAfterLayout/resolveStyle recursion is the same.
+
+ * dom/Document.cpp:
+ (WebCore::Document::resolveStyle):
+
+ Normally selection appearance update is done in post-layout but not all style resolutions schedule a layout.
+ Invoke it asynchronously in that case instead of the previous synchronous call.
+
+ * editing/FrameSelection.cpp:
+ (WebCore::FrameSelection::FrameSelection):
+ (WebCore::FrameSelection::updateAppearanceAfterLayout):
+ (WebCore::FrameSelection::scheduleAppearanceUpdateAfterStyleChange):
+ (WebCore::FrameSelection::appearanceUpdateTimerFired):
+ (WebCore::FrameSelection::updateAppearanceAfterLayoutOrStyleChange):
+ * editing/FrameSelection.h:
+
2017-06-17 Ryosuke Niwa <[email protected]>
REGRESSION(r209495): materiauxlaverdure.com fails to load
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.cpp (218582 => 218583)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.cpp 2017-06-20 10:29:53 UTC (rev 218582)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.cpp 2017-06-20 10:34:31 UTC (rev 218583)
@@ -1834,8 +1834,9 @@
if (updatedCompositingLayers && !frameView.needsLayout())
frameView.viewportContentsChanged();
+ // Usually this is handled by post-layout.
if (!frameView.needsLayout())
- frameView.frame().selection().updateAppearanceAfterLayout();
+ frameView.frame().selection().scheduleAppearanceUpdateAfterStyleChange();
// As a result of the style recalculation, the currently hovered element might have been
// detached (for example, by setting display:none in the :hover style), schedule another mouseMove event
@@ -1845,6 +1846,8 @@
if (m_gotoAnchorNeededAfterStylesheetsLoad && !styleScope().hasPendingSheets())
frameView.scrollToFragment(m_url);
+
+ // FIXME: Ideally we would ASSERT(!needsStyleRecalc()) here but we have some cases where it is not true.
}
bool Document::needsStyleRecalc() const
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/editing/FrameSelection.cpp (218582 => 218583)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/editing/FrameSelection.cpp 2017-06-20 10:29:53 UTC (rev 218582)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/editing/FrameSelection.cpp 2017-06-20 10:34:31 UTC (rev 218583)
@@ -111,6 +111,7 @@
, m_xPosForVerticalArrowNavigation(NoXPosForVerticalArrowNavigation())
, m_granularity(CharacterGranularity)
, m_caretBlinkTimer(*this, &FrameSelection::caretBlinkTimerFired)
+ , m_appearanceUpdateTimer(*this, &FrameSelection::appearanceUpdateTimerFired)
, m_caretInsidePositionFixed(false)
, m_absCaretBoundsDirty(true)
, m_caretPaint(true)
@@ -2397,6 +2398,22 @@
void FrameSelection::updateAppearanceAfterLayout()
{
+ m_appearanceUpdateTimer.stop();
+ updateAppearanceAfterLayoutOrStyleChange();
+}
+
+void FrameSelection::scheduleAppearanceUpdateAfterStyleChange()
+{
+ m_appearanceUpdateTimer.startOneShot(0_s);
+}
+
+void FrameSelection::appearanceUpdateTimerFired()
+{
+ updateAppearanceAfterLayoutOrStyleChange();
+}
+
+void FrameSelection::updateAppearanceAfterLayoutOrStyleChange()
+{
if (auto* client = m_frame->editor().client())
client->updateEditorStateAfterLayoutIfEditabilityChanged();
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/editing/FrameSelection.h (218582 => 218583)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/editing/FrameSelection.h 2017-06-20 10:29:53 UTC (rev 218582)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/editing/FrameSelection.h 2017-06-20 10:34:31 UTC (rev 218583)
@@ -151,6 +151,7 @@
void prepareForDestruction();
void updateAppearanceAfterLayout();
+ void scheduleAppearanceUpdateAfterStyleChange();
void setNeedsSelectionUpdate();
bool contains(const LayoutPoint&);
@@ -315,6 +316,9 @@
void caretBlinkTimerFired();
+ void updateAppearanceAfterLayoutOrStyleChange();
+ void appearanceUpdateTimerFired();
+
void setCaretVisibility(CaretVisibility);
bool recomputeCaretRect();
void invalidateCaretRect();
@@ -334,6 +338,7 @@
RefPtr<EditingStyle> m_typingStyle;
Timer m_caretBlinkTimer;
+ Timer m_appearanceUpdateTimer;
// The painted bounds of the caret in absolute coordinates
IntRect m_absCaretBounds;
bool m_caretInsidePositionFixed : 1;
