iframe updates its parent scrollable area set when its visibility change

LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html

Index: LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html
diff --git a/LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html b/LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html
index 9d8e356dd6002554b01438e39b18b9e7382ee1fa..102e9b3fa92b895178cc807c10ed31b78ade3c3f 100644
--- a/LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html
+++ b/LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html
@@ -3,28 +3,52 @@
   body {
     margin: 0;
   }
-  .non-fast {
+  .container {
       height: 250px;
       width: 250px;
+      margin-top: 50px;
       display: block;
       background-color: red;
-      overflow: scroll;
   }
-  .non-fast > div {
+  .container > div {
     height: 1000px;
     width: 1000px;
     display: block;
     background: linear-gradient(to bottom, red, white);
   }
+  .container > iframe {
+    height: 250px;

[Rick Byers, 2015/01/22 15:21:34]

perhaps this should be a slightly different size from 'container' so that you
can differentiate whether a reported rect is reffering to the .container or the
iframe?

[majidvp, 2015/01/22 22:34:56]

Done.

+    width: 250px;
+    display: block;
+    border: none;
+  }
+  section#div > .container {
+    overflow: scroll;
+  }
+  section#iframe > .container {
+    overflow: hidden;
+  }
 </style>
 
-<div id="first" class="non-fast">
-  This should be covered by the ONLY visible green overlay.
-</div>
-<div id="second" class="non-fast" style="margin-top:50px;">
-  This should not be visible.
-  <div></div>
-</div>
+<section id='div' style="display:none;">
+  <div class="to-show container">
+  </div>
+  <div class="to-hide container">
+    <div>This should not be visible.</div>
+  </div>
+</section>
+
+<section id="iframe" style="display:none;">
+  <div class="to-show container">
+  </div>
+  <div class="to-hide container">
+    <iframe src="data:text/html;charset=utf-8,<div style='height:1000; width:1000; background: linear-gradient(to bottom, red, white);'>">

[Rick Byers, 2015/01/22 15:21:34]

this style is invalid and will be ignored - did you mean '1000px'?

[majidvp, 2015/01/22 22:34:56]

Good catch. Chrome interprets it as if it was '1000px' which is why the test was
working.

[Rick Byers, 2015/01/23 21:58:24]

Oh, I've always seen Chrome ignore such rules.  Ah, it's probably because your
iframe has no !DOCTYPE so it's in "quirks mode".

+      This should not be visible.
+    </iframe>
+  </div>
+</section>
+
+<p>Two 250px by 250px squares should be visible and fully covered by green overlays.</p>
 
 <div id="console"></div>
 
@@ -34,63 +58,101 @@
   window.jsTestIsAsync = true;
   description('This test ensures that changing visibility of a non-fast ' +
               'scrollable area correctly updates list of non-fast scrollable ' +
-              'areas. (See http://crbug.com/434982)');
+              'areas (See http://crbug.com/434982). Two types of non-fast ' +
+              'scrollable regions are covered: div, and iframe.');
+
+  onload = next;
+
+  var testCases = [
+    // type, bounding box of element to show, bounding box of element to hide
+    ['div',    '[0, 50, 250, 250]', '[0, 350, 250, 250]'],
+    ['iframe', '[0, 50, 250, 250]', '[0, 350, 250, 250]']];

[Rick Byers, 2015/01/22 15:21:34]

it's perhaps slightly concerning that both test cases produce the exact same
output.  Would make it possible to mask a bug where you end up accidentally
testing the same case twice...  If it adds complexity to make them different,
then it may not be worth it.

[majidvp, 2015/01/22 22:34:56]

Done.

+
+  var index = 0;
+  function next(){
+    if (index < testCases.length) {
+      var type = testCases[index][0];
+      debug('running tests for non-fast scrollable ' + type);
+      runTest.apply(this, testCases[index]);
+    } else {
+      finishJSTest();
+
+      // This has no functional impact just visual
+      showSectionIf(function(){return true;});
+      awaitCompsitingUpdate(runNonFastScrollableRegionTest);
+    }
+
+    index++;
+  }
 
-  onload = function() {
-    awaitCompsitingUpdate(runTest);
-  };
+  // Execute tests for given non-fast scrollable type.
+  function runTest(type, elemToShowExpected, elemToHideExpected) {
+    // Hide all sections that are not relevant to this test to ensure
+    // |window.internals.nonFastScrollableRects| returns only the relevant non-
+    // fast scrollable regions.
+    showSectionIf(function(section){return section.id == type;});
 
-  function runTest() {
-    var first = document.getElementById('first'),
-        second = document.getElementById('second');
+    awaitCompsitingUpdate(function() {
+      shouldBeEqualToString('document.getElementById("' + type + '").style.display', 'block');
 
-    nonFastScrollableRects = internals.nonFastScrollableRects(document);
-    shouldBe('nonFastScrollableRects.length', '1');
-    shouldBeEqualToString('rectToString(nonFastScrollableRects[0])',
-                          '[0, 300, 250, 250]'); // second div
+      var elemToShow = document.querySelector('section#' + type + '> .to-show'),
+          elemToHide = document.querySelector('section#' + type + '> .to-hide');
 
-    // Hide container before making it non-fast scrollable to ensure any
-    // layout occurs while it is invisible.
-    first.style.visibility = 'hidden';
-    makeNonFastScrollable(first);
+      nonFastScrollableRects = internals.nonFastScrollableRects(document);
+      shouldBe('nonFastScrollableRects.length', '1');
+      shouldBeEqualToString('rectToString(nonFastScrollableRects[0])', elemToHideExpected);
 
-    // Change visibility (hidden -> visible), which should not cause any layout,
-    // and verify that non-fast scrollable areas are correctly updated.
-    awaitCompsitingUpdate(function() {
-      first.style.visibility = 'visible';
-      shouldBe('window.internals.needsLayoutCount()', '0');
+      // Hide container before making it non-fast scrollable to ensure any
+      // layout occurs while it is invisible.
+      elemToShow.style.visibility = 'hidden';
+      makeNonFastScrollable(elemToShow, type);
 
+      // Change visibility (hidden -> visible), which should not cause any layout,
+      // and verify that non-fast scrollable areas are correctly updated.
       awaitCompsitingUpdate(function() {
-        nonFastScrollableRects = internals.nonFastScrollableRects(document);
-        shouldBe('nonFastScrollableRects.length', '2');
-        shouldBeEqualToString('rectToString(nonFastScrollableRects[0])',
-                              '[0, 0, 250, 250]'); // first div
-        shouldBeEqualToString('rectToString(nonFastScrollableRects[1])',
-                              '[0, 300, 250, 250]'); // second div
-
-        // Change visibility (visible -> hidden)
-        second.style.visibility = 'hidden';
+        elemToShow.style.visibility = 'visible';
         shouldBe('window.internals.needsLayoutCount()', '0');
 
         awaitCompsitingUpdate(function() {
-          // This has no functional impact just visual
-          runNonFastScrollableRegionTest();
-
           nonFastScrollableRects = internals.nonFastScrollableRects(document);
-          shouldBe('nonFastScrollableRects.length', '1');
-          shouldBeEqualToString('rectToString(nonFastScrollableRects[0])',
-                                '[0, 0, 250, 250]'); // first div
-
-          finishJSTest();
+          shouldBe('nonFastScrollableRects.length', '2');
+          shouldBeEqualToString('rectToString(nonFastScrollableRects[0])', elemToShowExpected);
+          shouldBeEqualToString('rectToString(nonFastScrollableRects[1])', elemToHideExpected);
+
+          // Change visibility (visible -> hidden)
+          elemToHide.style.visibility = 'hidden';
+          shouldBe('window.internals.needsLayoutCount()', '0');
+
+          awaitCompsitingUpdate(function() {
+            nonFastScrollableRects = internals.nonFastScrollableRects(document);
+            shouldBe('nonFastScrollableRects.length', '1');
+            shouldBeEqualToString('rectToString(nonFastScrollableRects[0])', elemToShowExpected);
+            next();
+          });
         });
       });
     });
   }
 
   // Makes the container non-fast scrollable area by appending a large element to it.

[Rick Byers, 2015/01/22 15:21:34]

nit: this makes the container "scrollable", right?  I assume it's only non-fast
scrollable because tests are run in a low-dpi context, right?  Perhaps worth
clarifying that in the function name and comments (since we don't explicitly
force it to be true with an internals API).

[majidvp, 2015/01/22 22:34:56]

Changed the function name to be more appropriate and added a comment explaining
that when the layout test is executed with --dump-render-tree the scrollables
are non-fast.

-  function makeNonFastScrollable(container) {
-    var inner = document.createElement('div');
-    container.appendChild(inner);
+  function makeNonFastScrollable(container, type) {
+    var inner;
+    switch (type) {
+    case 'div':
+      inner = '<div>This should be covered by a green overlay.</div>';
+      break;
+    case 'iframe':
+       inner = '<iframe id="iframe" src="data:text/html;charset=utf-8,<div style=\'height:1000; width:1000; background: linear-gradient(to bottom, green, white);\'>This should be covered by a green overlay.</div>"></iframe>';

[Rick Byers, 2015/01/22 15:21:34]

Again your style is busted.   So how is this working?  Perhaps the iframe is
always scrollable despite the size of it's content?

[majidvp, 2015/01/22 22:34:56]

Fixed. Chrome interprets 1000 as 1000px.

+      break;
+    }
+    container.innerHTML = inner;
+  }
+
+  // Shows sections that pass condition but hides others
+  function showSectionIf(condition) {
+    var sections = document.querySelectorAll('section');
+    for (var i = 0; i < sections.length; i++)
+      sections[i].style.display = condition(sections[i]) ? 'block' : 'none';
   }
 
   // Waits for one RAF call to ensure compositing update has occurred and invokes task.