Sync "fully exit fullscreen" with the spec

Sync "fully exit fullscreen" with the spec

http://fullscreen.spec.whatwg.org/#fully-exit-fullscreen
https://www.w3.org/Bugs/Public/show_bug.cgi?id=26587

This should not change observable behavior.

When to actually invoke "fully exit fullscreen" is not finalized:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=26584

TEST=LayoutTests/fullscreen/full-screen-exit-when-popup.html (old)
TEST=LayoutTests/fullscreen/model/fully-exit-fullscreen-*.html (new)

The old test enters the fullyExitFullscreen() code path, but passes even
if fullyExitFullscreen() is made an alias of exitFullscreen().

The new tests will fail if a non-top-level document is used or if more
than one element is on the stack when exitFullscreen() is called.

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

[philipj_slow, 2014-08-13 21:48:32]

I haven't written the tests for this yet, but the existing ones pass. I'll write
the tests tomorrow, but to reduce roundtrips maybe you can look if you can think
of any particular case worth testing. Previously we removed all but the top
element, and now all but the bottom element is being popped.

[philipj_slow, 2014-08-14 14:00:05]

add test

[philipj_slow, 2014-08-14 22:15:17]

No need to review this quite yet, I found a spec bug that needs to be resolved
first:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=26587

[philipj_slow, 2014-08-14 22:30:36]

speculative changes

[philipj_slow, 2014-08-15 13:11:09]

more tests

[philipj_slow, 2014-08-15 13:36:19]

I'm happy with this now, PTAL.

(The tests continue to be a bit ugly, but I'll stick to ugly for a bit longer
thinking about what a good framework would be. Something like: listen to all
fullscreenchange and fullscreenerror events on all documents, verifying an
expected order, and allow functions to be registered for specific steps to drive
multi-step tests.)

[falken, 2014-08-18 02:46:46]

I see your CL description, but I'm a bit wary about adding the internals
function. Can we test by triggering the fully exit fullscreen behavior without
use of internals? 

The trigger might be per-spec but at least the tests will enable us to catch
regressions/progressions when our code changes. In contrast, it's possible some
refactoring makes fully exit fullscreen never be called in production yet these
layout tests will continue to pass.

WDYT?

If we really want the internals route, it would be good to comment with a FIXME
why we're using it.

[philipj_slow, 2014-08-18 08:26:29]

On 2014/08/18 02:46:46, falken wrote:
> I see your CL description, but I'm a bit wary about adding the internals
> function. Can we test by triggering the fully exit fullscreen behavior without
> use of internals? 
> 
> The trigger might be per-spec but at least the tests will enable us to catch
> regressions/progressions when our code changes. In contrast, it's possible
some
> refactoring makes fully exit fullscreen never be called in production yet
these
> layout tests will continue to pass.
> 
> WDYT?
> 
> If we really want the internals route, it would be good to comment with a
FIXME
> why we're using it.

The only way of triggering "fully exit fullscreen" that I could find was using
window.open() as in the existing test, but that isn't one of the triggers in the
spec, while none of the triggers in the spec seem to work, which is why I filed
that spec bug.

How about a fully-exit-fullscreen.js that uses window.open(), with a FIXME
pointing at the spec bug?

[falken, 2014-08-18 08:32:07]

On 2014/08/18 08:26:29, philipj wrote:
> On 2014/08/18 02:46:46, falken wrote:
> > I see your CL description, but I'm a bit wary about adding the internals
> > function. Can we test by triggering the fully exit fullscreen behavior
without
> > use of internals? 
> > 
> > The trigger might be per-spec but at least the tests will enable us to catch
> > regressions/progressions when our code changes. In contrast, it's possible
> some
> > refactoring makes fully exit fullscreen never be called in production yet
> these
> > layout tests will continue to pass.
> > 
> > WDYT?
> > 
> > If we really want the internals route, it would be good to comment with a
> FIXME
> > why we're using it.
> 
> The only way of triggering "fully exit fullscreen" that I could find was using
> window.open() as in the existing test, but that isn't one of the triggers in
the
> spec, while none of the triggers in the spec seem to work, which is why I
filed
> that spec bug.
> 
> How about a fully-exit-fullscreen.js that uses window.open(), with a FIXME
> pointing at the spec bug?

That sounds good to me.

[philipj_slow, 2014-08-18 11:59:52]

test with window.open()

[philipj_slow, 2014-08-18 12:38:51]

OK, PTAL and CQ if no nits :)

[falken, 2014-08-19 01:11:54]

lgtm, thanks!

[falken, 2014-08-19 01:11:58]

The CQ bit was checked by falken@chromium.org

[commit-bot: I haz the power, 2014-08-19 01:12:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/philipj@opera.com/470723002/80001

[commit-bot: I haz the power, 2014-08-19 01:18:10]

Committed patchset #5 (80001) as 180506