Changeset 228634 in webkit

Timestamp:

Feb 19, 2018 2:12:07 AM (6 years ago)

Author:

Carlos Garcia Campos

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):

Location:

releases/WebKitGTK/webkit-2.20/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/accessibility/AccessibilityObject.cpp (modified) (1 diff)
• WebCore/dom/Document.cpp (modified) (3 diffs)
• WebCore/dom/Document.h (modified) (1 diff)
• WebCore/html/MediaElementSession.cpp (modified) (2 diffs)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/WebProcess/cocoa/VideoFullscreenManager.mm (modified) (1 diff)

releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog

@@ +1 @@
+2018-02-05  Ryosuke Niwa  <rniwa@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  <fpizlo@apple.com>
 

releases/WebKitGTK/webkit-2.20/Source/WebCore/accessibility/AccessibilityObject.cpp

@@ -1770 +1770 @@
     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();
     }

releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Document.cpp

@@ -1940 +1940 @@
 }
 
-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;
@@ -1966 +1967 @@
 
     // 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();
@@ -1982 +1983 @@
         return;
     }
-    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(isSafeToUpdateStyleOrLayout());
+    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(isSafeToUpdateStyleOrLayout(*this));
 
     RenderView::RepaintRegionAccumulator repaintRegionAccumulator(renderView());

releases/WebKitGTK/webkit-2.20/Source/WebCore/dom/Document.h

@@ -1253 +1253 @@
     bool inStyleRecalc() const { return m_inStyleRecalc; }
     bool inRenderTreeUpdate() const { return m_inRenderTreeUpdate; }
-    WEBCORE_EXPORT bool isSafeToUpdateStyleOrLayout() const;
 
     void updateTextRenderer(Text&, unsigned offsetOfReplacedText, unsigned lengthOfReplacedText);

releases/WebKitGTK/webkit-2.20/Source/WebCore/html/MediaElementSession.cpp

@@ -696 +696 @@
 {
     Document& document = element.document();
-    if (element.isSuspended() || !element.hasAudio() || !element.hasVideo())
+    if (!document.hasLivingRenderTree() || document.activeDOMObjectsAreStopped() || element.isSuspended() || !element.hasAudio() || !element.hasVideo())
         return false;
 
@@ -716 +716 @@
 
     // 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;
 

releases/WebKitGTK/webkit-2.20/Source/WebKit/ChangeLog

@@ +1 @@
+2018-02-05  Ryosuke Niwa  <rniwa@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  <youenn@apple.com>
 

releases/WebKitGTK/webkit-2.20/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm

@@ -60 +60 @@
 {
     auto& document = element.document();
-    if (!document.isSafeToUpdateStyleOrLayout())
+    if (!document.hasLivingRenderTree() || document.activeDOMObjectsAreStopped())
         return { };