- Revision
- 227858
- Author
- [email protected]
- Date
- 2018-01-30 14:47:05 -0800 (Tue, 30 Jan 2018)
Log Message
Release assert in updateLayout() via AXObjectCache::childrenChanged
https://bugs.webkit.org/show_bug.cgi?id=182279
<rdar://problem/36994456>
Reviewed by Antti Koivisto.
Source/WebCore:
Disable the assertion in Document::updateLayout and Document::updateStyle* in this particular circumstance as fixing it
would require a large architectural refactoring of the accessibility code.
Test: accessibility/accessibility-object-update-during-style-resolution-crash.html
* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::childrenChanged): Disabled the release assertion here.
* dom/Document.cpp:
(WebCore::Document::isSafeToUpdateStyleOrLayout const): Check LayoutAssertionDisableScope::shouldDisable.
* dom/ScriptDisallowedScope.h:
(WebCore::ScriptDisallowedScope::LayoutAssertionDisableScope): Added.
(WebCore::ScriptDisallowedScope::LayoutAssertionDisableScope::LayoutAssertionDisableScope): Added.
(WebCore::ScriptDisallowedScope::LayoutAssertionDisableScope::~LayoutAssertionDisableScope): Added.
(WebCore::ScriptDisallowedScope::LayoutAssertionDisableScope::shouldDisable): Added.
* page/LayoutContext.cpp:
(WebCore::LayoutContext::layout): Check LayoutAssertionDisableScope::shouldDisable.
LayoutTests:
Added a regression test.
* accessibility/accessibility-object-update-during-style-resolution-crash-expected.txt: Added.
* accessibility/accessibility-object-update-during-style-resolution-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (227857 => 227858)
--- trunk/LayoutTests/ChangeLog 2018-01-30 22:45:09 UTC (rev 227857)
+++ trunk/LayoutTests/ChangeLog 2018-01-30 22:47:05 UTC (rev 227858)
@@ -1,3 +1,16 @@
+2018-01-30 Ryosuke Niwa <[email protected]>
+
+ Release assert in updateLayout() via AXObjectCache::childrenChanged
+ https://bugs.webkit.org/show_bug.cgi?id=182279
+ <rdar://problem/36994456>
+
+ Reviewed by Antti Koivisto.
+
+ Added a regression test.
+
+ * accessibility/accessibility-object-update-during-style-resolution-crash-expected.txt: Added.
+ * accessibility/accessibility-object-update-during-style-resolution-crash.html: Added.
+
2018-01-30 Matt Lewis <[email protected]>
Skipping imported/w3c/web-platform-tests/service-workers/service-worker/navigation-redirect.https.html.
Added: trunk/LayoutTests/accessibility/accessibility-object-update-during-style-resolution-crash-expected.txt (0 => 227858)
--- trunk/LayoutTests/accessibility/accessibility-object-update-during-style-resolution-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/accessibility/accessibility-object-update-during-style-resolution-crash-expected.txt 2018-01-30 22:47:05 UTC (rev 227858)
@@ -0,0 +1,4 @@
+This tests invoking updateLayout durign a live region update from the style recalc.
+WebKit should not hit a release assertion.
+
+PASS. WebKit did not crash.
Added: trunk/LayoutTests/accessibility/accessibility-object-update-during-style-resolution-crash.html (0 => 227858)
--- trunk/LayoutTests/accessibility/accessibility-object-update-during-style-resolution-crash.html (rev 0)
+++ trunk/LayoutTests/accessibility/accessibility-object-update-during-style-resolution-crash.html 2018-01-30 22:47:05 UTC (rev 227858)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests invoking updateLayout durign a live region update from the style recalc.<br>
+WebKit should not hit a release assertion.</p>
+<section style="display: none">
+ <label for="" id="input" type="text" aria-labelledby="hello"><div id="in-label" aria-live="polite"></div></label>
+ <span id="hello">hello</span>
+</section>
+<script>
+function runTest()
+{
+ document.querySelector('section').style.display = null;
+ document.body.getBoundingClientRect();
+}
+
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ document.body.getBoundingClientRect();
+ const webArea = accessibilityController.rootElement.childAtIndex(0);
+ runTest();
+ document.querySelector('section').style.display = 'none';
+ document.write('PASS. WebKit did not crash.');
+} else
+ document.write('<button _onclick_="runTest()">Go</button>');
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (227857 => 227858)
--- trunk/Source/WebCore/ChangeLog 2018-01-30 22:45:09 UTC (rev 227857)
+++ trunk/Source/WebCore/ChangeLog 2018-01-30 22:47:05 UTC (rev 227858)
@@ -1,3 +1,28 @@
+2018-01-30 Ryosuke Niwa <[email protected]>
+
+ Release assert in updateLayout() via AXObjectCache::childrenChanged
+ https://bugs.webkit.org/show_bug.cgi?id=182279
+ <rdar://problem/36994456>
+
+ Reviewed by Antti Koivisto.
+
+ Disable the assertion in Document::updateLayout and Document::updateStyle* in this particular circumstance as fixing it
+ would require a large architectural refactoring of the accessibility code.
+
+ Test: accessibility/accessibility-object-update-during-style-resolution-crash.html
+
+ * accessibility/AXObjectCache.cpp:
+ (WebCore::AXObjectCache::childrenChanged): Disabled the release assertion here.
+ * dom/Document.cpp:
+ (WebCore::Document::isSafeToUpdateStyleOrLayout const): Check LayoutAssertionDisableScope::shouldDisable.
+ * dom/ScriptDisallowedScope.h:
+ (WebCore::ScriptDisallowedScope::LayoutAssertionDisableScope): Added.
+ (WebCore::ScriptDisallowedScope::LayoutAssertionDisableScope::LayoutAssertionDisableScope): Added.
+ (WebCore::ScriptDisallowedScope::LayoutAssertionDisableScope::~LayoutAssertionDisableScope): Added.
+ (WebCore::ScriptDisallowedScope::LayoutAssertionDisableScope::shouldDisable): Added.
+ * page/LayoutContext.cpp:
+ (WebCore::LayoutContext::layout): Check LayoutAssertionDisableScope::shouldDisable.
+
2018-01-30 Zalan Bujtas <[email protected]>
[RenderTreeBuilder] Move RenderRubyRun::rubyBaseSafe to RenderTreeBuilder::Ruby
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (227857 => 227858)
--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2018-01-30 22:45:09 UTC (rev 227857)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2018-01-30 22:47:05 UTC (rev 227858)
@@ -95,6 +95,7 @@
#include "RenderTableRow.h"
#include "RenderView.h"
#include "SVGElement.h"
+#include "ScriptDisallowedScope.h"
#include "ScrollView.h"
#include "TextBoundaries.h"
#include "TextControlInnerElements.h"
@@ -832,7 +833,7 @@
handleMenuOpened(newChild);
handleLiveRegionCreated(newChild);
}
-
+
childrenChanged(get(node));
}
@@ -840,6 +841,9 @@
{
if (!renderer)
return;
+
+ // FIXME: Refactor the code to avoid calling updateLayout in this call stack.
+ ScriptDisallowedScope::LayoutAssertionDisableScope disableScope;
if (newChild) {
handleMenuOpened(newChild->node());
Modified: trunk/Source/WebCore/dom/Document.cpp (227857 => 227858)
--- trunk/Source/WebCore/dom/Document.cpp 2018-01-30 22:45:09 UTC (rev 227857)
+++ trunk/Source/WebCore/dom/Document.cpp 2018-01-30 22:47:05 UTC (rev 227858)
@@ -314,6 +314,8 @@
static const unsigned cMaxWriteRecursionDepth = 21;
bool Document::hasEverCreatedAnAXObjectCache = false;
+unsigned ScriptDisallowedScope::LayoutAssertionDisableScope::s_layoutAssertionDisableCount = 0;
+
// DOM Level 2 says (letters added):
//
// a) Name start characters must have one of the categories Ll, Lu, Lo, Lt, Nl.
@@ -1940,7 +1942,8 @@
{
bool isSafeToExecuteScript = ScriptDisallowedScope::InMainThread::isScriptAllowed();
bool isInFrameFlattening = view() && view()->isInChildFrameWithFrameFlattening();
- return isSafeToExecuteScript || isInFrameFlattening || !isInWebProcess();
+ bool isAssertionDisabled = ScriptDisallowedScope::LayoutAssertionDisableScope::shouldDisable();
+ return isSafeToExecuteScript || isInFrameFlattening || !isInWebProcess() || isAssertionDisabled;
}
bool Document::updateStyleIfNeeded()
Modified: trunk/Source/WebCore/dom/ScriptDisallowedScope.h (227857 => 227858)
--- trunk/Source/WebCore/dom/ScriptDisallowedScope.h 2018-01-30 22:45:09 UTC (rev 227857)
+++ trunk/Source/WebCore/dom/ScriptDisallowedScope.h 2018-01-30 22:47:05 UTC (rev 227858)
@@ -147,6 +147,25 @@
unsigned m_originalCount { 0 };
};
+ // FIXME: Remove all uses of this class.
+ class LayoutAssertionDisableScope {
+ public:
+ LayoutAssertionDisableScope()
+ {
+ s_layoutAssertionDisableCount++;
+ }
+
+ ~LayoutAssertionDisableScope()
+ {
+ s_layoutAssertionDisableCount--;
+ }
+
+ static bool shouldDisable() { return s_layoutAssertionDisableCount; }
+
+ private:
+ static unsigned s_layoutAssertionDisableCount;
+ };
+
private:
WEBCORE_EXPORT static unsigned s_count;
};
Modified: trunk/Source/WebCore/page/LayoutContext.cpp (227857 => 227858)
--- trunk/Source/WebCore/page/LayoutContext.cpp 2018-01-30 22:45:09 UTC (rev 227857)
+++ trunk/Source/WebCore/page/LayoutContext.cpp 2018-01-30 22:47:05 UTC (rev 227858)
@@ -122,7 +122,7 @@
{
LOG_WITH_STREAM(Layout, stream << "FrameView " << &view() << " LayoutContext::layout() with size " << view().layoutSize());
- RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!frame().document()->inRenderTreeUpdate());
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!frame().document()->inRenderTreeUpdate() || ScriptDisallowedScope::LayoutAssertionDisableScope::shouldDisable());
ASSERT(LayoutDisallowedScope::isLayoutAllowed());
ASSERT(!view().isPainting());
ASSERT(frame().view() == &view());
