Relevant repainted objects callback is inaccurate and inconsistent for PDF
authorbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Mar 2015 23:06:27 +0000 (23:06 +0000)
committerbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Mar 2015 23:06:27 +0000 (23:06 +0000)
documents
https://bugs.webkit.org/show_bug.cgi?id=143118
-and corresponding-
rdar://problem/13371582

Reviewed by Tim Horton.

Investigating this bug resulted in finding two things that should change for the
relevant repainted objects heuristic. First, we should not count any objects
painted while updating control tints. And secondly, we should not use it at all
for plugin documents. In other documents, we count the plugin area as “painted”
when we get to paint whether or not the plugin has actually loaded. This is
intentional because it allows us to account for chunks of the page that will be
filled in by possibly slow-loading ads. However, if the plugin is the whole
document, then the heuristic just doesn’t make any sense and it leads to
inconsistent behavior at different window sizes. So we’ll only count plugins when
the document is not a plugin document.

Don’t count objects during this paint!
* page/FrameView.cpp:
(WebCore::FrameView::updateControlTints):
* page/Page.h:
(WebCore::Page::setIsCountingRelevantRepaintedObjects):

Make sure the document is not a plugin document.
* rendering/RenderEmbeddedObject.cpp:
(WebCore::RenderEmbeddedObject::paint):

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

Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/Page.h
Source/WebCore/rendering/RenderEmbeddedObject.cpp

index c9a7736..3c0f91b 100644 (file)
@@ -1,3 +1,34 @@
+2015-03-26  Beth Dakin  <bdakin@apple.com>
+
+        Relevant repainted objects callback is inaccurate and inconsistent for PDF 
+        documents
+        https://bugs.webkit.org/show_bug.cgi?id=143118
+        -and corresponding-
+        rdar://problem/13371582
+
+        Reviewed by Tim Horton.
+
+        Investigating this bug resulted in finding two things that should change for the 
+        relevant repainted objects heuristic. First, we should not count any objects 
+        painted while updating control tints. And secondly, we should not use it at all 
+        for plugin documents. In other documents, we count the plugin area as “painted” 
+        when we get to paint whether or not the plugin has actually loaded. This is 
+        intentional because it allows us to account for chunks of the page that will be 
+        filled in by possibly slow-loading ads. However, if the plugin is the whole 
+        document, then the heuristic just doesn’t make any sense and it leads to 
+        inconsistent behavior at different window sizes. So we’ll only count plugins when 
+        the document is not a plugin document. 
+
+        Don’t count objects during this paint!
+        * page/FrameView.cpp:
+        (WebCore::FrameView::updateControlTints):
+        * page/Page.h:
+        (WebCore::Page::setIsCountingRelevantRepaintedObjects):
+
+        Make sure the document is not a plugin document.
+        * rendering/RenderEmbeddedObject.cpp:
+        (WebCore::RenderEmbeddedObject::paint):
+
 2015-03-26  Alex Christensen  <achristensen@webkit.org>
 
         Progress towards CMake on Mac.
index cb415d0..9c657bc 100644 (file)
@@ -3737,9 +3737,20 @@ void FrameView::updateControlTints()
     if (frame().document()->url().isEmpty())
         return;
 
+    // As noted above, this is a "fake" paint, so we should pause counting relevant repainted objects.
+    Page* page = frame().page();
+    bool isCurrentlyCountingRelevantRepaintedObject = false;
+    if (page) {
+        isCurrentlyCountingRelevantRepaintedObject = page->isCountingRelevantRepaintedObjects();
+        page->setIsCountingRelevantRepaintedObjects(false);
+    }
+
     RenderView* renderView = this->renderView();
     if ((renderView && renderView->theme().supportsControlTints()) || hasCustomScrollbars())
         paintControlTints();
+
+    if (page)
+        page->setIsCountingRelevantRepaintedObjects(isCurrentlyCountingRelevantRepaintedObject);
 }
 
 void FrameView::paintControlTints()
index eea7ea7..6e705f4 100644 (file)
@@ -359,6 +359,7 @@ public:
     WEBCORE_EXPORT Color pageExtendedBackgroundColor() const;
 
     bool isCountingRelevantRepaintedObjects() const;
+    void setIsCountingRelevantRepaintedObjects(bool isCounting) { m_isCountingRelevantRepaintedObjects = isCounting; }
     void startCountingRelevantRepaintedObjects();
     void resetRelevantPaintedObjectCounter();
     void addRelevantRepaintedObject(RenderObject*, const LayoutRect& objectPaintRect);
index d31f2ee..7838789 100644 (file)
@@ -246,14 +246,17 @@ void RenderEmbeddedObject::paint(PaintInfo& paintInfo, const LayoutPoint& paintO
 {
     Page* page = frame().page();
 
+    // The relevant repainted object heuristic is not tuned for plugin documents.
+    bool countsTowardsRelevantObjects = page && !document().isPluginDocument() && paintInfo.phase == PaintPhaseForeground;
+
     if (isPluginUnavailable()) {
-        if (page && paintInfo.phase == PaintPhaseForeground)
+        if (countsTowardsRelevantObjects)
             page->addRelevantUnpaintedObject(this, visualOverflowRect());
         RenderReplaced::paint(paintInfo, paintOffset);
         return;
     }
 
-    if (page && paintInfo.phase == PaintPhaseForeground)
+    if (countsTowardsRelevantObjects)
         page->addRelevantRepaintedObject(this, visualOverflowRect());
 
     RenderWidget::paint(paintInfo, paintOffset);