iframe updates its parent scrollable area set when its visibility change

iframe updates its parent scrollable area set when its visibility change.

This ensures that ScrollingCoordinator gets notified and non-fast scrollable regions are updated accordingly.

BUG=434982
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188913

[majidvp, 2015-01-21 18:45:17]

majidvp@chromium.org changed reviewers:
+ rbyers@chromium.org, skobes@chromium.org

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

Great!  The test is nice too, just a couple little issues.

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
File LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html
(right):

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html:20:
height: 250px;
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?

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html:45:
<iframe src="data:text/html;charset=utf-8,<div style='height:1000; width:1000;
background: linear-gradient(to bottom, red, white);'>">
this style is invalid and will be ignored - did you mean '1000px'?

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html:69:
['iframe', '[0, 50, 250, 250]', '[0, 350, 250, 250]']];
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.

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html:137:
// Makes the container non-fast scrollable area by appending a large element to
it.
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).

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html:145:
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>';
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]

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
File LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html
(right):

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html:20:
height: 250px;
On 2015/01/22 15:21:34, Rick Byers wrote:
> 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?

Done.

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html:45:
<iframe src="data:text/html;charset=utf-8,<div style='height:1000; width:1000;
background: linear-gradient(to bottom, red, white);'>">
On 2015/01/22 15:21:34, Rick Byers wrote:
> this style is invalid and will be ignored - did you mean '1000px'?

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

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html:69:
['iframe', '[0, 50, 250, 250]', '[0, 350, 250, 250]']];
On 2015/01/22 15:21:34, Rick Byers wrote:
> 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.

Done.

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html:137:
// Makes the container non-fast scrollable area by appending a large element to
it.
On 2015/01/22 15:21:34, Rick Byers wrote:
> 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).

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.

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html:145:
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>';
On 2015/01/22 15:21:34, Rick Byers wrote:
> Again your style is busted.   So how is this working?  Perhaps the iframe is
> always scrollable despite the size of it's content?

Fixed. Chrome interprets 1000 as 1000px.

[skobes, 2015-01-22 22:36:41]

lgtm

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

LGTM, thanks!

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
File LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html
(right):

https://codereview.chromium.org/850113003/diff/20001/LayoutTests/scrollingcoo...
LayoutTests/scrollingcoordinator/non-fast-scrollable-visibility-change.html:45:
<iframe src="data:text/html;charset=utf-8,<div style='height:1000; width:1000;
background: linear-gradient(to bottom, red, white);'>">
On 2015/01/22 22:34:56, majidvp wrote:
> On 2015/01/22 15:21:34, Rick Byers wrote:
> > this style is invalid and will be ignored - did you mean '1000px'?
> 
> Good catch. Chrome interprets it as if it was '1000px' which is why the test
was
> working.

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".

[majidvp, 2015-01-23 23:11:10]

The CQ bit was checked by majidvp@chromium.org

[commit-bot: I haz the power, 2015-01-23 23:12:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/850113003/80001

[commit-bot: I haz the power, 2015-01-23 23:15:28]

Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=188913