Unreviewed, rolling out r237785.
authorryanhaddad@apple.com <ryanhaddad@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Nov 2018 18:07:06 +0000 (18:07 +0000)
committerryanhaddad@apple.com <ryanhaddad@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Nov 2018 18:07:06 +0000 (18:07 +0000)
Introduced layout test and API test failures on macOS and iOS.

Reverted changeset:

"[iOS] Issue initial paint soon after the visuallyNonEmpty
milestone is fired."
https://bugs.webkit.org/show_bug.cgi?id=191078
https://trac.webkit.org/changeset/237785

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

17 files changed:
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/page/ChromeClient.h
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/platform/graphics/FontCascade.h
Source/WebCore/rendering/RenderText.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
Source/WebKit/WebProcess/WebPage/AcceleratedDrawingArea.cpp
Source/WebKit/WebProcess/WebPage/AcceleratedDrawingArea.h
Source/WebKit/WebProcess/WebPage/DrawingArea.h
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm
Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h
Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm

index 0dacdf7..c07afc5 100644 (file)
@@ -1,3 +1,16 @@
+2018-11-05  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r237785.
+
+        Introduced layout test and API test failures on macOS and iOS.
+
+        Reverted changeset:
+
+        "[iOS] Issue initial paint soon after the visuallyNonEmpty
+        milestone is fired."
+        https://bugs.webkit.org/show_bug.cgi?id=191078
+        https://trac.webkit.org/changeset/237785
+
 2018-11-05  Thibault Saunier  <tsaunier@igalia.com>
 
         [GStreamer][WebRTC] Error out when simulcast is activated
index 338e1b5..d0afeae 100644 (file)
@@ -1807,9 +1807,7 @@ void Document::scheduleStyleRecalc()
     // FIXME: Why on earth is this here? This is clearly misplaced.
     invalidateAccessKeyMap();
     
-    auto throttleStyleRecalc = !m_pendingStyleRecalcShouldForce && page() && page()->chrome().client().layerFlushThrottlingIsActive();
-    const auto styleRecalcDelay = 50_ms;
-    m_styleRecalcTimer.startOneShot(throttleStyleRecalc ? styleRecalcDelay : 0_s);
+    m_styleRecalcTimer.startOneShot(0_s);
 
     InspectorInstrumentation::didScheduleStyleRecalculation(*this);
 }
@@ -3033,8 +3031,6 @@ bool Document::shouldScheduleLayout()
         return false;
     if (styleScope().hasPendingSheetsBeforeBody())
         return false;
-    if (page() && page()->chrome().client().layerFlushThrottlingIsActive())
-        return false;
 
     return true;
 }
index a6dd873..712ca85 100644 (file)
@@ -333,7 +333,6 @@ public:
     
     // Returns true if layer tree updates are disabled.
     virtual bool layerTreeStateIsFrozen() const { return false; }
-    virtual bool layerFlushThrottlingIsActive() const { return false; }
 
     virtual bool adjustLayerFlushThrottling(LayerFlushThrottleState::Flags) { return false; }
 
index 4847c75..3e6246a 100644 (file)
@@ -37,7 +37,6 @@
 #include "DOMWindow.h"
 #include "DebugPageOverlays.h"
 #include "DeprecatedGlobalSettings.h"
-#include "DocumentLoader.h"
 #include "DocumentMarkerController.h"
 #include "EventHandler.h"
 #include "EventNames.h"
@@ -58,7 +57,6 @@
 #include "HTMLIFrameElement.h"
 #include "HTMLNames.h"
 #include "HTMLObjectElement.h"
-#include "HTMLParserIdioms.h"
 #include "HTMLPlugInImageElement.h"
 #include "ImageDocument.h"
 #include "InspectorClient.h"
@@ -89,7 +87,6 @@
 #include "RuntimeEnabledFeatures.h"
 #include "SVGDocument.h"
 #include "SVGSVGElement.h"
-#include "ScriptRunner.h"
 #include "ScriptedAnimationController.h"
 #include "ScrollAnimator.h"
 #include "ScrollingCoordinator.h"
@@ -300,7 +297,7 @@ void FrameView::resetLayoutMilestones()
     m_renderedSignificantAmountOfText = false;
     m_visuallyNonEmptyCharacterCount = 0;
     m_visuallyNonEmptyPixelCount = 0;
-    m_textRendererCountForVisuallyNonEmptyCharacters = 0;
+    m_renderTextCountForVisuallyNonEmptyCharacters = 0;
 }
 
 void FrameView::removeFromAXObjectCache()
@@ -4352,31 +4349,6 @@ void FrameView::updateLayoutAndStyleIfNeededRecursive()
     ASSERT(!needsLayout());
 }
 
-void FrameView::incrementVisuallyNonEmptyCharacterCount(const String& inlineText)
-{
-    if (m_isVisuallyNonEmpty && m_renderedSignificantAmountOfText)
-        return;
-
-    ++m_textRendererCountForVisuallyNonEmptyCharacters;
-
-    auto nonWhitespaceLength = [](auto& inlineText) {
-        auto length = inlineText.length();
-        for (unsigned i = 0; i < inlineText.length(); ++i) {
-            if (isNotHTMLSpace(inlineText[i]))
-                continue;
-            --length;
-        }
-        return length;
-    };
-    m_visuallyNonEmptyCharacterCount += nonWhitespaceLength(inlineText);
-
-    if (!m_isVisuallyNonEmpty && m_visuallyNonEmptyCharacterCount > visualCharacterThreshold)
-        updateIsVisuallyNonEmpty();
-
-    if (!m_renderedSignificantAmountOfText)
-        updateSignificantRenderedTextMilestoneIfNeeded();
-}
-
 static bool elementOverflowRectIsLargerThanThreshold(const Element& element)
 {
     // Require the document to grow a bit.
@@ -4395,63 +4367,23 @@ bool FrameView::qualifiesAsVisuallyNonEmpty() const
     if (!documentElement || !documentElement->renderer())
         return false;
 
+    // Ensure that we always get marked visually non-empty eventually.
+    if (!frame().document()->parsing() && frame().loader().stateMachine().committedFirstRealDocumentLoad())
+        return true;
+
     // FIXME: We should also ignore renderers with non-final style.
     if (frame().document()->styleScope().hasPendingSheetsBeforeBody())
         return false;
 
-    auto isVisible = [](const Element* element) {
-        if (!element || !element->renderer())
-            return false;
-        if (!element->renderer()->opacity())
-            return false;
-        return element->renderer()->style().visibility() == Visibility::Visible;
-    };
-
-    if (!isVisible(documentElement))
-        return false;
-
-    if (!isVisible(frame().document()->body()))
-        return false;
-
     if (!elementOverflowRectIsLargerThanThreshold(*documentElement))
         return false;
 
     // The first few hundred characters rarely contain the interesting content of the page.
     if (m_visuallyNonEmptyCharacterCount > visualCharacterThreshold)
         return true;
-
     // Use a threshold value to prevent very small amounts of visible content from triggering didFirstVisuallyNonEmptyLayout
     if (m_visuallyNonEmptyPixelCount > visualPixelThreshold)
         return true;
-
-    auto isMoreContentExpected = [&]() {
-        // Pending css/javascript/font loading/processing means we should wait a little longer.
-        auto hasPendingScriptExecution = frame().document()->scriptRunner() && frame().document()->scriptRunner()->hasPendingScripts();
-        if (hasPendingScriptExecution)
-            return true;
-
-        auto* documentLoader = frame().loader().documentLoader();
-        if (!documentLoader)
-            return false;
-
-        auto& resourceLoader = documentLoader->cachedResourceLoader();
-        if (!resourceLoader.requestCount())
-            return false;
-
-        auto& resources = resourceLoader.allCachedResources();
-        for (auto& resource : resources) {
-            if (resource.value->isLoaded())
-                continue;
-            if (resource.value->type() == CachedResource::Type::CSSStyleSheet || resource.value->type() == CachedResource::Type::Script || resource.value->type() == CachedResource::Type::FontResource)
-                return true;
-        }
-        return false;
-    };
-
-    // Finished parsing the main document and we still don't yet have enough content. Check if we might be getting some more.
-    if (frame().loader().stateMachine().committedFirstRealDocumentLoad() && !frame().document()->parsing())
-        return isMoreContentExpected();
-
     return false;
 }
 
@@ -4474,7 +4406,7 @@ void FrameView::updateSignificantRenderedTextMilestoneIfNeeded()
     if (m_visuallyNonEmptyCharacterCount < characterThreshold)
         return;
 
-    if (!m_textRendererCountForVisuallyNonEmptyCharacters || m_visuallyNonEmptyCharacterCount / static_cast<float>(m_textRendererCountForVisuallyNonEmptyCharacters) < meanLength)
+    if (!m_renderTextCountForVisuallyNonEmptyCharacters || m_visuallyNonEmptyCharacterCount / static_cast<float>(m_renderTextCountForVisuallyNonEmptyCharacters) < meanLength)
         return;
 
     m_renderedSignificantAmountOfText = true;
index d912102..b1377f3 100644 (file)
@@ -393,7 +393,7 @@ public:
     
     WEBCORE_EXPORT void updateLayoutAndStyleIfNeededRecursive();
 
-    void incrementVisuallyNonEmptyCharacterCount(const String&);
+    void incrementVisuallyNonEmptyCharacterCount(unsigned);
     void incrementVisuallyNonEmptyPixelCount(const IntSize&);
     void updateIsVisuallyNonEmpty();
     void updateSignificantRenderedTextMilestoneIfNeeded();
@@ -870,7 +870,7 @@ private:
     bool m_isVisuallyNonEmpty;
     bool m_firstVisuallyNonEmptyLayoutCallbackPending;
 
-    unsigned m_textRendererCountForVisuallyNonEmptyCharacters { 0 };
+    unsigned m_renderTextCountForVisuallyNonEmptyCharacters;
     bool m_renderedSignificantAmountOfText;
     bool m_significantRenderedTextMilestonePending;
 
@@ -937,6 +937,18 @@ private:
     FrameViewLayoutContext m_layoutContext;
 };
 
+inline void FrameView::incrementVisuallyNonEmptyCharacterCount(unsigned count)
+{
+    if (m_isVisuallyNonEmpty && m_renderedSignificantAmountOfText)
+        return;
+    m_visuallyNonEmptyCharacterCount += count;
+    ++m_renderTextCountForVisuallyNonEmptyCharacters;
+    if (!m_isVisuallyNonEmpty && m_visuallyNonEmptyCharacterCount > visualCharacterThreshold)
+        updateIsVisuallyNonEmpty();
+    if (!m_renderedSignificantAmountOfText)
+        updateSignificantRenderedTextMilestoneIfNeeded();
+}
+
 inline void FrameView::incrementVisuallyNonEmptyPixelCount(const IntSize& size)
 {
     if (m_isVisuallyNonEmpty)
index aacf4ea..970698a 100644 (file)
@@ -269,9 +269,9 @@ public:
 
     bool useBackslashAsYenSymbol() const { return m_useBackslashAsYenSymbol; }
     FontCascadeFonts* fonts() const { return m_fonts.get(); }
-    bool isLoadingCustomFonts() const;
 
 private:
+    bool isLoadingCustomFonts() const;
 
     bool advancedTextRenderingMode() const
     {
index 42257f7..6869f9f 100644 (file)
@@ -199,16 +199,7 @@ inline RenderText::RenderText(Node& node, const String& text)
     ASSERT(!m_text.isNull());
     setIsText();
     m_canUseSimpleFontCodePath = computeCanUseSimpleFontCodePath();
-
-    // FIXME: Find out how to increment the visually non empty character count when the font becomes available.
-    auto isTextVisible = false;
-    if (auto* parentElement = node.parentElement()) {
-        auto* style = parentElement->renderer() ? &parentElement->renderer()->style() : nullptr;
-        isTextVisible = style && style->visibility() == Visibility::Visible && !style->fontCascade().isLoadingCustomFonts();
-    }
-
-    if (isTextVisible)
-        view().frameView().incrementVisuallyNonEmptyCharacterCount(text);
+    view().frameView().incrementVisuallyNonEmptyCharacterCount(text.impl()->length());
 }
 
 RenderText::RenderText(Text& textNode, const String& text)
index 7051c11..489a5f7 100644 (file)
@@ -1,3 +1,16 @@
+2018-11-05  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r237785.
+
+        Introduced layout test and API test failures on macOS and iOS.
+
+        Reverted changeset:
+
+        "[iOS] Issue initial paint soon after the visuallyNonEmpty
+        milestone is fired."
+        https://bugs.webkit.org/show_bug.cgi?id=191078
+        https://trac.webkit.org/changeset/237785
+
 2018-11-05  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r237784 and r237788.
index 54b778a..879b10a 100644 (file)
@@ -931,14 +931,6 @@ bool WebChromeClient::layerTreeStateIsFrozen() const
     return false;
 }
 
-bool WebChromeClient::layerFlushThrottlingIsActive() const
-{
-    if (m_page.drawingArea())
-        return m_page.drawingArea()->layerFlushThrottlingIsActive();
-
-    return false;
-}
-
 #if ENABLE(ASYNC_SCROLLING)
 
 RefPtr<ScrollingCoordinator> WebChromeClient::createScrollingCoordinator(Page& page) const
index 4ebc479..d390a87 100644 (file)
@@ -242,7 +242,6 @@ private:
     }
 
     bool layerTreeStateIsFrozen() const final;
-    bool layerFlushThrottlingIsActive() const final;
 
 #if ENABLE(ASYNC_SCROLLING)
     RefPtr<WebCore::ScrollingCoordinator> createScrollingCoordinator(WebCore::Page&) const final;
index 9920465..7cf8079 100644 (file)
@@ -223,10 +223,6 @@ void AcceleratedDrawingArea::scheduleCompositingLayerFlush()
         m_layerTreeHost->scheduleLayerFlush();
 }
 
-void AcceleratedDrawingArea::scheduleInitialDeferredPaint()
-{
-}
-
 void AcceleratedDrawingArea::scheduleCompositingLayerFlushImmediately()
 {
     scheduleCompositingLayerFlush();
index 70abe43..6d68b5b 100644 (file)
@@ -56,7 +56,6 @@ protected:
 
     WebCore::GraphicsLayerFactory* graphicsLayerFactory() override;
     void setRootCompositingLayer(WebCore::GraphicsLayer*) override;
-    void scheduleInitialDeferredPaint() override;
     void scheduleCompositingLayerFlush() override;
     void scheduleCompositingLayerFlushImmediately() override;
 
index 4632b14..44161c1 100644 (file)
@@ -84,7 +84,6 @@ public:
     virtual bool forceRepaintAsync(CallbackID) { return false; }
     virtual void setLayerTreeStateIsFrozen(bool) { }
     virtual bool layerTreeStateIsFrozen() const { return false; }
-    virtual bool layerFlushThrottlingIsActive() const { return false; }
     virtual LayerTreeHost* layerTreeHost() const { return 0; }
 
     virtual void setPaintingEnabled(bool) { }
@@ -112,7 +111,6 @@ public:
     virtual WebCore::GraphicsLayerFactory* graphicsLayerFactory() { return nullptr; }
     virtual void setRootCompositingLayer(WebCore::GraphicsLayer*) = 0;
     virtual void scheduleCompositingLayerFlush() = 0;
-    virtual void scheduleInitialDeferredPaint() = 0;
     virtual void scheduleCompositingLayerFlushImmediately() = 0;
 
 #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
index 0194159..d342728 100644 (file)
@@ -63,7 +63,6 @@ private:
 
     WebCore::GraphicsLayerFactory* graphicsLayerFactory() override;
     void setRootCompositingLayer(WebCore::GraphicsLayer*) override;
-    void scheduleInitialDeferredPaint() override;
     void scheduleCompositingLayerFlush() override;
     void scheduleCompositingLayerFlushImmediately() override;
     void attachViewOverlayGraphicsLayer(WebCore::Frame*, WebCore::GraphicsLayer*) override;
@@ -80,8 +79,6 @@ private:
     bool supportsAsyncScrolling() override { return true; }
 
     void setLayerTreeStateIsFrozen(bool) override;
-    bool layerTreeStateIsFrozen() const override { return m_isFlushingSuspended; }
-    bool layerFlushThrottlingIsActive() const override { return m_initialDeferredPaintTimer.isActive() || (m_isThrottlingLayerFlushes && m_layerFlushTimer.isActive()); }
 
     void forceRepaint() override;
     bool forceRepaintAsync(CallbackID) override { return false; }
@@ -114,7 +111,6 @@ private:
     void updateScrolledExposedRect();
     void updateRootLayers();
 
-    void flushInitialDeferredPaint();
     void flushLayers();
 
     WebCore::TiledBacking* mainFrameTiledBacking() const;
@@ -148,12 +144,12 @@ private:
     std::optional<WebCore::FloatRect> m_viewExposedRect;
     std::optional<WebCore::FloatRect> m_scrolledViewExposedRect;
 
-    WebCore::Timer m_initialDeferredPaintTimer;
     WebCore::Timer m_layerFlushTimer;
     bool m_isFlushingSuspended { false };
     bool m_hasDeferredFlush { false };
     bool m_isThrottlingLayerFlushes { false };
     bool m_isLayerFlushThrottlingTemporarilyDisabledForInteraction { false };
+    bool m_isInitialThrottledLayerFlush { false };
 
     bool m_waitingForBackingStoreSwap { false };
     bool m_hadFlushDeferredWhileWaitingForBackingStoreSwap { false };
index d692c0c..b553852 100644 (file)
@@ -57,7 +57,6 @@ RemoteLayerTreeDrawingArea::RemoteLayerTreeDrawingArea(WebPage& webPage, const W
     : DrawingArea(DrawingAreaTypeRemoteLayerTree, webPage)
     , m_remoteLayerTreeContext(std::make_unique<RemoteLayerTreeContext>(webPage))
     , m_rootLayer(GraphicsLayer::create(graphicsLayerFactory(), *this))
-    , m_initialDeferredPaintTimer(*this, &RemoteLayerTreeDrawingArea::flushInitialDeferredPaint)
     , m_layerFlushTimer(*this, &RemoteLayerTreeDrawingArea::flushLayers)
 {
     webPage.corePage()->settings().setForceCompositingMode(true);
@@ -186,7 +185,7 @@ void RemoteLayerTreeDrawingArea::setLayerTreeStateIsFrozen(bool isFrozen)
 
     if (!m_isFlushingSuspended && m_hasDeferredFlush) {
         m_hasDeferredFlush = false;
-        scheduleInitialDeferredPaint();
+        scheduleCompositingLayerFlush();
     }
 }
 
@@ -274,23 +273,6 @@ void RemoteLayerTreeDrawingArea::scheduleCompositingLayerFlushImmediately()
     m_layerFlushTimer.startOneShot(0_s);
 }
 
-void RemoteLayerTreeDrawingArea::flushInitialDeferredPaint()
-{
-    flushLayers();
-
-    if (!m_isThrottlingLayerFlushes)
-        return;
-
-    scheduleCompositingLayerFlush();
-}
-
-void RemoteLayerTreeDrawingArea::scheduleInitialDeferredPaint()
-{
-    ASSERT(!m_isFlushingSuspended);
-    ASSERT(!m_layerFlushTimer.isActive());
-    m_initialDeferredPaintTimer.startOneShot(0_s);
-}
-
 void RemoteLayerTreeDrawingArea::scheduleCompositingLayerFlush()
 {
     if (m_isFlushingSuspended) {
@@ -304,11 +286,15 @@ void RemoteLayerTreeDrawingArea::scheduleCompositingLayerFlush()
         return;
     }
 
-    if (m_initialDeferredPaintTimer.isActive() || m_layerFlushTimer.isActive())
+    if (m_layerFlushTimer.isActive())
         return;
 
-    const auto flushDelay = 500_ms;
-    m_layerFlushTimer.startOneShot(m_isThrottlingLayerFlushes ? flushDelay : 0_s);
+    const Seconds initialFlushDelay = 500_ms;
+    const Seconds flushDelay = 1500_ms;
+    Seconds throttleDelay = m_isThrottlingLayerFlushes ? (m_isInitialThrottledLayerFlush ? initialFlushDelay : flushDelay) : 0_s;
+    m_isInitialThrottledLayerFlush = false;
+
+    m_layerFlushTimer.startOneShot(throttleDelay);
 }
 
 bool RemoteLayerTreeDrawingArea::adjustLayerFlushThrottling(WebCore::LayerFlushThrottleState::Flags flags)
@@ -319,6 +305,9 @@ bool RemoteLayerTreeDrawingArea::adjustLayerFlushThrottling(WebCore::LayerFlushT
     bool wasThrottlingLayerFlushes = m_isThrottlingLayerFlushes;
     m_isThrottlingLayerFlushes = flags & WebCore::LayerFlushThrottleState::Enabled;
 
+    if (!wasThrottlingLayerFlushes && m_isThrottlingLayerFlushes)
+        m_isInitialThrottledLayerFlush = true;
+
     // Re-schedule the flush if we stopped throttling.
     if (wasThrottlingLayerFlushes && !m_isThrottlingLayerFlushes && m_layerFlushTimer.isActive()) {
         m_layerFlushTimer.stop();
index 9a2ffd4..4dc35ae 100644 (file)
@@ -65,7 +65,6 @@ private:
     void setLayerTreeStateIsFrozen(bool) override;
     bool layerTreeStateIsFrozen() const override;
     void setRootCompositingLayer(WebCore::GraphicsLayer*) override;
-    void scheduleInitialDeferredPaint() override;
     void scheduleCompositingLayerFlush() override;
     void scheduleCompositingLayerFlushImmediately() override;
 
index 22b15a6..669df30 100644 (file)
@@ -185,10 +185,6 @@ bool TiledCoreAnimationDrawingArea::layerTreeStateIsFrozen() const
     return m_layerTreeStateIsFrozen;
 }
 
-void TiledCoreAnimationDrawingArea::scheduleInitialDeferredPaint()
-{
-}
-
 void TiledCoreAnimationDrawingArea::scheduleCompositingLayerFlush()
 {
     if (m_layerTreeStateIsFrozen)