Changeset 224159 in webkit
- Timestamp:
- Oct 29, 2017 3:15:55 AM (6 years ago)
- Location:
- trunk/Source/WebCore
- Files:
-
- 5 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebCore/ChangeLog
r224158 r224159 1 2017-10-29 Ryosuke Niwa <rniwa@webkit.org> 2 3 Assert that no script is executed during style recalc 4 https://bugs.webkit.org/show_bug.cgi?id=178845 5 <rdar://problem/35106129> 6 7 Reviewed by Antti Koivisto. 8 9 This patch adds NoEventDispatchAssertion to Document::updateStyle and Document::updateStyleIfNeeded 10 to make sure we don't start mutating DOM in the middle of a style update. 11 12 Added NoEventDispatchAssertion::EventAllowedScope for various places in SVGUseElement to update its 13 shadow tree since that happens while updating the style. 14 15 No new tests since there should be no behavioral change. 16 17 * dom/Document.cpp: 18 (WebCore::Document::resolveStyle): Added NoEventDispatchAssertion while flushing pending stylesheets 19 and calling FrameView::willRecalcStyle, and while the style tree solver is in works. Also moved in 20 the code to update the selection and schedule to dispatch a fake mouse event into the same scope. 21 Also increment m_styleRecalcCount in the same code since post resolution callbacks could run author 22 scripts which in turn trigger another (recursive) style recalc. 23 (WebCore::Document::updateStyleIfNeeded): Put everything but the call to resolveStyle in a scope with 24 NoEventDispatchAssertion. 25 * dom/Element.cpp: 26 (WebCore::Element::cloneElementWithChildren): Added NoEventDispatchAssertion::EventAllowedScope to the 27 newly cloned element for SVG use element's shadow tree. 28 (WebCore::Element::cloneElementWithoutChildren): Ditto. 29 * dom/EventDispatcher.cpp: 30 (WebCore::EventDispatcher::dispatchEvent): Make the assertion more precise to workaround the fact SVG 31 use elements update its shadow tree in the middle of style updates. Also removed a redundant assertion 32 since the result of NoEventDispatchAssertion::isEventDispatchAllowedInSubtree cannot chance without 33 pushing or popoing the stack frame. 34 * svg/SVGUseElement.cpp: 35 (WebCore::SVGUseElement::clearShadowTree): 36 (WebCore::SVGUseElement::updateShadowTree): Added NoEventDispatchAssertion to the user-agent shadow root 37 of a SVG use element. Since this is a newly created shadow tree which hasn't been exposed to author 38 scripts, it's safe to mutate them during the style recalc even though it's not the best design. 39 (WebCore::SVGUseElement::cloneTarget const): Ditto. 40 (WebCore::SVGUseElement::expandUseElementsInShadowTree const): Ditto. 41 (WebCore::SVGUseElement::expandSymbolElementsInShadowTree const): Ditto. 42 (WebCore::SVGUseElement::transferEventListenersToShadowTree const): 43 1 44 2017-10-28 Dean Jackson <dino@apple.com> 2 45 -
trunk/Source/WebCore/dom/Document.cpp
r224150 r224159 1793 1793 // hits a null-dereference due to security code always assuming the document has a SecurityOrigin. 1794 1794 1795 styleScope().flushPendingUpdate(); 1796 1797 frameView.willRecalcStyle(); 1795 { 1796 NoEventDispatchAssertion noEventDispatchAssertion; 1797 styleScope().flushPendingUpdate(); 1798 frameView.willRecalcStyle(); 1799 } 1798 1800 1799 1801 InspectorInstrumentationCookie cookie = InspectorInstrumentation::willRecalculateStyle(*this); 1800 1802 1801 m_inStyleRecalc = true;1802 1803 bool updatedCompositingLayers = false; 1803 1804 { 1804 1805 Style::PostResolutionCallbackDisabler disabler(*this); 1805 1806 WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates; 1807 NoEventDispatchAssertion noEventDispatchAssertion; 1808 1809 m_inStyleRecalc = true; 1806 1810 1807 1811 if (m_pendingStyleRecalcShouldForce) … … 1852 1856 if (m_renderView->needsLayout()) 1853 1857 frameView.layoutContext().scheduleLayout(); 1858 1859 // Usually this is handled by post-layout. 1860 if (!frameView.needsLayout()) 1861 frameView.frame().selection().scheduleAppearanceUpdateAfterStyleChange(); 1862 1863 // As a result of the style recalculation, the currently hovered element might have been 1864 // detached (for example, by setting display:none in the :hover style), schedule another mouseMove event 1865 // to check if any other elements ended up under the mouse pointer due to re-layout. 1866 if (m_hoveredElement && !m_hoveredElement->renderer()) 1867 frameView.frame().mainFrame().eventHandler().dispatchFakeMouseMoveEventSoon(); 1868 1869 ++m_styleRecalcCount; 1870 // FIXME: Assert ASSERT(!needsStyleRecalc()) here. Do we still have some cases where it's not true? 1854 1871 } 1855 1872 … … 1859 1876 implicitClose(); 1860 1877 } 1861 1862 ++m_styleRecalcCount;1863 1878 1864 1879 InspectorInstrumentation::didRecalculateStyle(cookie); … … 1870 1885 frameView.viewportContentsChanged(); 1871 1886 1872 // Usually this is handled by post-layout.1873 if (!frameView.needsLayout())1874 frameView.frame().selection().scheduleAppearanceUpdateAfterStyleChange();1875 1876 // As a result of the style recalculation, the currently hovered element might have been1877 // detached (for example, by setting display:none in the :hover style), schedule another mouseMove event1878 // to check if any other elements ended up under the mouse pointer due to re-layout.1879 if (m_hoveredElement && !m_hoveredElement->renderer())1880 frameView.frame().mainFrame().eventHandler().dispatchFakeMouseMoveEventSoon();1881 1882 1887 if (m_gotoAnchorNeededAfterStylesheetsLoad && !styleScope().hasPendingSheets()) 1883 1888 frameView.scrollToFragment(m_url); 1884 1885 // FIXME: Ideally we would ASSERT(!needsStyleRecalc()) here but we have some cases where it is not true.1886 1889 } 1887 1890 … … 1921 1924 bool Document::updateStyleIfNeeded() 1922 1925 { 1923 ASSERT(isMainThread()); 1924 ASSERT(!view() || !view()->isPainting()); 1925 1926 if (!view() || view()->layoutContext().isInRenderTreeLayout()) 1927 return false; 1928 1929 styleScope().flushPendingUpdate(); 1930 1931 if (!needsStyleRecalc()) 1932 return false; 1926 { 1927 NoEventDispatchAssertion noEventDispatchAssertion; 1928 ASSERT(isMainThread()); 1929 ASSERT(!view() || !view()->isPainting()); 1930 1931 if (!view() || view()->layoutContext().isInRenderTreeLayout()) 1932 return false; 1933 1934 styleScope().flushPendingUpdate(); 1935 1936 if (!needsStyleRecalc()) 1937 return false; 1938 } 1933 1939 1934 1940 resolveStyle(); -
trunk/Source/WebCore/dom/Element.cpp
r224150 r224159 342 342 { 343 343 Ref<Element> clone = cloneElementWithoutChildren(targetDocument); 344 345 // It's safe to dispatch events on the cloned node since author scripts have no access to it yet. 346 // This is needed for SVGUseElement::cloneTarget. 347 NoEventDispatchAssertion::EventAllowedScope allowedScope(clone.get()); 348 344 349 cloneChildNodes(clone); 345 350 return clone; … … 349 354 { 350 355 Ref<Element> clone = cloneElementWithoutAttributesAndChildren(targetDocument); 356 357 // It's safe to dispatch events on the cloned node since author scripts have no access to it yet. 358 // This is needed for SVGUseElement::cloneTarget. 359 NoEventDispatchAssertion::EventAllowedScope allowedScope(clone.get()); 360 351 361 // This will catch HTML elements in the wrong namespace that are not correctly copied. 352 362 // This is a sanity check as HTML overloads some of the DOM methods. -
trunk/Source/WebCore/dom/EventDispatcher.cpp
r224131 r224159 131 131 bool EventDispatcher::dispatchEvent(Node& node, Event& event) 132 132 { 133 ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEvent AllowedInMainThread());133 ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventDispatchAllowedInSubtree(node)); 134 134 Ref<Node> protectedNode(node); 135 135 RefPtr<FrameView> view = node.document().view(); … … 149 149 if (!event.target()) 150 150 return true; 151 152 ASSERT_WITH_SECURITY_IMPLICATION(NoEventDispatchAssertion::isEventAllowedInMainThread());153 151 154 152 InputElementClickState clickHandlingState; -
trunk/Source/WebCore/svg/SVGUseElement.cpp
r224131 r224159 217 217 { 218 218 if (auto root = userAgentShadowRoot()) { 219 // Safe because SVG use element's shadow tree is never used to fire synchronous events during layout or DOM mutations. 219 220 NoEventDispatchAssertion::EventAllowedScope scope(*root); 220 221 root->removeChildren(); … … 244 245 } 245 246 246 cloneTarget(ensureUserAgentShadowRoot(), *target); 247 expandUseElementsInShadowTree(); 248 expandSymbolElementsInShadowTree(); 247 { 248 // Safe because the cloned shadow tree has never been exposed to author scripts. 249 auto& shadowRoot = ensureUserAgentShadowRoot(); 250 NoEventDispatchAssertion::EventAllowedScope scope(shadowRoot); 251 cloneTarget(shadowRoot, *target); 252 expandUseElementsInShadowTree(); 253 expandSymbolElementsInShadowTree(); 254 updateRelativeLengthsInformation(); 255 } 256 249 257 transferEventListenersToShadowTree(); 250 251 updateRelativeLengthsInformation();252 258 253 259 // When we invalidate the other shadow trees, it's important that we don't … … 431 437 { 432 438 Ref<SVGElement> targetClone = static_cast<SVGElement&>(target.cloneElementWithChildren(document()).get()); 439 // Safe because the newy cloned nodes in the shadow tree has not been exposed to author scripts yet. 440 NoEventDispatchAssertion::EventAllowedScope scope(targetClone); 433 441 associateClonesWithOriginals(targetClone.get(), target); 434 442 removeDisallowedElementsFromSubtree(targetClone.get()); … … 463 471 464 472 auto replacementClone = SVGGElement::create(document()); 473 // Safe because the use element's shadow tree is not exposed to author scripts, and we don't fire synchronous events during layout & DOM layout. 474 NoEventDispatchAssertion::EventAllowedScope scope(replacementClone); 475 465 476 cloneDataAndChildren(replacementClone.get(), originalClone); 466 477 … … 496 507 497 508 auto replacementClone = SVGSVGElement::create(document()); 509 // Safe because the newly created SVG element and the newly created shadow tree has not been exposed to author scripts yet. 510 NoEventDispatchAssertion::EventAllowedScope scope(replacementClone); 498 511 cloneDataAndChildren(replacementClone.get(), originalClone); 499 512 … … 507 520 void SVGUseElement::transferEventListenersToShadowTree() const 508 521 { 522 // FIXME: Don't directly add event listeners on each descendant. Copy event listeners on the use element instead. 509 523 for (auto& descendant : descendantsOfType<SVGElement>(*userAgentShadowRoot())) { 510 524 if (EventTargetData* data = descendant.correspondingElement()->eventTargetData())
Note: See TracChangeset
for help on using the changeset viewer.