REGRESSION (r166422): All RenderBox objects grew 104 bytes from adding repaint timers.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 17 May 2014 10:08:47 +0000 (10:08 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 17 May 2014 10:08:47 +0000 (10:08 +0000)
<https://webkit.org/b/133027>
<rdar://problem/16867410>

Instead of storing a rarely-used repaint timer on every RenderBox, store one
on the RenderView, and keep a hash set of renderers needing repaint.

Renderers get a flag tracking whether they have a pending lazy repaint.
This way we can avoid hash lookups in the common case.

Also added a static assertion to catch RenderBox growing in the future.

Reviewed by Antti Koivisto.

* rendering/RenderBox.cpp:
(WebCore::SameSizeAsRenderBox::~SameSizeAsRenderBox):
(WebCore::RenderBox::RenderBox):
(WebCore::RenderBox::~RenderBox):
(WebCore::RenderBox::paintBoxDecorations):
(WebCore::RenderBox::layoutOverflowRectForPropagation):
* rendering/RenderBox.h:
* rendering/RenderElement.cpp:
(WebCore::RenderElement::RenderElement):
* rendering/RenderElement.h:
(WebCore::RenderElement::setRenderBoxNeedsLazyRepaint):
(WebCore::RenderElement::renderBoxNeedsLazyRepaint):
* rendering/RenderView.cpp:
(WebCore::RenderView::RenderView):
(WebCore::RenderView::scheduleLazyRepaint):
(WebCore::RenderView::unscheduleLazyRepaint):
(WebCore::RenderView::lazyRepaintTimerFired):
* rendering/RenderView.h:

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderBox.h
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h
Source/WebCore/rendering/RenderView.cpp
Source/WebCore/rendering/RenderView.h

index 5fae1ce..5605837 100644 (file)
@@ -1,3 +1,38 @@
+2014-05-17  Andreas Kling  <akling@apple.com>
+
+        REGRESSION (r166422): All RenderBox objects grew 104 bytes from adding repaint timers.
+        <https://webkit.org/b/133027>
+        <rdar://problem/16867410>
+
+        Instead of storing a rarely-used repaint timer on every RenderBox, store one
+        on the RenderView, and keep a hash set of renderers needing repaint.
+
+        Renderers get a flag tracking whether they have a pending lazy repaint.
+        This way we can avoid hash lookups in the common case.
+
+        Also added a static assertion to catch RenderBox growing in the future.
+
+        Reviewed by Antti Koivisto.
+
+        * rendering/RenderBox.cpp:
+        (WebCore::SameSizeAsRenderBox::~SameSizeAsRenderBox):
+        (WebCore::RenderBox::RenderBox):
+        (WebCore::RenderBox::~RenderBox):
+        (WebCore::RenderBox::paintBoxDecorations):
+        (WebCore::RenderBox::layoutOverflowRectForPropagation):
+        * rendering/RenderBox.h:
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::RenderElement):
+        * rendering/RenderElement.h:
+        (WebCore::RenderElement::setRenderBoxNeedsLazyRepaint):
+        (WebCore::RenderElement::renderBoxNeedsLazyRepaint):
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::RenderView):
+        (WebCore::RenderView::scheduleLazyRepaint):
+        (WebCore::RenderView::unscheduleLazyRepaint):
+        (WebCore::RenderView::lazyRepaintTimerFired):
+        * rendering/RenderView.h:
+
 2014-05-16  Jer Noble  <jer.noble@apple.com>
 
         [Mac][MSE] setCurrentTime() goes down fastSeek path in MediaPlayerPrivateMediaSourceAVFObjC.
index a21bbad..b66123e 100644 (file)
 
 namespace WebCore {
 
+struct SameSizeAsRenderBox : public RenderBoxModelObject {
+    virtual ~SameSizeAsRenderBox() { }
+    LayoutRect frameRect;
+    LayoutBoxExtent marginBox;
+    LayoutUnit preferredLogicalWidths[2];
+    void* pointers[2];
+};
+
+COMPILE_ASSERT(sizeof(RenderBox) == sizeof(SameSizeAsRenderBox), RenderBox_should_stay_small);
+
 using namespace HTMLNames;
 
 // Used by flexible boxes when flexing this element and by table cells.
@@ -103,7 +113,6 @@ RenderBox::RenderBox(Element& element, PassRef<RenderStyle> style, unsigned base
     , m_minPreferredLogicalWidth(-1)
     , m_maxPreferredLogicalWidth(-1)
     , m_inlineBoxWrapper(0)
-    , m_repaintTimer(this, &RenderBox::repaintTimerFired)
 {
     setIsBox();
 }
@@ -113,14 +122,13 @@ RenderBox::RenderBox(Document& document, PassRef<RenderStyle> style, unsigned ba
     , m_minPreferredLogicalWidth(-1)
     , m_maxPreferredLogicalWidth(-1)
     , m_inlineBoxWrapper(0)
-    , m_repaintTimer(this, &RenderBox::repaintTimerFired)
 {
     setIsBox();
 }
 
 RenderBox::~RenderBox()
 {
-    m_repaintTimer.stop();
+    view().unscheduleLazyRepaint(*this);
     if (hasControlStatesForRenderer(this))
         removeControlStatesForRenderer(this);
 }
@@ -1255,7 +1263,7 @@ void RenderBox::paintBoxDecorations(PaintInfo& paintInfo, const LayoutPoint& pai
     bool themePainted = style().hasAppearance() && !theme().paint(*this, controlStates, paintInfo, paintRect);
 
     if (controlStates && controlStates->needsRepaint())
-        m_repaintTimer.startOneShot(0);
+        view().scheduleLazyRepaint(*this);
 
     if (!themePainted) {
         if (bleedAvoidance == BackgroundBleedBackgroundOverBorder)
@@ -4781,10 +4789,4 @@ LayoutUnit RenderBox::offsetFromLogicalTopOfFirstPage() const
     return containerBlock->offsetFromLogicalTopOfFirstPage() + logicalTop();
 }
 
-void RenderBox::repaintTimerFired(Timer<RenderBox>&)
-{
-    if (!document().inPageCache())
-        this->repaint();
-}
-
 } // namespace WebCore
index 3ece312..84085ef 100644 (file)
@@ -723,12 +723,8 @@ protected:
     RefPtr<RenderOverflow> m_overflow;
 
 private:
-    void repaintTimerFired(Timer<RenderBox>&);
-
     // Used to store state between styleWillChange and styleDidChange
     static bool s_hadOverflowClip;
-
-    Timer<RenderBox> m_repaintTimer;
 };
 
 RENDER_OBJECT_TYPE_CASTS(RenderBox, isBox())
index 99b6f73..d2d8950 100644 (file)
@@ -83,6 +83,7 @@ RenderElement::RenderElement(Element& element, PassRef<RenderStyle> style, unsig
     , m_ancestorLineBoxDirty(false)
     , m_hasInitializedStyle(false)
     , m_renderInlineAlwaysCreatesLineBoxes(false)
+    , m_renderBoxNeedsLazyRepaint(false)
     , m_hasPausedImageAnimations(false)
     , m_firstChild(nullptr)
     , m_lastChild(nullptr)
@@ -96,6 +97,7 @@ RenderElement::RenderElement(Document& document, PassRef<RenderStyle> style, uns
     , m_ancestorLineBoxDirty(false)
     , m_hasInitializedStyle(false)
     , m_renderInlineAlwaysCreatesLineBoxes(false)
+    , m_renderBoxNeedsLazyRepaint(false)
     , m_hasPausedImageAnimations(false)
     , m_firstChild(nullptr)
     , m_lastChild(nullptr)
index 4f5048b..97eb4c7 100644 (file)
@@ -155,6 +155,9 @@ public:
 
     RenderNamedFlowThread* renderNamedFlowThreadWrapper();
 
+    void setRenderBoxNeedsLazyRepaint(bool b) { m_renderBoxNeedsLazyRepaint = b; }
+    bool renderBoxNeedsLazyRepaint() const { return m_renderBoxNeedsLazyRepaint; }
+
 protected:
     enum BaseTypeFlags {
         RenderLayerModelObjectFlag = 1 << 0,
@@ -224,6 +227,7 @@ private:
     bool m_hasInitializedStyle : 1;
 
     bool m_renderInlineAlwaysCreatesLineBoxes : 1;
+    bool m_renderBoxNeedsLazyRepaint : 1;
     bool m_hasPausedImageAnimations : 1;
 
     RenderObject* m_firstChild;
index 71bcb59..edca1aa 100644 (file)
@@ -102,6 +102,7 @@ RenderView::RenderView(Document& document, PassRef<RenderStyle> style)
     , m_selectionEndPos(-1)
     , m_rendererCount(0)
     , m_maximalOutlineSize(0)
+    , m_lazyRepaintTimer(this, &RenderView::lazyRepaintTimerFired)
     , m_pageLogicalHeight(0)
     , m_pageLogicalHeightChanged(false)
     , m_layoutState(nullptr)
@@ -136,6 +137,38 @@ RenderView::~RenderView()
 {
 }
 
+void RenderView::scheduleLazyRepaint(RenderBox& renderer)
+{
+    if (renderer.renderBoxNeedsLazyRepaint())
+        return;
+    renderer.setRenderBoxNeedsLazyRepaint(true);
+    m_renderersNeedingLazyRepaint.add(&renderer);
+    if (!m_lazyRepaintTimer.isActive())
+        m_lazyRepaintTimer.startOneShot(0);
+}
+
+void RenderView::unscheduleLazyRepaint(RenderBox& renderer)
+{
+    if (!renderer.renderBoxNeedsLazyRepaint())
+        return;
+    renderer.setRenderBoxNeedsLazyRepaint(false);
+    m_renderersNeedingLazyRepaint.remove(&renderer);
+    if (m_renderersNeedingLazyRepaint.isEmpty())
+        m_lazyRepaintTimer.stop();
+}
+
+void RenderView::lazyRepaintTimerFired(Timer<RenderView>&)
+{
+    bool shouldRepaint = !document().inPageCache();
+
+    for (auto& renderer : m_renderersNeedingLazyRepaint) {
+        if (shouldRepaint)
+            renderer->repaint();
+        renderer->setRenderBoxNeedsLazyRepaint(false);
+    }
+    m_renderersNeedingLazyRepaint.clear();
+}
+
 bool RenderView::hitTest(const HitTestRequest& request, HitTestResult& result)
 {
     return hitTest(request, result.hitTestLocation(), result);
index 9696533..cadf7bd 100644 (file)
@@ -246,6 +246,9 @@ public:
         bool m_wasAccumulatingRepaintRegion;
     };
 
+    void scheduleLazyRepaint(RenderBox&);
+    void unscheduleLazyRepaint(RenderBox&);
+
 protected:
     virtual void mapLocalToContainer(const RenderLayerModelObject* repaintContainer, TransformState&, MapCoordinatesFlags = ApplyContainerFlip, bool* wasFixed = 0) const override;
     virtual const RenderObject* pushMappingToContainer(const RenderLayerModelObject* ancestorToStopAt, RenderGeometryMap&) const override;
@@ -339,6 +342,11 @@ private:
 
     bool shouldUsePrintingLayout() const;
 
+    void lazyRepaintTimerFired(Timer<RenderView>&);
+
+    Timer<RenderView> m_lazyRepaintTimer;
+    HashSet<RenderBox*> m_renderersNeedingLazyRepaint;
+
     std::unique_ptr<ImageQualityController> m_imageQualityController;
     LayoutUnit m_pageLogicalHeight;
     bool m_pageLogicalHeightChanged;