<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[182033] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/182033">182033</a></dd>
<dt>Author</dt> <dd>bdakin@apple.com</dd>
<dt>Date</dt> <dd>2015-03-26 16:06:27 -0700 (Thu, 26 Mar 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorepageFrameViewcpp">trunk/Source/WebCore/page/FrameView.cpp</a></li>
<li><a href="#trunkSourceWebCorepagePageh">trunk/Source/WebCore/page/Page.h</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderEmbeddedObjectcpp">trunk/Source/WebCore/rendering/RenderEmbeddedObject.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (182032 => 182033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-03-26 22:53:46 UTC (rev 182032)
+++ trunk/Source/WebCore/ChangeLog 2015-03-26 23:06:27 UTC (rev 182033)
</span><span class="lines">@@ -1,3 +1,34 @@
</span><ins>+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):
+
</ins><span class="cx"> 2015-03-26 Alex Christensen <achristensen@webkit.org>
</span><span class="cx">
</span><span class="cx"> Progress towards CMake on Mac.
</span></span></pre></div>
<a id="trunkSourceWebCorepageFrameViewcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/FrameView.cpp (182032 => 182033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/FrameView.cpp 2015-03-26 22:53:46 UTC (rev 182032)
+++ trunk/Source/WebCore/page/FrameView.cpp 2015-03-26 23:06:27 UTC (rev 182033)
</span><span class="lines">@@ -3737,9 +3737,20 @@
</span><span class="cx"> if (frame().document()->url().isEmpty())
</span><span class="cx"> return;
</span><span class="cx">
</span><ins>+ // 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);
+ }
+
</ins><span class="cx"> RenderView* renderView = this->renderView();
</span><span class="cx"> if ((renderView && renderView->theme().supportsControlTints()) || hasCustomScrollbars())
</span><span class="cx"> paintControlTints();
</span><ins>+
+ if (page)
+ page->setIsCountingRelevantRepaintedObjects(isCurrentlyCountingRelevantRepaintedObject);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void FrameView::paintControlTints()
</span></span></pre></div>
<a id="trunkSourceWebCorepagePageh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/Page.h (182032 => 182033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/Page.h 2015-03-26 22:53:46 UTC (rev 182032)
+++ trunk/Source/WebCore/page/Page.h 2015-03-26 23:06:27 UTC (rev 182033)
</span><span class="lines">@@ -359,6 +359,7 @@
</span><span class="cx"> WEBCORE_EXPORT Color pageExtendedBackgroundColor() const;
</span><span class="cx">
</span><span class="cx"> bool isCountingRelevantRepaintedObjects() const;
</span><ins>+ void setIsCountingRelevantRepaintedObjects(bool isCounting) { m_isCountingRelevantRepaintedObjects = isCounting; }
</ins><span class="cx"> void startCountingRelevantRepaintedObjects();
</span><span class="cx"> void resetRelevantPaintedObjectCounter();
</span><span class="cx"> void addRelevantRepaintedObject(RenderObject*, const LayoutRect& objectPaintRect);
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderEmbeddedObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderEmbeddedObject.cpp (182032 => 182033)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderEmbeddedObject.cpp 2015-03-26 22:53:46 UTC (rev 182032)
+++ trunk/Source/WebCore/rendering/RenderEmbeddedObject.cpp 2015-03-26 23:06:27 UTC (rev 182033)
</span><span class="lines">@@ -246,14 +246,17 @@
</span><span class="cx"> {
</span><span class="cx"> Page* page = frame().page();
</span><span class="cx">
</span><ins>+ // The relevant repainted object heuristic is not tuned for plugin documents.
+ bool countsTowardsRelevantObjects = page && !document().isPluginDocument() && paintInfo.phase == PaintPhaseForeground;
+
</ins><span class="cx"> if (isPluginUnavailable()) {
</span><del>- if (page && paintInfo.phase == PaintPhaseForeground)
</del><ins>+ if (countsTowardsRelevantObjects)
</ins><span class="cx"> page->addRelevantUnpaintedObject(this, visualOverflowRect());
</span><span class="cx"> RenderReplaced::paint(paintInfo, paintOffset);
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- if (page && paintInfo.phase == PaintPhaseForeground)
</del><ins>+ if (countsTowardsRelevantObjects)
</ins><span class="cx"> page->addRelevantRepaintedObject(this, visualOverflowRect());
</span><span class="cx">
</span><span class="cx"> RenderWidget::paint(paintInfo, paintOffset);
</span></span></pre>
</div>
</div>
</body>
</html>