Title: [228634] releases/WebKitGTK/webkit-2.20/Source
Revision
228634
Author
carlo...@webkit.org
Date
2018-02-19 02:12:07 -0800 (Mon, 19 Feb 2018)

Log Message

Merge r228151 - Release assertion in inlineVideoFrame
https://bugs.webkit.org/show_bug.cgi?id=182513
<rdar://problem/37159363>

Reviewed by Zalan Bujtas.

Source/WebCore:

The bug was caused by the fact it's not always safe to invoke updateLayout even when isSafeToUpdateStyleOrLayout
on a document of a flattened frame on iOS. isSafeToUpdateStyleOrLayout returns true when the frame view is in
the frame-flattening mode to avoid hitting a release asssertion in updateLayout of the frame. However, it's still
not safe to invoke updateLayout on a parent frame in this case.

As a result, inlineVideoFrame (in Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm) invokes updateLayout
even when the top-level document is not safe to update when the video element is in a frame-flattened document.

Fixed this bug by explicitly checking that we still have a live render tree and document hasn't been stopped.
Also replaced other uses of isSafeToUpdateStyleOrLayout by more explicit checks.

* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::updateBackingStore): Made the early exit condition added in r227006 more explicit.
Namely, InspectorDOMAgent::pseudoElementCreated is invoked during style recalc.
* dom/Document.cpp:
(WebCore::isSafeToUpdateStyleOrLayout): Made this local to the file.
(WebCore::Document::updateStyleIfNeeded):
(WebCore::Document::updateLayout):
* dom/Document.h:
* html/MediaElementSession.cpp:
(WebCore::isMainContentForPurposesOfAutoplay): Made the early exit condition added in r227529 more explicit. Don't
update the layout when the render tree had been destroyed or the active DOM objects had been stopped.

Source/WebKit:

Fixed the bug. Don't try to update the layout when there is no live render tree or active DOM objects
had been stopped: i.e. during a document destruction.

* WebProcess/cocoa/VideoFullscreenManager.mm:
(WebKit::inlineVideoFrame):

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog (228633 => 228634)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog	2018-02-19 09:59:48 UTC (rev 228633)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog	2018-02-19 10:12:07 UTC (rev 228634)
@@ -1,3 +1,34 @@
+2018-02-05  Ryosuke Niwa  <rn...@webkit.org>
+
+        Release assertion in inlineVideoFrame
+        https://bugs.webkit.org/show_bug.cgi?id=182513
+        <rdar://problem/37159363>
+
+        Reviewed by Zalan Bujtas.
+
+        The bug was caused by the fact it's not always safe to invoke updateLayout even when isSafeToUpdateStyleOrLayout
+        on a document of a flattened frame on iOS. isSafeToUpdateStyleOrLayout returns true when the frame view is in
+        the frame-flattening mode to avoid hitting a release asssertion in updateLayout of the frame. However, it's still
+        not safe to invoke updateLayout on a parent frame in this case.
+
+        As a result, inlineVideoFrame (in Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm) invokes updateLayout
+        even when the top-level document is not safe to update when the video element is in a frame-flattened document.
+
+        Fixed this bug by explicitly checking that we still have a live render tree and document hasn't been stopped.
+        Also replaced other uses of isSafeToUpdateStyleOrLayout by more explicit checks.
+
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::updateBackingStore): Made the early exit condition added in r227006 more explicit.
+        Namely, InspectorDOMAgent::pseudoElementCreated is invoked during style recalc.
+        * dom/Document.cpp:
+        (WebCore::isSafeToUpdateStyleOrLayout): Made this local to the file.
+        (WebCore::Document::updateStyleIfNeeded):
+        (WebCore::Document::updateLayout):
+        * dom/Document.h:
+        * html/MediaElementSession.cpp:
+        (WebCore::isMainContentForPurposesOfAutoplay): Made the early exit condition added in r227529 more explicit. Don't
+        update the layout when the render tree had been destroyed or the active DOM objects had been stopped.
+
 2018-02-05  Filip Pizlo  <fpi...@apple.com>
 
         Global objects should be able to use TLCs to allocate from different blocks from each other

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AccessibilityObject.cpp (228633 => 228634)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AccessibilityObject.cpp	2018-02-19 09:59:48 UTC (rev 228633)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AccessibilityObject.cpp	2018-02-19 10:12:07 UTC (rev 228634)
@@ -1769,7 +1769,7 @@
     // Updating the layout may delete this object.
     RefPtr<AccessibilityObject> protectedThis(this);
     if (auto* document = this->document()) {
-        if (!document->view()->layoutContext().isInRenderTreeLayout() && !document->inRenderTreeUpdate() && document->isSafeToUpdateStyleOrLayout())
+        if (!document->view()->layoutContext().isInRenderTreeLayout() && !document->inRenderTreeUpdate() && !document->inStyleRecalc())
             document->updateLayoutIgnorePendingStylesheets();
     }
     updateChildrenIfNecessary();

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Document.cpp (228633 => 228634)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Document.cpp	2018-02-19 09:59:48 UTC (rev 228633)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Document.cpp	2018-02-19 10:12:07 UTC (rev 228634)
@@ -1939,10 +1939,11 @@
     return false;
 }
 
-bool Document::isSafeToUpdateStyleOrLayout() const
+static bool isSafeToUpdateStyleOrLayout(const Document& document)
 {
     bool isSafeToExecuteScript = ScriptDisallowedScope::InMainThread::isScriptAllowed();
-    bool isInFrameFlattening = view() && view()->isInChildFrameWithFrameFlattening();
+    auto* frameView = document.view();
+    bool isInFrameFlattening = frameView && frameView->isInChildFrameWithFrameFlattening();
     bool isAssertionDisabled = ScriptDisallowedScope::LayoutAssertionDisableScope::shouldDisable();
     return isSafeToExecuteScript || isInFrameFlattening || !isInWebProcess() || isAssertionDisabled;
 }
@@ -1965,7 +1966,7 @@
     }
 
     // The early exit above for !needsStyleRecalc() is needed when updateWidgetPositions() is called in runOrScheduleAsynchronousTasks().
-    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(isSafeToUpdateStyleOrLayout());
+    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(isSafeToUpdateStyleOrLayout(*this));
 
     resolveStyle();
     return true;
@@ -1981,7 +1982,7 @@
         ASSERT_NOT_REACHED();
         return;
     }
-    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(isSafeToUpdateStyleOrLayout());
+    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(isSafeToUpdateStyleOrLayout(*this));
 
     RenderView::RepaintRegionAccumulator repaintRegionAccumulator(renderView());
 

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Document.h (228633 => 228634)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Document.h	2018-02-19 09:59:48 UTC (rev 228633)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Document.h	2018-02-19 10:12:07 UTC (rev 228634)
@@ -1252,7 +1252,6 @@
 
     bool inStyleRecalc() const { return m_inStyleRecalc; }
     bool inRenderTreeUpdate() const { return m_inRenderTreeUpdate; }
-    WEBCORE_EXPORT bool isSafeToUpdateStyleOrLayout() const;
 
     void updateTextRenderer(Text&, unsigned offsetOfReplacedText, unsigned lengthOfReplacedText);
 

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/html/MediaElementSession.cpp (228633 => 228634)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/html/MediaElementSession.cpp	2018-02-19 09:59:48 UTC (rev 228633)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/html/MediaElementSession.cpp	2018-02-19 10:12:07 UTC (rev 228634)
@@ -695,7 +695,7 @@
 static bool isMainContentForPurposesOfAutoplay(const HTMLMediaElement& element)
 {
     Document& document = element.document();
-    if (element.isSuspended() || !element.hasAudio() || !element.hasVideo())
+    if (!document.hasLivingRenderTree() || document.activeDOMObjectsAreStopped() || element.isSuspended() || !element.hasAudio() || !element.hasVideo())
         return false;
 
     // Elements which have not yet been laid out, or which are not yet in the DOM, cannot be main content.
@@ -715,7 +715,7 @@
         return false;
 
     // Main content elements must be in the main frame.
-    if (!document.frame() || !document.frame()->isMainFrame() || !document.isSafeToUpdateStyleOrLayout())
+    if (!document.frame() || !document.frame()->isMainFrame())
         return false;
 
     MainFrame& mainFrame = document.frame()->mainFrame();

Modified: releases/WebKitGTK/webkit-2.20/Source/WebKit/ChangeLog (228633 => 228634)


--- releases/WebKitGTK/webkit-2.20/Source/WebKit/ChangeLog	2018-02-19 09:59:48 UTC (rev 228633)
+++ releases/WebKitGTK/webkit-2.20/Source/WebKit/ChangeLog	2018-02-19 10:12:07 UTC (rev 228634)
@@ -1,3 +1,17 @@
+2018-02-05  Ryosuke Niwa  <rn...@webkit.org>
+
+        Release assertion in inlineVideoFrame
+        https://bugs.webkit.org/show_bug.cgi?id=182513
+        <rdar://problem/37159363>
+
+        Reviewed by Zalan Bujtas.
+
+        Fixed the bug. Don't try to update the layout when there is no live render tree or active DOM objects
+        had been stopped: i.e. during a document destruction.
+
+        * WebProcess/cocoa/VideoFullscreenManager.mm:
+        (WebKit::inlineVideoFrame):
+
 2018-02-05  Youenn Fablet  <you...@apple.com>
 
         WebsiteDataStore::resolveDirectoriesIfNecessary() should not overwrite its resolved serviceWorkerRegistrationDirectory  if already set

Modified: releases/WebKitGTK/webkit-2.20/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm (228633 => 228634)


--- releases/WebKitGTK/webkit-2.20/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm	2018-02-19 09:59:48 UTC (rev 228633)
+++ releases/WebKitGTK/webkit-2.20/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm	2018-02-19 10:12:07 UTC (rev 228634)
@@ -59,7 +59,7 @@
 static IntRect inlineVideoFrame(HTMLVideoElement& element)
 {
     auto& document = element.document();
-    if (!document.isSafeToUpdateStyleOrLayout())
+    if (!document.hasLivingRenderTree() || document.activeDOMObjectsAreStopped())
         return { };
 
     document.updateLayoutIgnorePendingStylesheets();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to