Mac fullscreen low power: Add unittests

Mac fullscreen low power: Add unittests

Fix a bug in ChildWindowsChanged, discovered by those tests.

Move command line check to controller, to make unittests work.

Don't allow entering low power mode until the fullscreen low power
layer has been valid for 15 frames. This ensures that we won't get
flashes of invalid content.

BUG=616364
Committed: https://crrev.com/15c873daec19bf34e8b3d4925c9668faaba5e634
Cr-Commit-Position: refs/heads/master@{#399135}

[ccameron, 2016-06-08 03:20:00]

Description was changed from

==========
Mac fullscreen low power: Add unittests

And fix a bug in ChildWindowsChanged, discovered by those tests.

Also, don't allow entering low power mode until the fullscreen low power
layer has been valid for 15 frames. This ensures that we won't get
flashes of invalid content.

BUG=594460
==========

to

==========
Mac fullscreen low power: Add unittests

Fix a bug in ChildWindowsChanged, discovered by those tests.

Move command line check to controller, to make unittests work.

Don't allow entering low power mode until the fullscreen low power
layer has been valid for 15 frames. This ensures that we won't get
flashes of invalid content.

BUG= 616364
==========

[ccameron, 2016-06-08 03:20:01]

ccameron@chromium.org changed reviewers:
+ spqchan@chromium.org

[ccameron, 2016-06-08 03:47:18]

ptal

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm (right):

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm:130:
EnterOrExitLowPowerModeIfNeeded();
Oops -- I didn't notice this bug until I wrote a test for it.

[ccameron, 2016-06-08 04:10:32]

Description was changed from

==========
Mac fullscreen low power: Add unittests

Fix a bug in ChildWindowsChanged, discovered by those tests.

Move command line check to controller, to make unittests work.

Don't allow entering low power mode until the fullscreen low power
layer has been valid for 15 frames. This ensures that we won't get
flashes of invalid content.

BUG= 616364
==========

to

==========
Mac fullscreen low power: Add unittests

Fix a bug in ChildWindowsChanged, discovered by those tests.

Move command line check to controller, to make unittests work.

Don't allow entering low power mode until the fullscreen low power
layer has been valid for 15 frames. This ensures that we won't get
flashes of invalid content.

BUG=616364
==========

[spqchan, 2016-06-08 21:43:59]

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_controller_private.mm (right):

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_controller_private.mm:1199: static bool
fullscreen_low_power_enabled_at_command_line =
Why static? Do you mean const?

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm (right):

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm:149: const uint64_t
kMinValidFrames = 15;
Put this as a constant at the top of the file.

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/fullscreen_low_power_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator_unittest.mm:65: //
Return true of the fullscreen low power window is in front of the content
sp: "if"

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator_unittest.mm:129:
TEST_F(FullscreenLowPowerTest, Layout) {
This test might be more effective if you toggle the valid layout with each
invalid layout.

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator_unittest.mm:136: //
Valid layout
period

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator_unittest.mm:177: //
Valid layout again
period

[ccameron, 2016-06-09 20:47:09]

Thanks, updated!

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/browser_window_controller_private.mm (right):

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/browser_window_controller_private.mm:1199: static bool
fullscreen_low_power_enabled_at_command_line =
On 2016/06/08 21:43:58, spqchan wrote:
> Why static? Do you mean const?

Oh -- I wanted to avoid re-looking-up at each call. But it should be const, as
well -- added that.

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm (right):

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm:149: const uint64_t
kMinValidFrames = 15;
On 2016/06/08 21:43:58, spqchan wrote:
> Put this as a constant at the top of the file.

Done.

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/fullscreen_low_power_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator_unittest.mm:65: //
Return true of the fullscreen low power window is in front of the content
On 2016/06/08 21:43:58, spqchan wrote:
> sp: "if"

Done.

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator_unittest.mm:129:
TEST_F(FullscreenLowPowerTest, Layout) {
On 2016/06/08 21:43:58, spqchan wrote:
> This test might be more effective if you toggle the valid layout with each
> invalid layout.

Good point -- added a valid layout after each frame.

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator_unittest.mm:136: //
Valid layout
On 2016/06/08 21:43:59, spqchan wrote:
> period

Done.

https://codereview.chromium.org/2047103003/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator_unittest.mm:177: //
Valid layout again
On 2016/06/08 21:43:58, spqchan wrote:
> period

(deleted, merged into above test)

[spqchan, 2016-06-09 21:09:43]

LGTM with a couple of nits

https://codereview.chromium.org/2047103003/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm (right):

https://codereview.chromium.org/2047103003/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm:7: namespace {
empty line after {

https://codereview.chromium.org/2047103003/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm:11: }  // namespace
empty line before }

[ccameron, 2016-06-09 22:35:22]

Thanks!

https://codereview.chromium.org/2047103003/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm (right):

https://codereview.chromium.org/2047103003/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm:7: namespace {
On 2016/06/09 21:09:43, spqchan wrote:
> empty line after {

Done.

https://codereview.chromium.org/2047103003/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/fullscreen_low_power_coordinator.mm:11: }  // namespace
On 2016/06/09 21:09:43, spqchan wrote:
> empty line before }

Done.

[ccameron, 2016-06-09 22:35:28]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2016-06-09 22:35:28]

The patchset sent to the CQ was uploaded after l-g-t-m from spqchan@chromium.org
Link to the patchset: https://codereview.chromium.org/2047103003/#ps80001
(title: "IRF")

[commit-bot: I haz the power, 2016-06-09 22:36:08]

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

[commit-bot: I haz the power, 2016-06-10 00:36:02]

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

[commit-bot: I haz the power, 2016-06-10 00:36:04]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)

[ccameron, 2016-06-10 08:22:36]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2016-06-10 08:22:50]

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

[commit-bot: I haz the power, 2016-06-10 08:26:37]

Description was changed from

==========
Mac fullscreen low power: Add unittests

Fix a bug in ChildWindowsChanged, discovered by those tests.

Move command line check to controller, to make unittests work.

Don't allow entering low power mode until the fullscreen low power
layer has been valid for 15 frames. This ensures that we won't get
flashes of invalid content.

BUG=616364
==========

to

==========
Mac fullscreen low power: Add unittests

Fix a bug in ChildWindowsChanged, discovered by those tests.

Move command line check to controller, to make unittests work.

Don't allow entering low power mode until the fullscreen low power
layer has been valid for 15 frames. This ensures that we won't get
flashes of invalid content.

BUG=616364
==========

[commit-bot: I haz the power, 2016-06-10 08:26:38]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-10 08:26:43]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-10 08:28:02]

Description was changed from

==========
Mac fullscreen low power: Add unittests

Fix a bug in ChildWindowsChanged, discovered by those tests.

Move command line check to controller, to make unittests work.

Don't allow entering low power mode until the fullscreen low power
layer has been valid for 15 frames. This ensures that we won't get
flashes of invalid content.

BUG=616364
==========

to

==========
Mac fullscreen low power: Add unittests

Fix a bug in ChildWindowsChanged, discovered by those tests.

Move command line check to controller, to make unittests work.

Don't allow entering low power mode until the fullscreen low power
layer has been valid for 15 frames. This ensures that we won't get
flashes of invalid content.

BUG=616364

Committed: https://crrev.com/15c873daec19bf34e8b3d4925c9668faaba5e634
Cr-Commit-Position: refs/heads/master@{#399135}
==========

[commit-bot: I haz the power, 2016-06-10 08:28:03]

Patchset 5 (id:??) landed as
https://crrev.com/15c873daec19bf34e8b3d4925c9668faaba5e634
Cr-Commit-Position: refs/heads/master@{#399135}