<!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>[209375] trunk</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/209375">209375</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-12-05 18:37:43 -0800 (Mon, 05 Dec 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>ERROR: post-layout: dirty renderer(s) - Encountered with LayoutTest media/modern-media-controls/media-controller/media-controller-fullscreen-ltr.html
https://bugs.webkit.org/show_bug.cgi?id=165312

Patch by Antoine Quint &lt;graouts@apple.com&gt; on 2016-12-05
Reviewed by Simon Fraser.

Source/WebCore:

Reverting part of the code added in https://bugs.webkit.org/show_bug.cgi?id=165287 that triggered
an assertion. We go back to removing previous media controls as we add new ones when the fullscreen
status changes, and simply hide the controls during the animated transition using a CSS pseudo-class.
This also fixes an issue where we wouldn't have removed the previous controls should we have entered
fullscreen in a different way than clicking on the fullscreen button in the media controls.

We restore testing coverage that was fixed due to this assertion.

* Modules/modern-media-controls/controls/media-controls.css:
(:host(:-webkit-animating-full-screen-transition) .media-controls):
* Modules/modern-media-controls/controls/media-controls.js:
(MediaControls.prototype.fadeIn):
(MediaControls.prototype.presentInElement): Deleted.
* Modules/modern-media-controls/media/fullscreen-support.js:
(FullscreenSupport.prototype.buttonWasClicked):
* Modules/modern-media-controls/media/media-controller.js:
(MediaController.prototype._updateControlsIfNeeded):

LayoutTests:

Restore previously skipped tests.

* platform/mac/TestExpectations:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsplatformmacTestExpectations">trunk/LayoutTests/platform/mac/TestExpectations</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreModulesmodernmediacontrolscontrolsmediacontrolscss">trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.css</a></li>
<li><a href="#trunkSourceWebCoreModulesmodernmediacontrolscontrolsmediacontrolsjs">trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js</a></li>
<li><a href="#trunkSourceWebCoreModulesmodernmediacontrolsmediafullscreensupportjs">trunk/Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js</a></li>
<li><a href="#trunkSourceWebCoreModulesmodernmediacontrolsmediamediacontrollerjs">trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (209374 => 209375)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2016-12-06 02:30:38 UTC (rev 209374)
+++ trunk/LayoutTests/ChangeLog        2016-12-06 02:37:43 UTC (rev 209375)
</span><span class="lines">@@ -1,3 +1,14 @@
</span><ins>+2016-12-05  Antoine Quint  &lt;graouts@apple.com&gt;
+
+        ERROR: post-layout: dirty renderer(s) - Encountered with LayoutTest media/modern-media-controls/media-controller/media-controller-fullscreen-ltr.html
+        https://bugs.webkit.org/show_bug.cgi?id=165312
+
+        Reviewed by Simon Fraser.
+
+        Restore previously skipped tests.
+
+        * platform/mac/TestExpectations:
+
</ins><span class="cx"> 2016-12-05  Dave Hyatt  &lt;hyatt@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [CSS Parser] Tweak more layout tests to pass
</span></span></pre></div>
<a id="trunkLayoutTestsplatformmacTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/mac/TestExpectations (209374 => 209375)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/platform/mac/TestExpectations        2016-12-06 02:30:38 UTC (rev 209374)
+++ trunk/LayoutTests/platform/mac/TestExpectations        2016-12-06 02:37:43 UTC (rev 209375)
</span><span class="lines">@@ -1472,10 +1472,6 @@
</span><span class="cx"> webkit.org/b/165290 media/modern-media-controls/tracks-panel/tracks-panel-hide-click-outside.html [ Pass Timeout ]
</span><span class="cx"> webkit.org/b/164598 media/modern-media-controls/ios-inline-media-controls/ios-inline-media-controls-buttons-styles.html [ Pass Failure ]
</span><span class="cx"> 
</span><del>-webkit.org/b/165312 media/modern-media-controls/media-controller/media-controller-fullscreen-ltr.html [ Skip ]
-webkit.org/b/165312 media/modern-media-controls/seek-backward-support/seek-backward-support.html [ Skip ]
-webkit.org/b/165312 media/modern-media-controls/seek-forward-support/seek-forward-support.html [ Skip ]
-
</del><span class="cx"> webkit.org/b/164277 fast/preloader/image-srcset.html [ Pass Failure ]
</span><span class="cx"> 
</span><span class="cx"> webkit.org/b/161650 http/tests/cache/disk-cache/disk-cache-remove-several-pending-writes.html [ Pass Failure ]
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (209374 => 209375)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2016-12-06 02:30:38 UTC (rev 209374)
+++ trunk/Source/WebCore/ChangeLog        2016-12-06 02:37:43 UTC (rev 209375)
</span><span class="lines">@@ -1,3 +1,28 @@
</span><ins>+2016-12-05  Antoine Quint  &lt;graouts@apple.com&gt;
+
+        ERROR: post-layout: dirty renderer(s) - Encountered with LayoutTest media/modern-media-controls/media-controller/media-controller-fullscreen-ltr.html
+        https://bugs.webkit.org/show_bug.cgi?id=165312
+
+        Reviewed by Simon Fraser.
+
+        Reverting part of the code added in https://bugs.webkit.org/show_bug.cgi?id=165287 that triggered
+        an assertion. We go back to removing previous media controls as we add new ones when the fullscreen
+        status changes, and simply hide the controls during the animated transition using a CSS pseudo-class.
+        This also fixes an issue where we wouldn't have removed the previous controls should we have entered
+        fullscreen in a different way than clicking on the fullscreen button in the media controls.
+
+        We restore testing coverage that was fixed due to this assertion.
+
+        * Modules/modern-media-controls/controls/media-controls.css:
+        (:host(:-webkit-animating-full-screen-transition) .media-controls):
+        * Modules/modern-media-controls/controls/media-controls.js:
+        (MediaControls.prototype.fadeIn):
+        (MediaControls.prototype.presentInElement): Deleted.
+        * Modules/modern-media-controls/media/fullscreen-support.js:
+        (FullscreenSupport.prototype.buttonWasClicked):
+        * Modules/modern-media-controls/media/media-controller.js:
+        (MediaController.prototype._updateControlsIfNeeded):
+
</ins><span class="cx"> 2016-12-05  Dean Jackson  &lt;dino@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         MediaDocuments crash with modern media controls
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesmodernmediacontrolscontrolsmediacontrolscss"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.css (209374 => 209375)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.css        2016-12-06 02:30:38 UTC (rev 209374)
+++ trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.css        2016-12-06 02:37:43 UTC (rev 209375)
</span><span class="lines">@@ -36,6 +36,10 @@
</span><span class="cx">     cursor: default;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+:host(:-webkit-animating-full-screen-transition) .media-controls {
+    display: none;
+}
+
</ins><span class="cx"> .media-controls &gt; .controls-bar {
</span><span class="cx">     position: absolute;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesmodernmediacontrolscontrolsmediacontrolsjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js (209374 => 209375)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js        2016-12-06 02:30:38 UTC (rev 209374)
+++ trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js        2016-12-06 02:37:43 UTC (rev 209375)
</span><span class="lines">@@ -101,11 +101,9 @@
</span><span class="cx">         this._invalidateChildren();
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    presentInElement(parentElement, animated)
</del><ins>+    fadeIn()
</ins><span class="cx">     {
</span><del>-        if (animated)
-            this.element.classList.add(&quot;fade-in&quot;);
-        parentElement.appendChild(this.element);
</del><ins>+        this.element.classList.add(&quot;fade-in&quot;);
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     // Private
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesmodernmediacontrolsmediafullscreensupportjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js (209374 => 209375)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js        2016-12-06 02:30:38 UTC (rev 209374)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js        2016-12-06 02:37:43 UTC (rev 209375)
</span><span class="lines">@@ -60,8 +60,6 @@
</span><span class="cx"> 
</span><span class="cx">     buttonWasClicked(control)
</span><span class="cx">     {
</span><del>-        this.mediaController.controls.element.remove();
-
</del><span class="cx">         const media = this.mediaController.media;
</span><span class="cx">         if (media.webkitDisplayingFullscreen)
</span><span class="cx">             media.webkitExitFullscreen();
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesmodernmediacontrolsmediamediacontrollerjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller.js (209374 => 209375)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller.js        2016-12-06 02:30:38 UTC (rev 209374)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller.js        2016-12-06 02:37:43 UTC (rev 209375)
</span><span class="lines">@@ -94,11 +94,13 @@
</span><span class="cx"> 
</span><span class="cx">         this.controls = new ControlsClass;
</span><span class="cx"> 
</span><del>-        if (previousControls)
</del><ins>+        if (previousControls) {
+            this.controls.fadeIn();
+            this.shadowRoot.replaceChild(this.controls.element, previousControls.element);
</ins><span class="cx">             this.controls.usesLTRUserInterfaceLayoutDirection = previousControls.usesLTRUserInterfaceLayoutDirection;
</span><ins>+        } else
+            this.shadowRoot.appendChild(this.controls.element);        
</ins><span class="cx"> 
</span><del>-        this.controls.presentInElement(this.shadowRoot, !!previousControls);
-
</del><span class="cx">         this._updateControlsSize();
</span><span class="cx"> 
</span><span class="cx">         this._supportingObjects = [AirplaySupport, ControlsVisibilitySupport, ElapsedTimeSupport, FullscreenSupport, MuteSupport, PiPSupport, PlacardSupport, PlaybackSupport, RemainingTimeSupport, ScrubbingSupport, SeekBackwardSupport, SeekForwardSupport, SkipBackSupport, StartSupport, StatusSupport, TracksSupport, VolumeSupport].map(SupportClass =&gt; {
</span></span></pre>
</div>
</div>

</body>
</html>