Diff
Modified: trunk/Source/WebCore/ChangeLog (109272 => 109273)
--- trunk/Source/WebCore/ChangeLog 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/ChangeLog 2012-02-29 23:10:06 UTC (rev 109273)
@@ -1,3 +1,67 @@
+2012-02-29 Beth Dakin <bda...@apple.com>
+
+ https://bugs.webkit.org/show_bug.cgi?id=79705
+ didNewFirstVisuallyNonEmptyLayout should be enhanced to look at size instead
+ of a raw tally
+ -and corresponding-
+ <rdar://problem/10821314>
+
+ Reviewed by Dave Hyatt.
+
+ Instead of firing didNewFirstVisuallyNonEmptyLayout() when a raw tally of
+ relevant painted objects has reached a port-defined threshold, this patch
+ looks at the size of those objects with respect to the view area. The patch
+ also looks at relevant objects that cannot yet be fully painted, such as
+ incrementally loading images.
+
+ We no longer need a HashSet for the relevant painted objects since Region can
+ do all of the heavy lifting. We now have a Region for the painted and
+ unpainted regions. We do need a HashSet for the unpainted objects though,
+ because we need to know if a painted object needs to be subtracted from the
+ unpainted region before being added to the painted region.
+ * page/Page.cpp:
+ (WebCore):
+ (WebCore::Page::isCountingRelevantRepaintedObjects):
+ (WebCore::Page::startCountingRelevantRepaintedObjects):
+ (WebCore::Page::resetRelevantPaintedObjectCounter):
+ (WebCore::Page::addRelevantRepaintedObject):
+ (WebCore::Page::addRelevantUnpaintedObject):
+ * page/Page.h:
+ (Page):
+
+ New function on Region iterates through the rects and calculates the total
+ area.
+ * platform/graphics/Region.cpp:
+ (WebCore::Region::totalArea):
+ (WebCore):
+ * platform/graphics/Region.h:
+ (Region):
+
+ Remove code from these classes since they are not actually relevant objects.
+ * rendering/InlineBox.cpp:
+ (WebCore::InlineBox::paint):
+ * rendering/RenderRegion.cpp:
+ (WebCore::RenderRegion::paintReplaced):
+ * rendering/RenderReplaced.cpp:
+ (WebCore::RenderReplaced::paint):
+
+ All of these other callers send a rect that actually represents their size
+ (usually the visualOverflowRect) instead of the paintInfo's paintRect, and
+ they call addRelevantUnpaintedObject when appropriate.
+ * rendering/InlineTextBox.cpp:
+ (WebCore::InlineTextBox::paint):
+ * rendering/RenderEmbeddedObject.cpp:
+ (WebCore::RenderEmbeddedObject::paint):
+ (WebCore::RenderEmbeddedObject::paintReplaced):
+ * rendering/RenderHTMLCanvas.cpp:
+ (WebCore::RenderHTMLCanvas::paintReplaced):
+ * rendering/RenderImage.cpp:
+ (WebCore::RenderImage::paintReplaced):
+ * rendering/RenderVideo.cpp:
+ (WebCore::RenderVideo::paintReplaced):
+ * rendering/svg/RenderSVGRoot.cpp:
+ (WebCore::RenderSVGRoot::paintReplaced):
+
2012-02-29 Joshua Bell <jsb...@chromium.org>
IndexedDB: Resource leak with IDBObjectStore.openCursor
Modified: trunk/Source/WebCore/page/Page.cpp (109272 => 109273)
--- trunk/Source/WebCore/page/Page.cpp 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/page/Page.cpp 2012-02-29 23:10:06 UTC (rev 109273)
@@ -1009,46 +1009,93 @@
}
#endif
+// FIXME: gPaintedObjectCounterThreshold is no longer used for calculating relevant repainted areas,
+// and it should be removed. For the time being, it is useful because it allows us to avoid doing
+// any of this work for ports that don't make sure of didNewFirstVisuallyNonEmptyLayout. We should
+// remove this when we resolve <rdar://problem/10791680> Need to merge didFirstVisuallyNonEmptyLayout
+// and didNewFirstVisuallyNonEmptyLayout
static uint64_t gPaintedObjectCounterThreshold = 0;
+// These are magical constants that might be tweaked over time.
+static double gMinimumPaintedAreaRatio = 0.1;
+static double gMaximumUnpaintedAreaRatio = 0.1;
+
void Page::setRelevantRepaintedObjectsCounterThreshold(uint64_t threshold)
{
gPaintedObjectCounterThreshold = threshold;
}
+bool Page::isCountingRelevantRepaintedObjects() const
+{
+ return m_isCountingRelevantRepaintedObjects && gPaintedObjectCounterThreshold > 0;
+}
+
void Page::startCountingRelevantRepaintedObjects()
{
m_isCountingRelevantRepaintedObjects = true;
- // Clear the HashSet in case we didn't hit the threshold last time.
- m_relevantPaintedRenderObjects.clear();
+ // Reset everything in case we didn't hit the threshold last time.
+ resetRelevantPaintedObjectCounter();
}
+void Page::resetRelevantPaintedObjectCounter()
+{
+ m_relevantUnpaintedRenderObjects.clear();
+ m_relevantPaintedRegion = Region();
+ m_relevantUnpaintedRegion = Region();
+}
+
void Page::addRelevantRepaintedObject(RenderObject* object, const IntRect& objectPaintRect)
{
- if (!m_isCountingRelevantRepaintedObjects)
+ if (!isCountingRelevantRepaintedObjects())
return;
- // We don't need to do anything if there is no counter threshold.
- if (!gPaintedObjectCounterThreshold)
- return;
-
// The objects are only relevant if they are being painted within the viewRect().
if (RenderView* view = object->view()) {
if (!objectPaintRect.intersects(pixelSnappedIntRect(view->viewRect())))
return;
}
- m_relevantPaintedRenderObjects.add(object);
+ // If this object was previously counted as an unpainted object, remove it from that HashSet
+ // and corresponding Region. FIXME: This doesn't do the right thing if the objects overlap.
+ if (m_relevantUnpaintedRenderObjects.contains(object)) {
+ m_relevantUnpaintedRenderObjects.remove(object);
+ m_relevantUnpaintedRegion.subtract(objectPaintRect);
+ }
- if (m_relevantPaintedRenderObjects.size() == static_cast<int>(gPaintedObjectCounterThreshold)) {
+ m_relevantPaintedRegion.unite(objectPaintRect);
+
+ RenderView* view = object->view();
+ if (!view)
+ return;
+
+ float viewArea = view->viewRect().width() * view->viewRect().height();
+ float ratioOfViewThatIsPainted = m_relevantPaintedRegion.totalArea() / viewArea;
+ float ratioOfViewThatIsUnpainted = m_relevantUnpaintedRegion.totalArea() / viewArea;
+
+ if (ratioOfViewThatIsPainted > gMinimumPaintedAreaRatio && ratioOfViewThatIsUnpainted < gMaximumUnpaintedAreaRatio) {
m_isCountingRelevantRepaintedObjects = false;
- m_relevantPaintedRenderObjects.clear();
+ resetRelevantPaintedObjectCounter();
if (Frame* frame = mainFrame())
frame->loader()->didNewFirstVisuallyNonEmptyLayout();
}
}
+void Page::addRelevantUnpaintedObject(RenderObject* object, const IntRect& objectPaintRect)
+{
+ if (!isCountingRelevantRepaintedObjects())
+ return;
+
+ // The objects are only relevant if they are being painted within the viewRect().
+ if (RenderView* view = object->view()) {
+ if (!objectPaintRect.intersects(pixelSnappedIntRect(view->viewRect())))
+ return;
+ }
+
+ m_relevantUnpaintedRenderObjects.add(object);
+ m_relevantUnpaintedRegion.unite(objectPaintRect);
+}
+
Page::PageClients::PageClients()
: chromeClient(0)
, contextMenuClient(0)
Modified: trunk/Source/WebCore/page/Page.h (109272 => 109273)
--- trunk/Source/WebCore/page/Page.h 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/page/Page.h 2012-02-29 23:10:06 UTC (rev 109273)
@@ -27,6 +27,7 @@
#include "PageVisibilityState.h"
#include "PlatformScreen.h"
#include "PlatformString.h"
+#include "Region.h"
#include "Supplementable.h"
#include "ViewportArguments.h"
#include <wtf/Forward.h>
@@ -320,9 +321,12 @@
PlatformDisplayID displayID() const { return m_displayID; }
+ bool isCountingRelevantRepaintedObjects() const;
void setRelevantRepaintedObjectsCounterThreshold(uint64_t);
void startCountingRelevantRepaintedObjects();
+ void resetRelevantPaintedObjectCounter();
void addRelevantRepaintedObject(RenderObject*, const IntRect& objectPaintRect);
+ void addRelevantUnpaintedObject(RenderObject*, const IntRect& objectPaintRect);
private:
void initGroup();
@@ -418,7 +422,9 @@
#endif
PlatformDisplayID m_displayID;
- HashSet<RenderObject*> m_relevantPaintedRenderObjects;
+ HashSet<RenderObject*> m_relevantUnpaintedRenderObjects;
+ Region m_relevantPaintedRegion;
+ Region m_relevantUnpaintedRegion;
bool m_isCountingRelevantRepaintedObjects;
};
Modified: trunk/Source/WebCore/platform/graphics/Region.cpp (109272 => 109273)
--- trunk/Source/WebCore/platform/graphics/Region.cpp 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/platform/graphics/Region.cpp 2012-02-29 23:10:06 UTC (rev 109273)
@@ -77,6 +77,20 @@
return contains(IntRect(point, IntSize(1, 1)));
}
+unsigned Region::totalArea() const
+{
+ Vector<IntRect> rects = this->rects();
+ size_t size = rects.size();
+ unsigned totalArea = 0;
+
+ for (size_t i = 0; i < size; ++i) {
+ IntRect rect = rects[i];
+ totalArea += (rect.width() * rect.height());
+ }
+
+ return totalArea;
+}
+
Region::Shape::Shape()
{
}
Modified: trunk/Source/WebCore/platform/graphics/Region.h (109272 => 109273)
--- trunk/Source/WebCore/platform/graphics/Region.h 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/platform/graphics/Region.h 2012-02-29 23:10:06 UTC (rev 109273)
@@ -52,6 +52,8 @@
bool contains(const IntPoint&) const;
+ unsigned totalArea() const;
+
#ifndef NDEBUG
void dump() const;
#endif
Modified: trunk/Source/WebCore/rendering/InlineBox.cpp (109272 => 109273)
--- trunk/Source/WebCore/rendering/InlineBox.cpp 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/rendering/InlineBox.cpp 2012-02-29 23:10:06 UTC (rev 109273)
@@ -213,11 +213,6 @@
if (!paintInfo.shouldPaintWithinRoot(renderer()) || (paintInfo.phase != PaintPhaseForeground && paintInfo.phase != PaintPhaseSelection))
return;
- if (Frame* frame = renderer()->frame()) {
- if (Page* page = frame->page())
- page->addRelevantRepaintedObject(renderer(), paintInfo.rect);
- }
-
LayoutPoint childPoint = paintOffset;
if (parent()->renderer()->style()->isFlippedBlocksWritingMode()) // Faster than calling containingBlock().
childPoint = renderer()->containingBlock()->flipForWritingModeForChild(toRenderBox(renderer()), childPoint);
Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (109272 => 109273)
--- trunk/Source/WebCore/rendering/InlineTextBox.cpp 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp 2012-02-29 23:10:06 UTC (rev 109273)
@@ -497,8 +497,14 @@
return;
if (Frame* frame = renderer()->frame()) {
- if (Page* page = frame->page())
- page->addRelevantRepaintedObject(renderer(), paintInfo.rect);
+ if (Page* page = frame->page()) {
+ // FIXME: Right now, InlineTextBoxes never call addRelevantUnpaintedObject() even though they might
+ // legitimately be unpainted if they are waiting on a slow-loading web font. We should fix that, and
+ // when we do, we will have to account for the fact the InlineTextBoxes do not always have unique
+ // renderers and Page currently relies on each unpainted object having a unique renderer.
+ if (paintInfo.phase == PaintPhaseForeground)
+ page->addRelevantRepaintedObject(renderer(), IntRect(adjustedPaintOffset.x(), adjustedPaintOffset.y(), logicalWidth(), logicalHeight()));
+ }
}
if (m_truncation != cNoTruncation) {
Modified: trunk/Source/WebCore/rendering/RenderEmbeddedObject.cpp (109272 => 109273)
--- trunk/Source/WebCore/rendering/RenderEmbeddedObject.cpp 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/rendering/RenderEmbeddedObject.cpp 2012-02-29 23:10:06 UTC (rev 109273)
@@ -139,11 +139,20 @@
void RenderEmbeddedObject::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
{
+ Page* page = 0;
+ if (Frame* frame = this->frame())
+ page = frame->page();
+
if (pluginCrashedOrWasMissing()) {
+ if (page && paintInfo.phase == PaintPhaseForeground)
+ page->addRelevantUnpaintedObject(this, visualOverflowRect());
RenderReplaced::paint(paintInfo, paintOffset);
return;
}
-
+
+ if (page && paintInfo.phase == PaintPhaseForeground)
+ page->addRelevantRepaintedObject(this, visualOverflowRect());
+
RenderPart::paint(paintInfo, paintOffset);
}
@@ -167,11 +176,6 @@
float textWidth;
if (!getReplacementTextGeometry(paintOffset, contentRect, path, replacementTextRect, font, run, textWidth))
return;
-
- if (Frame* frame = this->frame()) {
- if (Page* page = frame->page())
- page->addRelevantRepaintedObject(this, paintInfo.rect);
- }
GraphicsContextStateSaver stateSaver(*context);
context->clip(contentRect);
Modified: trunk/Source/WebCore/rendering/RenderHTMLCanvas.cpp (109272 => 109273)
--- trunk/Source/WebCore/rendering/RenderHTMLCanvas.cpp 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/rendering/RenderHTMLCanvas.cpp 2012-02-29 23:10:06 UTC (rev 109273)
@@ -58,13 +58,16 @@
void RenderHTMLCanvas::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
{
+ LayoutRect rect = contentBoxRect();
+ rect.moveBy(paintOffset);
+
if (Frame* frame = this->frame()) {
- if (Page* page = frame->page())
- page->addRelevantRepaintedObject(this, paintInfo.rect);
+ if (Page* page = frame->page()) {
+ if (paintInfo.phase == PaintPhaseForeground)
+ page->addRelevantRepaintedObject(this, rect);
+ }
}
- LayoutRect rect = contentBoxRect();
- rect.moveBy(paintOffset);
bool useLowQualityScale = style()->imageRendering() == ImageRenderingOptimizeContrast;
static_cast<HTMLCanvasElement*>(node())->paint(paintInfo.context, rect, useLowQualityScale);
}
Modified: trunk/Source/WebCore/rendering/RenderImage.cpp (109272 => 109273)
--- trunk/Source/WebCore/rendering/RenderImage.cpp 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/rendering/RenderImage.cpp 2012-02-29 23:10:06 UTC (rev 109273)
@@ -264,10 +264,17 @@
GraphicsContext* context = paintInfo.context;
+ Page* page = 0;
+ if (Frame* frame = this->frame())
+ page = frame->page();
+
if (!m_imageResource->hasImage() || m_imageResource->errorOccurred()) {
if (paintInfo.phase == PaintPhaseSelection)
return;
+ if (page && paintInfo.phase == PaintPhaseForeground)
+ page->addRelevantUnpaintedObject(this, visualOverflowRect());
+
if (cWidth > 2 && cHeight > 2) {
// Draw an outline rect where the image should be.
context->setStrokeStyle(SolidStroke);
@@ -325,12 +332,19 @@
}
} else if (m_imageResource->hasImage() && cWidth > 0 && cHeight > 0) {
RefPtr<Image> img = m_imageResource->image(cWidth, cHeight);
- if (!img || img->isNull())
+ if (!img || img->isNull()) {
+ if (page && paintInfo.phase == PaintPhaseForeground)
+ page->addRelevantUnpaintedObject(this, visualOverflowRect());
return;
+ }
- if (Frame* frame = this->frame()) {
- if (Page* page = frame->page())
- page->addRelevantRepaintedObject(this, paintInfo.rect);
+ if (page && paintInfo.phase == PaintPhaseForeground) {
+ // For now, count images as unpainted if they are still progressively loading. We may want
+ // to refine this in the future to account for the portion of the image that has painted.
+ if (cachedImage()->isLoading())
+ page->addRelevantUnpaintedObject(this, visualOverflowRect());
+ else
+ page->addRelevantRepaintedObject(this, visualOverflowRect());
}
#if PLATFORM(MAC)
Modified: trunk/Source/WebCore/rendering/RenderRegion.cpp (109272 => 109273)
--- trunk/Source/WebCore/rendering/RenderRegion.cpp 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/rendering/RenderRegion.cpp 2012-02-29 23:10:06 UTC (rev 109273)
@@ -139,11 +139,6 @@
if (!m_flowThread || !isValid())
return;
- if (Frame* frame = this->frame()) {
- if (Page* page = frame->page())
- page->addRelevantRepaintedObject(this, paintInfo.rect);
- }
-
setRegionBoxesRegionStyle();
m_flowThread->paintIntoRegion(paintInfo, this, LayoutPoint(paintOffset.x() + borderLeft() + paddingLeft(), paintOffset.y() + borderTop() + paddingTop()));
restoreRegionBoxesOriginalStyle();
Modified: trunk/Source/WebCore/rendering/RenderReplaced.cpp (109272 => 109273)
--- trunk/Source/WebCore/rendering/RenderReplaced.cpp 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/rendering/RenderReplaced.cpp 2012-02-29 23:10:06 UTC (rev 109273)
@@ -137,11 +137,6 @@
drawSelectionTint = false;
}
- if (Frame* frame = this->frame()) {
- if (Page* page = frame->page())
- page->addRelevantRepaintedObject(this, paintInfo.rect);
- }
-
bool completelyClippedOut = false;
if (style()->hasBorderRadius()) {
LayoutRect borderRect = LayoutRect(adjustedPaintOffset, size());
Modified: trunk/Source/WebCore/rendering/RenderVideo.cpp (109272 => 109273)
--- trunk/Source/WebCore/rendering/RenderVideo.cpp 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/rendering/RenderVideo.cpp 2012-02-29 23:10:06 UTC (rev 109273)
@@ -198,21 +198,29 @@
MediaPlayer* mediaPlayer = mediaElement()->player();
bool displayingPoster = videoElement()->shouldDisplayPosterImage();
+ Page* page = 0;
+ if (Frame* frame = this->frame())
+ page = frame->page();
+
if (!displayingPoster) {
- if (!mediaPlayer)
+ if (!mediaPlayer) {
+ if (page && paintInfo.phase == PaintPhaseForeground)
+ page->addRelevantUnpaintedObject(this, visualOverflowRect());
return;
+ }
updatePlayer();
}
LayoutRect rect = videoBox();
- if (rect.isEmpty())
+ if (rect.isEmpty()) {
+ if (page && paintInfo.phase == PaintPhaseForeground)
+ page->addRelevantUnpaintedObject(this, visualOverflowRect());
return;
+ }
rect.moveBy(paintOffset);
- if (Frame* frame = this->frame()) {
- if (Page* page = frame->page())
- page->addRelevantRepaintedObject(this, paintInfo.rect);
- }
+ if (page && paintInfo.phase == PaintPhaseForeground)
+ page->addRelevantRepaintedObject(this, visualOverflowRect());
if (displayingPoster)
paintIntoRect(paintInfo.context, rect);
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp (109272 => 109273)
--- trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2012-02-29 23:08:25 UTC (rev 109272)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2012-02-29 23:10:06 UTC (rev 109273)
@@ -249,17 +249,22 @@
if (paintInfo.context->paintingDisabled())
return;
+ Page* page = 0;
+ if (Frame* frame = this->frame())
+ page = frame->page();
+
// Don't paint if we don't have kids, except if we have filters we should paint those.
if (!firstChild()) {
SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this);
- if (!resources || !resources->filter())
+ if (!resources || !resources->filter()) {
+ if (page && paintInfo.phase == PaintPhaseForeground)
+ page->addRelevantUnpaintedObject(this, visualOverflowRect());
return;
+ }
}
- if (Frame* frame = this->frame()) {
- if (Page* page = frame->page())
- page->addRelevantRepaintedObject(this, paintInfo.rect);
- }
+ if (page && paintInfo.phase == PaintPhaseForeground)
+ page->addRelevantRepaintedObject(this, visualOverflowRect());
// Make a copy of the PaintInfo because applyTransform will modify the damage rect.
PaintInfo childPaintInfo(paintInfo);
