Improve Fullscreen unit tests

Improve Fullscreen unit tests

Test for difference between fullscreenElementFrom and
currentFullScreenElementFrom, weird as it is. The latter should go away,
but that is blocked on refactoring which will affect the timing. The
tests will help catch regressions.

There were previously no (unit) tests for the overlay fullscreen video
mode currently handled in FullscreenController::didEnterFullscreen and
::didExitFullscreen. This will also have to be refactored, so add a test
that failes if either of the setHasTransparentBackground calls are
commented out.

BUG=402376, 402421
Committed: https://crrev.com/40d1eb7898a35f3eb9d6b6abae7accc4a8a70d10
Cr-Commit-Position: refs/heads/master@{#435142}

[foolip, 2016-11-24 17:03:20]

Description was changed from

==========
Update Fullscreen unit tests

BUG=402376
==========

to

==========
Improve Fullscreen unit tests

Test for difference between fullscreenElementFrom and
currentFullScreenElementFrom, weird as it is. The latter should go away,
but that is blocked on refactoring which will affect the timing. The
tests will help catch regressions.

There were previously no (unit) tests for the overlay fullscreen video
mode currently handled in FullscreenController::didEnterFullscreen and
::didExitFullscreen. This will also have to be refactored, so add a test
that failes if either of the setHasTransparentBackground calls are
commented out.

BUG=402376,402421
==========

[foolip, 2016-11-24 17:03:26]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-24 17:03:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[foolip, 2016-11-24 17:07:00]

foolip@chromium.org changed reviewers:
+ eae@chromium.org, esprehn@chromium.org

[foolip, 2016-11-24 17:07:01]

eae@, esprehn@, can either of you help review or delegate if this isn't your
thing? This is building towards having FullscreenController know less about
Fullscreen internals in  https://codereview.chromium.org/2495423004/ which is
needed to change the timing of state change + event firing, which via a few
other things is blocking top layer and unprefixing.

Will comment on a specific thing I assume is naughty.

[foolip, 2016-11-24 17:08:53]

https://codereview.chromium.org/2527093003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/2527093003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:7991:
static_cast<WebLayerTreeViewImplForTesting*>(
I can only assume this isn't cool? Is it acceptable, or should this kind of
thing be done by subclassing WebLayerTreeViewImplForTesting? Or something else
entirely?

[commit-bot: I haz the power, 2016-11-24 21:17:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-24 21:17:13]

Dry run: This issue passed the CQ dry run.

[eae, 2016-11-24 21:45:31]

https://codereview.chromium.org/2527093003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/2527093003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:7991:
static_cast<WebLayerTreeViewImplForTesting*>(
On 2016/11/24 17:08:53, foolip wrote:
> I can only assume this isn't cool? Is it acceptable, or should this kind of
> thing be done by subclassing WebLayerTreeViewImplForTesting? Or something else
> entirely?

It is a little gross but not without precedence. Elliot, what's your take on
this?

[foolip, 2016-11-28 14:22:21]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-28 14:22:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[foolip, 2016-11-28 14:23:38]

https://codereview.chromium.org/2527093003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/2527093003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:7991:
static_cast<WebLayerTreeViewImplForTesting*>(
On 2016/11/24 21:45:31, eae wrote:
> On 2016/11/24 17:08:53, foolip wrote:
> > I can only assume this isn't cool? Is it acceptable, or should this kind of
> > thing be done by subclassing WebLayerTreeViewImplForTesting? Or something
else
> > entirely?
> 
> It is a little gross but not without precedence. Elliot, what's your take on
> this?

A new week and it wasn't nearly as messy as I thought to add a special
TestFullscreenWebViewClient for this, so PTAL :)

[commit-bot: I haz the power, 2016-11-28 16:36:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-28 16:36:43]

Dry run: This issue passed the CQ dry run.

[foolip, 2016-11-29 13:07:32]

Friendly ping :)

[chrishtr, 2016-11-29 23:41:58]

chrishtr@chromium.org changed reviewers:
+ chrishtr@chromium.org

[chrishtr, 2016-11-29 23:41:58]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-11-29 23:41:58]

lgtm

[commit-bot: I haz the power, 2016-11-29 23:43:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 01:46:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 01:46:33]

Try jobs failed on following builders:
  blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[foolip, 2016-11-30 07:13:20]

The CQ bit was checked by foolip@chromium.org

[commit-bot: I haz the power, 2016-11-30 07:14:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 07:52:29]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480490000041070,
"parent_rev": "a79166c884f637e32cb267f6878941137893389d", "commit_rev":
"370504175ab320dd7b79ce3bf4c263b03644798e"}

[commit-bot: I haz the power, 2016-11-30 07:52:53]

Description was changed from

==========
Improve Fullscreen unit tests

Test for difference between fullscreenElementFrom and
currentFullScreenElementFrom, weird as it is. The latter should go away,
but that is blocked on refactoring which will affect the timing. The
tests will help catch regressions.

There were previously no (unit) tests for the overlay fullscreen video
mode currently handled in FullscreenController::didEnterFullscreen and
::didExitFullscreen. This will also have to be refactored, so add a test
that failes if either of the setHasTransparentBackground calls are
commented out.

BUG=402376,402421
==========

to

==========
Improve Fullscreen unit tests

Test for difference between fullscreenElementFrom and
currentFullScreenElementFrom, weird as it is. The latter should go away,
but that is blocked on refactoring which will affect the timing. The
tests will help catch regressions.

There were previously no (unit) tests for the overlay fullscreen video
mode currently handled in FullscreenController::didEnterFullscreen and
::didExitFullscreen. This will also have to be refactored, so add a test
that failes if either of the setHasTransparentBackground calls are
commented out.

BUG=402376,402421
==========

[commit-bot: I haz the power, 2016-11-30 07:52:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-30 07:56:19]

Description was changed from

==========
Improve Fullscreen unit tests

Test for difference between fullscreenElementFrom and
currentFullScreenElementFrom, weird as it is. The latter should go away,
but that is blocked on refactoring which will affect the timing. The
tests will help catch regressions.

There were previously no (unit) tests for the overlay fullscreen video
mode currently handled in FullscreenController::didEnterFullscreen and
::didExitFullscreen. This will also have to be refactored, so add a test
that failes if either of the setHasTransparentBackground calls are
commented out.

BUG=402376,402421
==========

to

==========
Improve Fullscreen unit tests

Test for difference between fullscreenElementFrom and
currentFullScreenElementFrom, weird as it is. The latter should go away,
but that is blocked on refactoring which will affect the timing. The
tests will help catch regressions.

There were previously no (unit) tests for the overlay fullscreen video
mode currently handled in FullscreenController::didEnterFullscreen and
::didExitFullscreen. This will also have to be refactored, so add a test
that failes if either of the setHasTransparentBackground calls are
commented out.

BUG=402376,402421

Committed: https://crrev.com/40d1eb7898a35f3eb9d6b6abae7accc4a8a70d10
Cr-Commit-Position: refs/heads/master@{#435142}
==========

[commit-bot: I haz the power, 2016-11-30 07:56:20]

Patchset 2 (id:??) landed as
https://crrev.com/40d1eb7898a35f3eb9d6b6abae7accc4a8a70d10
Cr-Commit-Position: refs/heads/master@{#435142}