Add FakeDisplayDelegate for off device usage.

Add FakeDisplayDelegate for off device usage.

Add a NativeDisplayDelegate implementation that handles fake displays.
Fake displays mimic physical displays when running off device. Allow
user the specify initial display state with command line flag and
provide API to programatically change display state at runtime.

Except for changing the attached displays other NativeDisplayDelegate
functionality is a noop.

Add FakeDisplaySnapshot to store the fake display state. For now it's
very simple but I intend to add a builder to make it easy to construct
more complex displays.

Add FakeDisplayController interface and method on NativeDisplayDelegate
to get one. This interface will allow changing display state at runtime
if implemented. Only FakeDisplayDelegate will implement it for now.

Fake isn't the best term to describe "not real" displays but there are
already virtual displays implemented for casting.

BUG=611475
Committed: https://crrev.com/b77b33d434ddf9fbb31193c559d9907df2020db1
Cr-Commit-Position: refs/heads/master@{#419505}

[kylechar, 2016-09-09 13:59:38]

Description was changed from

==========
Add VirtualDisplayDelegate.

Add a NativeDisplayDelegate implementation that handles virtual
displays. Virtual displays mimic physical displays for running off
device. Allow user the specify initial display state with command line
flag and provide API to programatically change display state at
runtime.

BUG=611475
==========

to

==========
Add VirtualDisplayDelegate.

Add a NativeDisplayDelegate implementation that handles virtual
displays. Virtual displays mimic physical displays for running off
device. Allow user the specify initial display state with command line
flag and provide API to programatically change display state at
runtime.

Other than changing the attached displays other NativeDisplayDelegate
functionality is a noop.

BUG=611475
==========

[kylechar, 2016-09-09 14:01:54]

kylechar@chromium.org changed reviewers:
+ oshima@chromium.org, rjkroege@chromium.org

[kylechar, 2016-09-09 14:01:54]

oshima: ui/display/*
rjkroege: ui/ozone/*

[kylechar, 2016-09-09 20:13:22]

Description was changed from

==========
Add VirtualDisplayDelegate.

Add a NativeDisplayDelegate implementation that handles virtual
displays. Virtual displays mimic physical displays for running off
device. Allow user the specify initial display state with command line
flag and provide API to programatically change display state at
runtime.

Other than changing the attached displays other NativeDisplayDelegate
functionality is a noop.

BUG=611475
==========

to

==========
Add VirtualDisplayDelegate.

Add a NativeDisplayDelegate implementation that handles virtual
displays. Virtual displays mimic physical displays for running off
device. Allow user the specify initial display state with command line
flag and provide API to programatically change display state at
runtime.

Except for changing the attached displays other NativeDisplayDelegate
functionality is a noop.

BUG=611475
==========

[rjkroege, 2016-09-12 20:31:06]

Maybe correct the typo in the description?

https://codereview.chromium.org/2324163002/diff/20001/ui/display/display_swit...
File ui/display/display_switches.cc (right):

https://codereview.chromium.org/2324163002/diff/20001/ui/display/display_swit...
ui/display/display_switches.cc:15: const char kDisplayBounds[] =
"display-bounds";
this is redundant with HostDisplayBounds? Refactor please?

https://codereview.chromium.org/2324163002/diff/20001/ui/ozone/platform/headl...
File ui/ozone/platform/headless/ozone_platform_headless.cc (right):

https://codereview.chromium.org/2324163002/diff/20001/ui/ozone/platform/headl...
ui/ozone/platform/headless/ozone_platform_headless.cc:12: #include
"ui/display/chromeos/virtual_display_delegate.h"
no. this code needs to build with chromeos=false

https://codereview.chromium.org/2324163002/diff/20001/ui/ozone/platform/x11/o...
File ui/ozone/platform/x11/ozone_platform_x11.cc (right):

https://codereview.chromium.org/2324163002/diff/20001/ui/ozone/platform/x11/o...
ui/ozone/platform/x11/ozone_platform_x11.cc:15: #include
"ui/display/chromeos/virtual_display_delegate.h"
same comment. we will want this to build without chromeos.

[kylechar, 2016-09-13 14:15:01]

https://codereview.chromium.org/2324163002/diff/20001/ui/display/display_swit...
File ui/display/display_switches.cc (right):

https://codereview.chromium.org/2324163002/diff/20001/ui/display/display_swit...
ui/display/display_switches.cc:15: const char kDisplayBounds[] =
"display-bounds";
On 2016/09/12 20:31:05, rjkroege wrote:
> this is redundant with HostDisplayBounds? Refactor please?

They're similar but incompatible, so I think a different name is appropriate?

Although, I think --display-config= is maybe a better flag name. The
--ash-window-host-bounds= no longer matches reality (it's used to specify much
more than just bounds) although it was probably accurate at some point.

https://codereview.chromium.org/2324163002/diff/20001/ui/ozone/platform/headl...
File ui/ozone/platform/headless/ozone_platform_headless.cc (right):

https://codereview.chromium.org/2324163002/diff/20001/ui/ozone/platform/headl...
ui/ozone/platform/headless/ozone_platform_headless.cc:12: #include
"ui/display/chromeos/virtual_display_delegate.h"
On 2016/09/12 20:31:05, rjkroege wrote:
> no. this code needs to build with chromeos=false

I could move it to src/ui/display/ then?

[kylechar, 2016-09-13 15:31:22]

I've been doing a bit of digging into the original reason DisplaySnapshotVirtual
was added and found crbug.com/425060. The "virtual display" concept actually has
some additional significance related to casting (that isn't used yet). It's
probably not appropriate for me to take over the "virtual display" idea though.
So I'll have to find a new name and add a new DisplaySnapshot implementation.

[kylechar, 2016-09-13 18:05:46]

Patchset #3 (id:40001) has been deleted

[kylechar, 2016-09-13 18:36:18]

Description was changed from

==========
Add VirtualDisplayDelegate.

Add a NativeDisplayDelegate implementation that handles virtual
displays. Virtual displays mimic physical displays for running off
device. Allow user the specify initial display state with command line
flag and provide API to programatically change display state at
runtime.

Except for changing the attached displays other NativeDisplayDelegate
functionality is a noop.

BUG=611475
==========

to

==========
Add FakeDisplayDelegate for off device usage.

Add a NativeDisplayDelegate implementation that handles fake displays.
Fake displays mimic physical displays when running off device. Allow
user the specify initial display state with command line flag and
provide API to programatically change display state at runtime.

Except for changing the attached displays other NativeDisplayDelegate
functionality is a noop.

Add FakeDisplaySnapshot to store the fake display state. For now it's
very simple but I intend to add a builder to make it easy to construct
fake more complex examples.

Add FakeDisplayController interface and method on NativeDisplayDelegate
to get one. This interface will allow changing display state at runtime
if implemented. Only FakeDisplayDelegate will implement it for now.

Fake isn't the best term to describe "not real" displays but there are
already virtual displays implemented for casting.

BUG=611475
==========

[kylechar, 2016-09-13 18:37:48]

I've updated the CL and used "fake display" instead of "virtual display". It's
not perfect but unless the original idea is abandoned I can't use "virtual
display".

[kylechar, 2016-09-13 19:08:24]

Description was changed from

==========
Add FakeDisplayDelegate for off device usage.

Add a NativeDisplayDelegate implementation that handles fake displays.
Fake displays mimic physical displays when running off device. Allow
user the specify initial display state with command line flag and
provide API to programatically change display state at runtime.

Except for changing the attached displays other NativeDisplayDelegate
functionality is a noop.

Add FakeDisplaySnapshot to store the fake display state. For now it's
very simple but I intend to add a builder to make it easy to construct
fake more complex examples.

Add FakeDisplayController interface and method on NativeDisplayDelegate
to get one. This interface will allow changing display state at runtime
if implemented. Only FakeDisplayDelegate will implement it for now.

Fake isn't the best term to describe "not real" displays but there are
already virtual displays implemented for casting.

BUG=611475
==========

to

==========
Add FakeDisplayDelegate for off device usage.

Add a NativeDisplayDelegate implementation that handles fake displays.
Fake displays mimic physical displays when running off device. Allow
user the specify initial display state with command line flag and
provide API to programatically change display state at runtime.

Except for changing the attached displays other NativeDisplayDelegate
functionality is a noop.

Add FakeDisplaySnapshot to store the fake display state. For now it's
very simple but I intend to add a builder to make it easy to construct
more complex displays.

Add FakeDisplayController interface and method on NativeDisplayDelegate
to get one. This interface will allow changing display state at runtime
if implemented. Only FakeDisplayDelegate will implement it for now.

Fake isn't the best term to describe "not real" displays but there are
already virtual displays implemented for casting.

BUG=611475
==========

[kylechar, 2016-09-14 20:44:55]

Ping. Will you have time to look this week oshima/rjkroege?

[rjkroege, 2016-09-15 00:52:46]

lgtm with some nits/comments that could be addressed mostly with TODOs.

https://codereview.chromium.org/2324163002/diff/100001/ui/display/display_swi...
File ui/display/display_switches.h (right):

https://codereview.chromium.org/2324163002/diff/100001/ui/display/display_swi...
ui/display/display_switches.h:18: DISPLAY_EXPORT extern const char
kHostWindowBounds[];
plz put TODO to remove this given its functionality overlap with kDisplayConfig.

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
File ui/display/fake_display_delegate.cc (right):

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
ui/display/fake_display_delegate.cc:40: // Add the first display as internal.
I could see it desirable to see what happens to upstream code when the first
display is not the internal. Should a future extension of this code permit the
command line flag to specify which display is internal?

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
File ui/display/fake_display_delegate.h (right):

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
ui/display/fake_display_delegate.h:34: //  --display-config=800x800,800x800
This is a screen config yes? I'd like us to follow the mostly standard
nomenclature.

A NativeDisplayDelegate is mis-named. It should be a NativeScreenDelegate. But
we've not done that yet.

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
File ui/display/fake_display_snapshot.cc (right):

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
ui/display/fake_display_snapshot.cc:31: "Fake Display",
given that there could be more than one, can you give each display a different
name?

[kylechar, 2016-09-15 17:31:54]

kylechar@chromium.org changed reviewers:
+ msw@chromium.org

[kylechar, 2016-09-15 17:31:55]

+msw for services/ui/*

https://codereview.chromium.org/2324163002/diff/100001/ui/display/display_swi...
File ui/display/display_switches.h (right):

https://codereview.chromium.org/2324163002/diff/100001/ui/display/display_swi...
ui/display/display_switches.h:18: DISPLAY_EXPORT extern const char
kHostWindowBounds[];
On 2016/09/15 00:52:46, rjkroege wrote:
> plz put TODO to remove this given its functionality overlap with
kDisplayConfig.

Done.

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
File ui/display/fake_display_delegate.cc (right):

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
ui/display/fake_display_delegate.cc:40: // Add the first display as internal.
On 2016/09/15 00:52:46, rjkroege wrote:
> I could see it desirable to see what happens to upstream code when the first
> display is not the internal. Should a future extension of this code permit the
> command line flag to specify which display is internal?

Yes definitely.

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
File ui/display/fake_display_delegate.h (right):

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
ui/display/fake_display_delegate.h:34: //  --display-config=800x800,800x800
On 2016/09/15 00:52:46, rjkroege wrote:
> This is a screen config yes? I'd like us to follow the mostly standard
> nomenclature.
> 
> A NativeDisplayDelegate is mis-named. It should be a NativeScreenDelegate. But
> we've not done that yet.

sgtm, changed to --screen-config.

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
File ui/display/fake_display_snapshot.cc (right):

https://codereview.chromium.org/2324163002/diff/100001/ui/display/fake_displa...
ui/display/fake_display_snapshot.cc:31: "Fake Display",
On 2016/09/15 00:52:46, rjkroege wrote:
> given that there could be more than one, can you give each display a different
> name?

Done.

[msw, 2016-09-15 19:30:08]

services/ui lgtm with some very optional nits

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
File services/ui/display/platform_screen_ozone.cc (right):

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
services/ui/display/platform_screen_ozone.cc:79: wait_for_display_update_ =
true;
optional nit: wait_for_display_update_ =
fake_display_controller_->AddDisplay(display_size) != Display::kInvalidDisplayID

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
services/ui/display/platform_screen_ozone.cc:82: wait_for_display_update_ =
true;
optional nit: wait_for_display_update_ =
fake_display_controller_->RemoveDisplay(cached_displays_.back().id)

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
File services/ui/display/platform_screen_ozone.h (right):

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
services/ui/display/platform_screen_ozone.h:106: // If not null it provides a
way to modify the display state when running off
optional nit: "when running without a device (ie. linux desktop)" or similar?
(ok as-is if "off device" is already widely accepted terminology for that)

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
services/ui/display/platform_screen_ozone.h:112: bool wait_for_display_update_ =
false;
optional nit: specialize this flag and comment for display mode changes? (this
flag shouldn't be re-used for other, potentially concurrent changes, right?)

[kylechar, 2016-09-15 21:35:40]

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
File services/ui/display/platform_screen_ozone.cc (right):

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
services/ui/display/platform_screen_ozone.cc:79: wait_for_display_update_ =
true;
On 2016/09/15 19:30:08, msw wrote:
> optional nit: wait_for_display_update_ =
> fake_display_controller_->AddDisplay(display_size) !=
Display::kInvalidDisplayID

Done.

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
services/ui/display/platform_screen_ozone.cc:82: wait_for_display_update_ =
true;
On 2016/09/15 19:30:08, msw wrote:
> optional nit: wait_for_display_update_ =
> fake_display_controller_->RemoveDisplay(cached_displays_.back().id)

Done.

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
File services/ui/display/platform_screen_ozone.h (right):

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
services/ui/display/platform_screen_ozone.h:106: // If not null it provides a
way to modify the display state when running off
On 2016/09/15 19:30:08, msw wrote:
> optional nit: "when running without a device (ie. linux desktop)" or similar?
> (ok as-is if "off device" is already widely accepted terminology for that)

It's the terminology Rob and I have been using, not sure if it's accepted. I've
added the clarification part though!

https://codereview.chromium.org/2324163002/diff/120001/services/ui/display/pl...
services/ui/display/platform_screen_ozone.h:112: bool wait_for_display_update_ =
false;
On 2016/09/15 19:30:08, msw wrote:
> optional nit: specialize this flag and comment for display mode changes? (this
> flag shouldn't be re-used for other, potentially concurrent changes, right?)

I'm not totally sure there is a case where we want concurrent changes?
ui::DisplayConfigurator posts a task with a 500ms delay before it handles the
display configuration changing. I'd imagine any user input that should trigger a
configuration change is going to want to wait until the last configuration
change is done.

It's possible that power state or content protection would happen concurrently
though? I'll change it to wait_for_display_config_update_ to be a bit clearer.

[msw, 2016-09-15 22:38:32]

lgtm

[oshima, 2016-09-16 20:50:00]

lgtm

https://codereview.chromium.org/2324163002/diff/140001/ui/display/display_swi...
File ui/display/display_switches.cc (right):

https://codereview.chromium.org/2324163002/diff/140001/ui/display/display_swi...
ui/display/display_switches.cc:39: const char kUseFirstDisplayAsInternal[] =
"ash-use-first-display-as-internal";
you may remove "ash-".

[kylechar, 2016-09-16 20:59:15]

Thanks oshima!

https://codereview.chromium.org/2324163002/diff/140001/ui/display/display_swi...
File ui/display/display_switches.cc (right):

https://codereview.chromium.org/2324163002/diff/140001/ui/display/display_swi...
ui/display/display_switches.cc:39: const char kUseFirstDisplayAsInternal[] =
"ash-use-first-display-as-internal";
On 2016/09/16 20:50:00, oshima wrote:
> you may remove "ash-". 

Done.

[kylechar, 2016-09-16 20:59:21]

The CQ bit was checked by kylechar@chromium.org

[kylechar, 2016-09-16 20:59:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjkroege@chromium.org, msw@chromium.org, oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/2324163002/#ps160001
(title: "Address oshima comments.")

[commit-bot: I haz the power, 2016-09-16 21:00:02]

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-09-16 21:08:17]

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

[commit-bot: I haz the power, 2016-09-16 21:08:19]

Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[kylechar, 2016-09-19 13:49:34]

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

[commit-bot: I haz the power, 2016-09-19 13:49:54]

Dry run: 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-09-19 14:27:53]

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

[commit-bot: I haz the power, 2016-09-19 14:27:54]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[kylechar, 2016-09-19 14:55:11]

Patchset #10 (id:200001) has been deleted

[kylechar, 2016-09-19 16:54:59]

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

[commit-bot: I haz the power, 2016-09-19 16:55:21]

Dry run: 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-09-19 18:05:48]

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

[commit-bot: I haz the power, 2016-09-19 18:05:49]

Dry run: This issue passed the CQ dry run.

[kylechar, 2016-09-19 18:06:21]

The CQ bit was checked by kylechar@chromium.org

[kylechar, 2016-09-19 18:06:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjkroege@chromium.org, msw@chromium.org, oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/2324163002/#ps240001
(title: "Fix windows compile problems.")

[commit-bot: I haz the power, 2016-09-19 18:06:46]

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-09-19 18:12:26]

Description was changed from

==========
Add FakeDisplayDelegate for off device usage.

Add a NativeDisplayDelegate implementation that handles fake displays.
Fake displays mimic physical displays when running off device. Allow
user the specify initial display state with command line flag and
provide API to programatically change display state at runtime.

Except for changing the attached displays other NativeDisplayDelegate
functionality is a noop.

Add FakeDisplaySnapshot to store the fake display state. For now it's
very simple but I intend to add a builder to make it easy to construct
more complex displays.

Add FakeDisplayController interface and method on NativeDisplayDelegate
to get one. This interface will allow changing display state at runtime
if implemented. Only FakeDisplayDelegate will implement it for now.

Fake isn't the best term to describe "not real" displays but there are
already virtual displays implemented for casting.

BUG=611475
==========

to

==========
Add FakeDisplayDelegate for off device usage.

Add a NativeDisplayDelegate implementation that handles fake displays.
Fake displays mimic physical displays when running off device. Allow
user the specify initial display state with command line flag and
provide API to programatically change display state at runtime.

Except for changing the attached displays other NativeDisplayDelegate
functionality is a noop.

Add FakeDisplaySnapshot to store the fake display state. For now it's
very simple but I intend to add a builder to make it easy to construct
more complex displays.

Add FakeDisplayController interface and method on NativeDisplayDelegate
to get one. This interface will allow changing display state at runtime
if implemented. Only FakeDisplayDelegate will implement it for now.

Fake isn't the best term to describe "not real" displays but there are
already virtual displays implemented for casting.

BUG=611475
==========

[commit-bot: I haz the power, 2016-09-19 18:12:27]

Committed patchset #11 (id:240001)

[commit-bot: I haz the power, 2016-09-19 18:13:59]

Description was changed from

==========
Add FakeDisplayDelegate for off device usage.

Add a NativeDisplayDelegate implementation that handles fake displays.
Fake displays mimic physical displays when running off device. Allow
user the specify initial display state with command line flag and
provide API to programatically change display state at runtime.

Except for changing the attached displays other NativeDisplayDelegate
functionality is a noop.

Add FakeDisplaySnapshot to store the fake display state. For now it's
very simple but I intend to add a builder to make it easy to construct
more complex displays.

Add FakeDisplayController interface and method on NativeDisplayDelegate
to get one. This interface will allow changing display state at runtime
if implemented. Only FakeDisplayDelegate will implement it for now.

Fake isn't the best term to describe "not real" displays but there are
already virtual displays implemented for casting.

BUG=611475
==========

to

==========
Add FakeDisplayDelegate for off device usage.

Add a NativeDisplayDelegate implementation that handles fake displays.
Fake displays mimic physical displays when running off device. Allow
user the specify initial display state with command line flag and
provide API to programatically change display state at runtime.

Except for changing the attached displays other NativeDisplayDelegate
functionality is a noop.

Add FakeDisplaySnapshot to store the fake display state. For now it's
very simple but I intend to add a builder to make it easy to construct
more complex displays.

Add FakeDisplayController interface and method on NativeDisplayDelegate
to get one. This interface will allow changing display state at runtime
if implemented. Only FakeDisplayDelegate will implement it for now.

Fake isn't the best term to describe "not real" displays but there are
already virtual displays implemented for casting.

BUG=611475

Committed: https://crrev.com/b77b33d434ddf9fbb31193c559d9907df2020db1
Cr-Commit-Position: refs/heads/master@{#419505}
==========

[commit-bot: I haz the power, 2016-09-19 18:14:00]

Patchset 11 (id:??) landed as
https://crrev.com/b77b33d434ddf9fbb31193c559d9907df2020db1
Cr-Commit-Position: refs/heads/master@{#419505}