<!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>[170638] 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/170638">170638</a></dd>
<dt>Author</dt> <dd>antti@apple.com</dd>
<dt>Date</dt> <dd>2014-07-01 08:16:24 -0700 (Tue, 01 Jul 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>REGRESSION(160908): vube.com video won't play after going into and out of fullscreen
https://bugs.webkit.org/show_bug.cgi?id=134489
Reviewed by Zalan Bujtas.
Test: fullscreen/full-screen-plugin.html
It is difficult to restore the render tree correctly in all cases after removing a full screen
renderer from the tree. <a href="http://trac.webkit.org/projects/webkit/changeset/160908">r160908</a> avoided dealing with this by simply always reconstructing the subtree.
Unfortunately plugin lifetime is currently tied to its renderer so this would cause the plugin to restart.
With this patch we avoid reconstruction in normal cases and only force it if the render tree is complicated.
* dom/Document.cpp:
(WebCore::unwrapFullScreenRenderer):
Force reconstruction conditionally.
(WebCore::Document::webkitWillEnterFullScreenForElement):
(WebCore::Document::webkitDidExitFullScreenForElement):
* rendering/RenderFullScreen.cpp:
(WebCore::RenderFullScreen::wrapRenderer):
(WebCore::RenderFullScreen::unwrapRenderer):
Deal with the simple case of single child, possibly in anonymous wrapper.
In other cases request reconstruction.
This is covered by the existing fullscreen tests.
* rendering/RenderFullScreen.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomDocumentcpp">trunk/Source/WebCore/dom/Document.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderFullScreencpp">trunk/Source/WebCore/rendering/RenderFullScreen.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderFullScreenh">trunk/Source/WebCore/rendering/RenderFullScreen.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (170637 => 170638)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-07-01 13:10:18 UTC (rev 170637)
+++ trunk/Source/WebCore/ChangeLog 2014-07-01 15:16:24 UTC (rev 170638)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2014-07-01 Antti Koivisto <antti@apple.com>
+
+ REGRESSION(160908): vube.com video won't play after going into and out of fullscreen
+ https://bugs.webkit.org/show_bug.cgi?id=134489
+
+ Reviewed by Zalan Bujtas.
+
+ Test: fullscreen/full-screen-plugin.html
+
+ It is difficult to restore the render tree correctly in all cases after removing a full screen
+ renderer from the tree. r160908 avoided dealing with this by simply always reconstructing the subtree.
+ Unfortunately plugin lifetime is currently tied to its renderer so this would cause the plugin to restart.
+
+ With this patch we avoid reconstruction in normal cases and only force it if the render tree is complicated.
+
+ * dom/Document.cpp:
+ (WebCore::unwrapFullScreenRenderer):
+
+ Force reconstruction conditionally.
+
+ (WebCore::Document::webkitWillEnterFullScreenForElement):
+ (WebCore::Document::webkitDidExitFullScreenForElement):
+ * rendering/RenderFullScreen.cpp:
+ (WebCore::RenderFullScreen::wrapRenderer):
+ (WebCore::RenderFullScreen::unwrapRenderer):
+
+ Deal with the simple case of single child, possibly in anonymous wrapper.
+ In other cases request reconstruction.
+ This is covered by the existing fullscreen tests.
+
+ * rendering/RenderFullScreen.h:
+
</ins><span class="cx"> 2014-07-01 Zan Dobersek <zdobersek@igalia.com>
</span><span class="cx">
</span><span class="cx"> Remove remaining Vector<> copies in WebCore accessibility code
</span></span></pre></div>
<a id="trunkSourceWebCoredomDocumentcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Document.cpp (170637 => 170638)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/Document.cpp 2014-07-01 13:10:18 UTC (rev 170637)
+++ trunk/Source/WebCore/dom/Document.cpp 2014-07-01 15:16:24 UTC (rev 170638)
</span><span class="lines">@@ -5311,6 +5311,17 @@
</span><span class="cx"> return isAttributeOnAllOwners(allowfullscreenAttr, webkitallowfullscreenAttr, ownerElement());
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+static void unwrapFullScreenRenderer(RenderFullScreen* fullScreenRenderer, Element* fullScreenElement)
+{
+ if (!fullScreenRenderer)
+ return;
+ bool requiresRenderTreeRebuild;
+ fullScreenRenderer->unwrapRenderer(requiresRenderTreeRebuild);
+
+ if (requiresRenderTreeRebuild && fullScreenElement && fullScreenElement->parentNode())
+ fullScreenElement->parentNode()->setNeedsStyleRecalc(ReconstructRenderTree);
+}
+
</ins><span class="cx"> void Document::webkitWillEnterFullScreenForElement(Element* element)
</span><span class="cx"> {
</span><span class="cx"> if (!hasLivingRenderTree() || inPageCache())
</span><span class="lines">@@ -5324,8 +5335,7 @@
</span><span class="cx">
</span><span class="cx"> ASSERT(page()->settings().fullScreenEnabled());
</span><span class="cx">
</span><del>- if (m_fullScreenRenderer)
- m_fullScreenRenderer->unwrapRenderer();
</del><ins>+ unwrapFullScreenRenderer(m_fullScreenRenderer, m_fullScreenElement.get());
</ins><span class="cx">
</span><span class="cx"> m_fullScreenElement = element;
</span><span class="cx">
</span><span class="lines">@@ -5388,16 +5398,12 @@
</span><span class="cx"> m_fullScreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
</span><span class="cx">
</span><span class="cx"> m_areKeysEnabledInFullScreen = false;
</span><del>-
- if (m_fullScreenRenderer)
- m_fullScreenRenderer->unwrapRenderer();
</del><span class="cx">
</span><del>- if (m_fullScreenElement->parentNode())
- m_fullScreenElement->parentNode()->setNeedsStyleRecalc(ReconstructRenderTree);
</del><ins>+ unwrapFullScreenRenderer(m_fullScreenRenderer, m_fullScreenElement.get());
</ins><span class="cx">
</span><span class="cx"> m_fullScreenElement = nullptr;
</span><span class="cx"> scheduleForcedStyleRecalc();
</span><del>-
</del><ins>+
</ins><span class="cx"> // When webkitCancelFullScreen is called, we call webkitExitFullScreen on the topDocument(). That
</span><span class="cx"> // means that the events will be queued there. So if we have no events here, start the timer on
</span><span class="cx"> // the exiting document.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderFullScreencpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderFullScreen.cpp (170637 => 170638)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderFullScreen.cpp 2014-07-01 13:10:18 UTC (rev 170637)
+++ trunk/Source/WebCore/rendering/RenderFullScreen.cpp 2014-07-01 15:16:24 UTC (rev 170638)
</span><span class="lines">@@ -138,11 +138,32 @@
</span><span class="cx"> return fullscreenRenderer;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void RenderFullScreen::unwrapRenderer()
</del><ins>+void RenderFullScreen::unwrapRenderer(bool& requiresRenderTreeRebuild)
</ins><span class="cx"> {
</span><ins>+ requiresRenderTreeRebuild = false;
</ins><span class="cx"> if (parent()) {
</span><del>- RenderObject* child;
</del><ins>+ auto* child = firstChild();
+ // Things can get very complicated with anonymous block generation.
+ // We can restore correctly without rebuild in simple cases only.
+ // FIXME: We should have a mechanism for removing a block without reconstructing the tree.
+ if (child != lastChild())
+ requiresRenderTreeRebuild = true;
+ else if (child && child->isAnonymousBlock()) {
+ auto& anonymousBlock = toRenderBlock(*child);
+ if (anonymousBlock.firstChild() != anonymousBlock.lastChild())
+ requiresRenderTreeRebuild = true;
+ }
+
</ins><span class="cx"> while ((child = firstChild())) {
</span><ins>+ if (child->isAnonymousBlock() && !requiresRenderTreeRebuild) {
+ if (auto* nonAnonymousChild = toRenderBlock(*child).firstChild())
+ child = nonAnonymousChild;
+ else {
+ child->removeFromParent();
+ child->destroy();
+ continue;
+ }
+ }
</ins><span class="cx"> // We have to clear the override size, because as a flexbox, we
</span><span class="cx"> // may have set one on the child, and we don't want to leave that
</span><span class="cx"> // lying around on the child.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderFullScreenh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderFullScreen.h (170637 => 170638)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderFullScreen.h 2014-07-01 13:10:18 UTC (rev 170637)
+++ trunk/Source/WebCore/rendering/RenderFullScreen.h 2014-07-01 15:16:24 UTC (rev 170638)
</span><span class="lines">@@ -44,7 +44,7 @@
</span><span class="cx"> void createPlaceholder(PassRef<RenderStyle>, const LayoutRect& frameRect);
</span><span class="cx">
</span><span class="cx"> static RenderFullScreen* wrapRenderer(RenderObject*, RenderElement*, Document&);
</span><del>- void unwrapRenderer();
</del><ins>+ void unwrapRenderer(bool& requiresRenderTreeRebuild);
</ins><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> virtual void willBeDestroyed() override;
</span></span></pre>
</div>
</div>
</body>
</html>