didFirstVisuallyNonEmptyLayout dispatched too early
https://bugs.webkit.org/show_bug.cgi?id=64412
Reviewed by Darin Adler and Sam Weinig.
Improve the mechanism that dispatches didFirstVisuallyNonEmptyLayout
- Wait until a threshold of characters and pixels has been exceeded before dispatching.
- Wait until stylesheets are loaded (painting is disabled in this case).
* page/FrameView.cpp:
(WebCore::FrameView::reset):
(WebCore::FrameView::performPostLayoutTasks):
* page/FrameView.h:
(WebCore::FrameView::incrementVisuallyNonEmptyCharacterCount):
(WebCore::FrameView::incrementVisuallyNonEmptyPixelCount):
* rendering/RenderImage.cpp:
(WebCore::RenderImage::RenderImage):
(WebCore::RenderImage::imageChanged):
* rendering/RenderImage.h:
* rendering/RenderText.cpp:
(WebCore::RenderText::RenderText):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@90900 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 454fa40..f94b49f 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2011-07-12 Antti Koivisto <antti@apple.com>
+
+ didFirstVisuallyNonEmptyLayout dispatched too early
+ https://bugs.webkit.org/show_bug.cgi?id=64412
+
+ Reviewed by Darin Adler and Sam Weinig.
+
+ Improve the mechanism that dispatches didFirstVisuallyNonEmptyLayout
+
+ - Wait until a threshold of characters and pixels has been exceeded before dispatching.
+ - Wait until stylesheets are loaded (painting is disabled in this case).
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::reset):
+ (WebCore::FrameView::performPostLayoutTasks):
+ * page/FrameView.h:
+ (WebCore::FrameView::incrementVisuallyNonEmptyCharacterCount):
+ (WebCore::FrameView::incrementVisuallyNonEmptyPixelCount):
+ * rendering/RenderImage.cpp:
+ (WebCore::RenderImage::RenderImage):
+ (WebCore::RenderImage::imageChanged):
+ * rendering/RenderImage.h:
+ * rendering/RenderText.cpp:
+ (WebCore::RenderText::RenderText):
+
2011-07-12 MORITA Hajime <morrita@google.com>
[Refactoring][ShadowContentElement] Forwarded node list should be a linked-list.
diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
index a0581d3..de9d7b3 100644
--- a/Source/WebCore/page/FrameView.cpp
+++ b/Source/WebCore/page/FrameView.cpp
@@ -227,6 +227,8 @@
m_lastPaintTime = 0;
m_paintBehavior = PaintBehaviorNormal;
m_isPainting = false;
+ m_visuallyNonEmptyCharacterCount = 0;
+ m_visuallyNonEmptyPixelCount = 0;
m_isVisuallyNonEmpty = false;
m_firstVisuallyNonEmptyLayoutCallbackPending = true;
m_maintainScrollPositionAnchor = 0;
@@ -2042,8 +2044,13 @@
m_firstLayoutCallbackPending = false;
m_frame->loader()->didFirstLayout();
}
-
- if (m_isVisuallyNonEmpty && m_firstVisuallyNonEmptyLayoutCallbackPending) {
+
+ // Ensure that we always send this eventually.
+ if (!m_frame->document()->parsing())
+ m_isVisuallyNonEmpty = true;
+
+ // If the layout was done with pending sheets, we are not in fact visually non-empty yet.
+ if (m_isVisuallyNonEmpty && !m_frame->document()->didLayoutWithPendingStylesheets() && m_firstVisuallyNonEmptyLayoutCallbackPending) {
m_firstVisuallyNonEmptyLayoutCallbackPending = false;
m_frame->loader()->didFirstVisuallyNonEmptyLayout();
}
diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
index e495420..f828319 100644
--- a/Source/WebCore/page/FrameView.h
+++ b/Source/WebCore/page/FrameView.h
@@ -227,6 +227,8 @@
void updateLayoutAndStyleIfNeededRecursive();
void flushDeferredRepaints();
+ void incrementVisuallyNonEmptyCharacterCount(unsigned);
+ void incrementVisuallyNonEmptyPixelCount(const IntSize&);
void setIsVisuallyNonEmpty() { m_isVisuallyNonEmpty = true; }
void forceLayout(bool allowSubtree = false);
@@ -433,6 +435,8 @@
PaintBehavior m_paintBehavior;
bool m_isPainting;
+ unsigned m_visuallyNonEmptyCharacterCount;
+ unsigned m_visuallyNonEmptyPixelCount;
bool m_isVisuallyNonEmpty;
bool m_firstVisuallyNonEmptyLayoutCallbackPending;
@@ -449,6 +453,29 @@
static double s_deferredRepaintDelayIncrementDuringLoading;
};
+inline void FrameView::incrementVisuallyNonEmptyCharacterCount(unsigned count)
+{
+ if (m_isVisuallyNonEmpty)
+ return;
+ m_visuallyNonEmptyCharacterCount += count;
+ // Use a threshold value to prevent very small amounts of visible content from triggering didFirstVisuallyNonEmptyLayout.
+ // The first few hundred characters rarely contain the interesting content of the page.
+ static const unsigned visualCharacterThreshold = 200;
+ if (m_visuallyNonEmptyCharacterCount > visualCharacterThreshold)
+ setIsVisuallyNonEmpty();
+}
+
+inline void FrameView::incrementVisuallyNonEmptyPixelCount(const IntSize& size)
+{
+ if (m_isVisuallyNonEmpty)
+ return;
+ m_visuallyNonEmptyPixelCount += size.width() * size.height();
+ // Use a threshold value to prevent very small amounts of visible content from triggering didFirstVisuallyNonEmptyLayout
+ static const unsigned visualPixelThreshold = 256 * 256;
+ if (m_visuallyNonEmptyPixelCount > visualPixelThreshold)
+ setIsVisuallyNonEmpty();
+}
+
} // namespace WebCore
#endif // FrameView_h
diff --git a/Source/WebCore/rendering/RenderImage.cpp b/Source/WebCore/rendering/RenderImage.cpp
index 849984b..733a219 100644
--- a/Source/WebCore/rendering/RenderImage.cpp
+++ b/Source/WebCore/rendering/RenderImage.cpp
@@ -51,10 +51,9 @@
RenderImage::RenderImage(Node* node)
: RenderReplaced(node, IntSize(0, 0))
, m_needsToSetSizeForAltText(false)
+ , m_didIncrementVisuallyNonEmptyPixelCount(false)
{
updateAltText();
-
- view()->frameView()->setIsVisuallyNonEmpty();
}
RenderImage::~RenderImage()
@@ -140,6 +139,11 @@
if (newImage != m_imageResource->imagePtr() || !newImage)
return;
+
+ if (!m_didIncrementVisuallyNonEmptyPixelCount) {
+ view()->frameView()->incrementVisuallyNonEmptyPixelCount(m_imageResource->imageSize(1.0f));
+ m_didIncrementVisuallyNonEmptyPixelCount = true;
+ }
bool imageSizeChanged = false;
diff --git a/Source/WebCore/rendering/RenderImage.h b/Source/WebCore/rendering/RenderImage.h
index 68910ed..7bed7c5 100644
--- a/Source/WebCore/rendering/RenderImage.h
+++ b/Source/WebCore/rendering/RenderImage.h
@@ -98,6 +98,7 @@
String m_altText;
OwnPtr<RenderImageResource> m_imageResource;
bool m_needsToSetSizeForAltText;
+ bool m_didIncrementVisuallyNonEmptyPixelCount;
friend class RenderImageScaleObserver;
};
diff --git a/Source/WebCore/rendering/RenderText.cpp b/Source/WebCore/rendering/RenderText.cpp
index 8d529c0..72f12b1 100644
--- a/Source/WebCore/rendering/RenderText.cpp
+++ b/Source/WebCore/rendering/RenderText.cpp
@@ -111,10 +111,7 @@
setIsText();
- // FIXME: It would be better to call this only if !m_text->containsOnlyWhitespace().
- // But that might slow things down, and maybe should only be done if visuallyNonEmpty
- // is still false. Not making any change for now, but should consider in the future.
- view()->frameView()->setIsVisuallyNonEmpty();
+ view()->frameView()->incrementVisuallyNonEmptyCharacterCount(m_text.length());
}
#ifndef NDEBUG
