Protect frames during style and layout changes
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 May 2019 21:02:38 +0000 (21:02 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 May 2019 21:02:38 +0000 (21:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198047
<rdar://problem/50954082>

Reviewed by Zalan Bujtas.

Be more careful about the scope and lifetime of objects that participate in layout or
style updates. If a method decides a layout or style update is needed, it needs to
confirm that the elements it was operating on are still valid and needed in the
current operation.

Source/WebCore:

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::getOrCreate):
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::accessibilityHitTest const):
* css/CSSComputedStyleDeclaration.cpp:
(WebCore::ComputedStyleExtractor::valueForPropertyinStyle):
* css/CSSComputedStyleDeclaration.h:
* css/SVGCSSComputedStyleDeclaration.cpp:
(WebCore::ComputedStyleExtractor::svgPropertyValue):
* dom/Document.cpp:
(WebCore::Document::setFocusedElement):
* editing/TypingCommand.cpp:
(WebCore::TypingCommand::insertTextRunWithoutNewlines):
(WebCore::TypingCommand::insertLineBreak):
(WebCore::TypingCommand::insertParagraphSeparator):
(WebCore::TypingCommand::insertParagraphSeparatorInQuotedContent):
* editing/ios/EditorIOS.mm:
(WebCore::Editor::setDictationPhrasesAsChildOfElement):
* html/HTMLLabelElement.cpp:
(WebCore::HTMLLabelElement::focus):
* html/HTMLTextAreaElement.cpp:
(WebCore::HTMLTextAreaElement::appendFormData):
* html/ImageDocument.cpp:
(WebCore::ImageDocument::imageClicked):
* html/ValidationMessage.cpp:
(WebCore::ValidationMessage::buildBubbleTree):
* page/FrameView.cpp:
(WebCore::FrameView::autoSizeIfEnabled):
(WebCore::FrameView::trackedRepaintRectsAsText const):
* page/PrintContext.cpp:
(WebCore::PrintContext::pageProperty):
(WebCore::PrintContext::numberOfPages):
(WebCore::PrintContext::spoolAllPagesWithBoundaries):

Source/WebKitLegacy/mac:

* DOM/DOM.mm:
(-[DOMRange renderedImageForcingBlackText:renderedImageForcingBlackText:]):
* WebView/WebHTMLView.mm:
(-[WebHTMLView _selectionDraggingImage]):
(-[WebHTMLView selectionImageForcingBlackText:selectionImageForcingBlackText:]):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245823 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AXObjectCache.cpp
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Source/WebCore/css/CSSComputedStyleDeclaration.cpp
Source/WebCore/css/CSSComputedStyleDeclaration.h
Source/WebCore/css/SVGCSSComputedStyleDeclaration.cpp
Source/WebCore/dom/Document.cpp
Source/WebCore/editing/TypingCommand.cpp
Source/WebCore/editing/ios/EditorIOS.mm
Source/WebCore/html/HTMLLabelElement.cpp
Source/WebCore/html/HTMLTextAreaElement.cpp
Source/WebCore/html/ImageDocument.cpp
Source/WebCore/html/ValidationMessage.cpp
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/PrintContext.cpp
Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/DOM/DOM.mm
Source/WebKitLegacy/mac/WebView/WebHTMLView.mm

index ec55eea..c2f7c68 100644 (file)
@@ -1,3 +1,50 @@
+2019-05-28  Brent Fulgham  <bfulgham@apple.com>
+
+        Protect frames during style and layout changes
+        https://bugs.webkit.org/show_bug.cgi?id=198047
+        <rdar://problem/50954082>
+
+        Reviewed by Zalan Bujtas.
+
+        Be more careful about the scope and lifetime of objects that participate in layout or
+        style updates. If a method decides a layout or style update is needed, it needs to
+        confirm that the elements it was operating on are still valid and needed in the
+        current operation.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::getOrCreate):
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::accessibilityHitTest const):
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::ComputedStyleExtractor::valueForPropertyinStyle):
+        * css/CSSComputedStyleDeclaration.h:
+        * css/SVGCSSComputedStyleDeclaration.cpp:
+        (WebCore::ComputedStyleExtractor::svgPropertyValue):
+        * dom/Document.cpp:
+        (WebCore::Document::setFocusedElement):
+        * editing/TypingCommand.cpp:
+        (WebCore::TypingCommand::insertTextRunWithoutNewlines):
+        (WebCore::TypingCommand::insertLineBreak):
+        (WebCore::TypingCommand::insertParagraphSeparator):
+        (WebCore::TypingCommand::insertParagraphSeparatorInQuotedContent):
+        * editing/ios/EditorIOS.mm:
+        (WebCore::Editor::setDictationPhrasesAsChildOfElement):
+        * html/HTMLLabelElement.cpp:
+        (WebCore::HTMLLabelElement::focus):
+        * html/HTMLTextAreaElement.cpp:
+        (WebCore::HTMLTextAreaElement::appendFormData):
+        * html/ImageDocument.cpp:
+        (WebCore::ImageDocument::imageClicked):
+        * html/ValidationMessage.cpp:
+        (WebCore::ValidationMessage::buildBubbleTree):
+        * page/FrameView.cpp:
+        (WebCore::FrameView::autoSizeIfEnabled):
+        (WebCore::FrameView::trackedRepaintRectsAsText const):
+        * page/PrintContext.cpp:
+        (WebCore::PrintContext::pageProperty):
+        (WebCore::PrintContext::numberOfPages):
+        (WebCore::PrintContext::spoolAllPagesWithBoundaries):
+
 2019-05-28  Antti Koivisto  <antti@apple.com>
 
         [async scrolling] Fixed positioning inside stacking context overflow scroll is jumpy
index b809586..a0e1a4e 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2008-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -594,6 +594,8 @@ AccessibilityObject* AXObjectCache::getOrCreate(Node* node)
     if (!inCanvasSubtree && !isHidden && !insideMeterElement)
         return nullptr;
 
+    auto protectedNode = makeRef(*node);
+
     // Fallback content is only focusable as long as the canvas is displayed and visible.
     // Update the style before Element::isFocusable() gets called.
     if (inCanvasSubtree)
index eaf9409..468e558 100644 (file)
@@ -1,5 +1,5 @@
 /*
-* Copyright (C) 2008 Apple Inc. All rights reserved.
+* Copyright (C) 2008-2019 Apple Inc. All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
@@ -2417,6 +2417,9 @@ AccessibilityObjectInterface* AccessibilityRenderObject::accessibilityHitTest(co
     
     m_renderer->document().updateLayout();
 
+    if (!m_renderer || !m_renderer->hasLayer())
+        return nullptr;
+
     RenderLayer* layer = downcast<RenderBox>(*m_renderer).layer();
      
     HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AccessibilityHitTest);
index 745c134..36ae3e1 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2004 Zack Rusin <zack@kde.org>
- * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Alexey Proskuryakov <ap@webkit.org>
  * Copyright (C) 2007 Nicholas Shanks <webkit@nickshanks.com>
  * Copyright (C) 2011 Sencha, Inc. All rights reserved.
@@ -4223,7 +4223,7 @@ RefPtr<CSSValue> ComputedStyleExtractor::valueForPropertyinStyle(const RenderSty
         case CSSPropertyKerning:
         case CSSPropertyTextAnchor:
         case CSSPropertyVectorEffect:
-            return svgPropertyValue(propertyID, DoNotUpdateLayout);
+            return svgPropertyValue(propertyID);
         case CSSPropertyCustom:
             ASSERT_NOT_REACHED();
             return nullptr;
index e258b7e..6aae987 100644 (file)
@@ -88,7 +88,7 @@ private:
     // no pseudo-element.
     RenderElement* styledRenderer() const;
 
-    RefPtr<CSSValue> svgPropertyValue(CSSPropertyID, EUpdateLayout);
+    RefPtr<CSSValue> svgPropertyValue(CSSPropertyID);
     Ref<CSSValue> adjustSVGPaintForCurrentColor(SVGPaintType, const String& url, const Color&, const Color& currentColor) const;
     static Ref<CSSValue> valueForShadow(const ShadowData*, CSSPropertyID, const RenderStyle&, AdjustPixelValuesForComputedStyle = AdjustPixelValues);
     Ref<CSSPrimitiveValue> currentColorOrValidColor(const RenderStyle*, const Color&) const;
index 7fb137e..58271b3 100644 (file)
@@ -1,6 +1,7 @@
 /*
     Copyright (C) 2007 Eric Seidel <eric@webkit.org>
     Copyright (C) 2007 Alexey Proskuryakov <ap@webkit.org>
+    Copyright (C) 2019 Apple Inc. All rights reserved.
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
@@ -81,15 +82,11 @@ Ref<CSSValue> ComputedStyleExtractor::adjustSVGPaintForCurrentColor(SVGPaintType
     return CSSPrimitiveValue::create(color);
 }
 
-RefPtr<CSSValue> ComputedStyleExtractor::svgPropertyValue(CSSPropertyID propertyID, EUpdateLayout updateLayout)
+RefPtr<CSSValue> ComputedStyleExtractor::svgPropertyValue(CSSPropertyID propertyID)
 {
     if (!m_element)
         return nullptr;
 
-    // Make sure our layout is up to date before we allow a query on these attributes.
-    if (updateLayout)
-        m_element->document().updateLayout();
-
     auto* style = m_element->computedStyle();
     if (!style)
         return nullptr;
index a8a8400..d8f3a7a 100644 (file)
@@ -4258,8 +4258,8 @@ bool Document::setFocusedElement(Element* element, FocusDirection direction, Foc
             }
             if (focusWidget)
                 focusWidget->setFocus(true);
-            else
-                view()->setFocus(true);
+            else if (auto* frameView = view())
+                frameView->setFocus(true);
         }
     }
 
index 67063ee..01f53db 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2008, 2016 Apple Inc.  All rights reserved.
+ * Copyright (C) 2005-2019 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -551,6 +551,8 @@ void TypingCommand::insertTextRunWithoutNewlines(const String &text, bool select
 
     applyCommandToComposite(WTFMove(command), endingSelection());
 
+    Frame& frame = this->frame();
+    Ref<Frame> protector(frame);
     typingAddedToOpenCommand(InsertText);
 }
 
@@ -563,6 +565,9 @@ void TypingCommand::insertLineBreak()
         return;
 
     applyCommandToComposite(InsertLineBreakCommand::create(document()));
+
+    Frame& frame = this->frame();
+    Ref<Frame> protector(frame);
     typingAddedToOpenCommand(InsertLineBreak);
 }
 
@@ -583,6 +588,9 @@ void TypingCommand::insertParagraphSeparator()
         return;
 
     applyCommandToComposite(InsertParagraphSeparatorCommand::create(document(), false, false, EditAction::TypingInsertParagraph));
+
+    Frame& frame = this->frame();
+    Ref<Frame> protector(frame);
     typingAddedToOpenCommand(InsertParagraphSeparator);
 }
 
@@ -607,6 +615,9 @@ void TypingCommand::insertParagraphSeparatorInQuotedContent()
     }
         
     applyCommandToComposite(BreakBlockquoteCommand::create(document()));
+
+    Frame& frame = this->frame();
+    Ref<Frame> protector(frame);
     typingAddedToOpenCommand(InsertParagraphSeparatorInQuotedContent);
 }
 
index 985addd..4126ed1 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006, 2007, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -279,9 +279,14 @@ void Editor::setDictationPhrasesAsChildOfElement(const Vector<Vector<String>>& d
 
     element.appendChild(createFragmentFromText(*context, dictationPhrasesBuilder.toString()));
 
+    auto weakElement = makeWeakPtr(element);
+
     // We need a layout in order to add markers below.
     document().updateLayout();
 
+    if (!weakElement)
+        return;
+
     if (!element.firstChild()->isTextNode()) {
         // Shouldn't happen.
         ASSERT(element.firstChild()->isTextNode());
index abee8fb..cae2e17 100644 (file)
@@ -2,7 +2,7 @@
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
- * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
  * This library is free software; you can redistribute it and/or
@@ -149,6 +149,7 @@ bool HTMLLabelElement::willRespondToMouseClickEvents()
 
 void HTMLLabelElement::focus(bool restorePreviousSelection, FocusDirection direction)
 {
+    Ref<HTMLLabelElement> protectedThis(*this);
     if (document().haveStylesheetsLoaded()) {
         document().updateLayout();
         if (isFocusable()) {
index 71d93b6..86adeef 100644 (file)
@@ -2,7 +2,7 @@
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
- * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  * Copyright (C) 2007 Samuel Weinig (sam@webkit.org)
  *
@@ -223,6 +223,7 @@ bool HTMLTextAreaElement::appendFormData(DOMFormData& formData, bool)
     if (name().isEmpty())
         return false;
 
+    Ref<HTMLTextAreaElement> protectedThis(*this);
     document().updateLayout();
 
     formData.append(name(), m_wrap == HardWrap ? valueWithHardLineBreaks() : value());
index 2eb3c50..151f44c 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006, 2007, 2008, 2010, 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -399,6 +399,9 @@ void ImageDocument::imageClicked(int x, int y)
 
         updateLayout();
 
+        if (!view())
+            return;
+
         float scale = this->scale();
 
         IntSize viewportSize = view()->visibleSize();
index 4cc00bc..dae9f12 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2010, 2012 Google Inc. All rights reserved.
+ * Copyright (C) 2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -187,7 +188,14 @@ void ValidationMessage::buildBubbleTree()
     // contains non-absolute or non-fixed renderers as children.
     m_bubble->setInlineStyleProperty(CSSPropertyPosition, CSSValueAbsolute);
     shadowRoot.appendChild(*m_bubble);
+
+    auto weakElement = makeWeakPtr(*m_element);
+
     document.updateLayout();
+
+    if (!weakElement || !m_element->renderer())
+        return;
+
     adjustBubblePosition(m_element->renderer()->absoluteBoundingBoxRect(), m_bubble.get());
 
     auto clipper = HTMLDivElement::create(document);
index 08b0814..120f12d 100644 (file)
@@ -3,7 +3,7 @@
  *                     1999 Lars Knoll <knoll@kde.org>
  *                     1999 Antti Koivisto <koivisto@kde.org>
  *                     2000 Dirk Mueller <mueller@kde.org>
- * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
  *           (C) 2006 Graham Dennis (graham.dennis@gmail.com)
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  * Copyright (C) 2009 Google Inc. All rights reserved.
@@ -3468,6 +3468,8 @@ void FrameView::autoSizeIfEnabled()
     auto& documentRenderer = downcast<RenderElement>(*firstChild);
     documentRenderer.mutableStyle().setMaxWidth(Length(m_autoSizeConstraint.width(), Fixed));
     resize(m_autoSizeConstraint.width(), m_autoSizeConstraint.height());
+
+    Ref<FrameView> protectedThis(*this);
     document->updateStyleIfNeeded();
     document->updateLayoutIgnorePendingStylesheets();
 
@@ -4866,8 +4868,11 @@ void FrameView::resetTrackedRepaints()
 
 String FrameView::trackedRepaintRectsAsText() const
 {
-    if (frame().document())
-        frame().document()->updateLayout();
+    Frame& frame = this->frame();
+    Ref<Frame> protector(frame);
+
+    if (auto* document = frame.document())
+        document->updateLayout();
 
     TextStream ts;
     if (!m_trackedRepaintRects.isEmpty()) {
index d2b35f3..00c0c4e 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2007 Alp Toker <alp@atoker.com>
- * Copyright (C) 2007, 2016 Apple Inc.
+ * Copyright (C) 2007-2019 Apple Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -345,6 +345,8 @@ String PrintContext::pageProperty(Frame* frame, const char* propertyName, int pa
     ASSERT(frame);
     ASSERT(frame->document());
 
+    Ref<Frame> protectedFrame(*frame);
+
     auto& document = *frame->document();
     PrintContext printContext(frame);
     printContext.begin(800); // Any width is OK here.
@@ -400,6 +402,8 @@ bool PrintContext::beginAndComputePageRectsWithPageSize(Frame& frame, const Floa
 
 int PrintContext::numberOfPages(Frame& frame, const FloatSize& pageSizeInPixels)
 {
+    Ref<Frame> protectedFrame(frame);
+
     PrintContext printContext(&frame);
     if (!printContext.beginAndComputePageRectsWithPageSize(frame, pageSizeInPixels))
         return -1;
@@ -409,6 +413,8 @@ int PrintContext::numberOfPages(Frame& frame, const FloatSize& pageSizeInPixels)
 
 void PrintContext::spoolAllPagesWithBoundaries(Frame& frame, GraphicsContext& graphicsContext, const FloatSize& pageSizeInPixels)
 {
+    Ref<Frame> protectedFrame(frame);
+
     PrintContext printContext(&frame);
     if (!printContext.beginAndComputePageRectsWithPageSize(frame, pageSizeInPixels))
         return;
index 5010755..a4f6e55 100644 (file)
@@ -1,3 +1,21 @@
+2019-05-28  Brent Fulgham  <bfulgham@apple.com>
+        Protect frames during style and layout changes
+        https://bugs.webkit.org/show_bug.cgi?id=198047
+        <rdar://problem/50954082>
+
+        Reviewed by Zalan Bujtas.
+
+        Be more careful about the scope and lifetime of objects that participate in layout or
+        style updates. If a method decides a layout or style update is needed, it needs to
+        confirm that the elements it was operating on are still valid and needed in the
+        current operation.
+
+        * DOM/DOM.mm:
+        (-[DOMRange renderedImageForcingBlackText:renderedImageForcingBlackText:]):
+        * WebView/WebHTMLView.mm:
+        (-[WebHTMLView _selectionDraggingImage]):
+        (-[WebHTMLView selectionImageForcingBlackText:selectionImageForcingBlackText:]):
+
 2019-05-27  Chris Dumez  <cdumez@apple.com>
 
         Use a strongly-typed identifier for pages
index 81364d0..ab60aff 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
  * Copyright (C) 2006 James G. Speth (speth@end.com)
  * Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
  *
@@ -618,6 +618,8 @@ id <DOMEventTarget> kit(EventTarget* target)
     if (!frame)
         return nil;
 
+    Ref<Frame> protectedFrame(*frame);
+
     // iOS uses CGImageRef for drag images, which doesn't support separate logical/physical sizes.
 #if PLATFORM(MAC)
     RetainPtr<NSImage> renderedImage = createDragImageForRange(*frame, range, forceBlackText);
index 2bfe868..f3e53c6 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2019 Apple Inc. All rights reserved.
  *           (C) 2006, 2007 Graham Dennis (graham.dennis@gmail.com)
  *
  * Redistribution and use in source and binary forms, with or without
@@ -2200,6 +2200,8 @@ ALLOW_DEPRECATED_DECLARATIONS_END
     if (!coreFrame)
         return nil;
 
+    Ref<Frame> protectedCoreFrame(*coreFrame);
+
     TextIndicatorData textIndicator;
     auto dragImage = createDragImageForSelection(*coreFrame, textIndicator);
     [dragImage _web_dissolveToFraction:WebDragImageAlpha];
@@ -6960,6 +6962,8 @@ static CGImageRef selectionImage(Frame* frame, bool forceBlackText)
     if (!coreFrame)
         return nil;
 
+    Ref<Frame> protectedCoreFrame(*coreFrame);
+
 #if PLATFORM(IOS_FAMILY)
     return selectionImage(coreFrame, forceBlackText);
 #else