Add FakeDisplaySnapshot builder and related changes.

Add FakeDisplaySnapshot builder and related changes.

Adds a builder for FakeDisplaySnapshot to allow us to easily construct
the display snapshots. Will update more tests to use this functionality
in a follow up CL.

Update platform_screen_ozone_unittests.cc to use FakeDisplaySnapshot
instead of DisplaySnapshotVirtual.

Update --screen-config to take display DPI and able to start without any
initial displays.

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

[kylechar, 2016-09-20 14:22:49]

Description was changed from

==========
Add FakeDisplaySnapshot builder and related changes.

Adds a builder for FakeDisplaySnapshot to allow us to easily construct
the class for tests.

Update platform_screen_ozone_unittests.cc to use FakeDisplaySnapshot
instead of DisplaySnapshotVirtual.

Add --display-config=none option to allow FakeDisplayDelegate to start
without any displays.

BUG=TBD
==========

to

==========
Add FakeDisplaySnapshot builder and related changes.

Adds a builder for FakeDisplaySnapshot to allow us to easily construct
the class for tests.

Update platform_screen_ozone_unittests.cc to use FakeDisplaySnapshot
instead of DisplaySnapshotVirtual.

Update --screen-config to take display DPI and to start without any
initial displays.

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

[kylechar, 2016-09-20 16:41:33]

Description was changed from

==========
Add FakeDisplaySnapshot builder and related changes.

Adds a builder for FakeDisplaySnapshot to allow us to easily construct
the class for tests.

Update platform_screen_ozone_unittests.cc to use FakeDisplaySnapshot
instead of DisplaySnapshotVirtual.

Update --screen-config to take display DPI and to start without any
initial displays.

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

to

==========
Add FakeDisplaySnapshot builder and related changes.

Adds a builder for FakeDisplaySnapshot to allow us to easily construct
the display snapshots. Will update more tests to use this functionality
in a follow up CL.

Update platform_screen_ozone_unittests.cc to use FakeDisplaySnapshot
instead of DisplaySnapshotVirtual.

Update --screen-config to take display DPI and ale to start without any
initial displays.

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

[kylechar, 2016-09-20 16:42:56]

Patchset #3 (id:40001) has been deleted

[kylechar, 2016-09-20 16:49:03]

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

[kylechar, 2016-09-20 16:49:04]

derat: ui/display/*
sky: services/ui/display/*

[kylechar, 2016-09-20 16:49:53]

kylechar@chromium.org changed reviewers:
+ derat@chromium.org, sky@chromium.org

[kylechar, 2016-09-20 16:49:54]

Hit enter early last time...

derat: ui/display/*
sky: services/ui/display/*
rjkroege: FYI

[sky, 2016-09-20 17:01:58]

LGTM

[Daniel Erat, 2016-09-20 17:12:49]

mostly looks great, but a few nits

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
File ui/display/fake_display_delegate.cc (right):

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_delegate.cc:66: return false;
nit: worthwhile logging a warning or error here?

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_delegate.cc:217: int found = sscanf(spec.c_str(),
"%dx%d^%d", &width, &height, &dpi);
nit: probably worthwhile adding a comment here explicitly mentioning that dpi is
optional

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_delegate.cc:226:
builder.SetName(base::StringPrintf("Fake Display %d", next_display_id_));
did you mean to use |id|, or maybe |next_display_id_ - 1|, rather than
next_display_id_?

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
File ui/display/fake_display_snapshot.cc (right):

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_snapshot.cc:71: FakeDisplaySnapshot::Builder::~Builder()
{}
nit: drop FakeDisplaySnapshot:: from this line and the c'tor?

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_snapshot.cc:73: std::unique_ptr<FakeDisplaySnapshot>
FakeDisplaySnapshot::Builder::Build() {
same here

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_snapshot.cc:75: NOTREACHED();
nit: include a message here too? << "Modes or display ID missing"

[kylechar, 2016-09-20 17:47:41]

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
File ui/display/fake_display_delegate.cc (right):

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_delegate.cc:66: return false;
On 2016/09/20 17:12:49, Daniel Erat wrote:
> nit: worthwhile logging a warning or error here?

Done.

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_delegate.cc:217: int found = sscanf(spec.c_str(),
"%dx%d^%d", &width, &height, &dpi);
On 2016/09/20 17:12:49, Daniel Erat wrote:
> nit: probably worthwhile adding a comment here explicitly mentioning that dpi
is
> optional

Done.

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_delegate.cc:226:
builder.SetName(base::StringPrintf("Fake Display %d", next_display_id_));
On 2016/09/20 17:12:49, Daniel Erat wrote:
> did you mean to use |id|, or maybe |next_display_id_ - 1|, rather than
> next_display_id_?

It was intended. Using |next_display_id_| after it's incremented means it starts
with "Fake Display 1" which seemed nicer. It would be fine to use
|next_display_id_ - 1| to though. I'd rather not use |id| for the name.

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
File ui/display/fake_display_snapshot.cc (right):

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_snapshot.cc:71: FakeDisplaySnapshot::Builder::~Builder()
{}
On 2016/09/20 17:12:49, Daniel Erat wrote:
> nit: drop FakeDisplaySnapshot:: from this line and the c'tor?

Oh ya, that's not needed with the using. Done.

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_snapshot.cc:73: std::unique_ptr<FakeDisplaySnapshot>
FakeDisplaySnapshot::Builder::Build() {
On 2016/09/20 17:12:49, Daniel Erat wrote:
> same here

Done.

https://codereview.chromium.org/2340383002/diff/60001/ui/display/fake_display...
ui/display/fake_display_snapshot.cc:75: NOTREACHED();
On 2016/09/20 17:12:49, Daniel Erat wrote:
> nit: include a message here too? << "Modes or display ID missing"

Done.

[Daniel Erat, 2016-09-20 18:01:15]

lgtm

thanks!

[kylechar, 2016-09-20 18:01:55]

Thanks derat/sky for the quick review.

[kylechar, 2016-09-20 18:02:01]

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

[commit-bot: I haz the power, 2016-09-20 18:02:40]

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

[kylechar, 2016-09-20 18:07:42]

Description was changed from

==========
Add FakeDisplaySnapshot builder and related changes.

Adds a builder for FakeDisplaySnapshot to allow us to easily construct
the display snapshots. Will update more tests to use this functionality
in a follow up CL.

Update platform_screen_ozone_unittests.cc to use FakeDisplaySnapshot
instead of DisplaySnapshotVirtual.

Update --screen-config to take display DPI and ale to start without any
initial displays.

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

to

==========
Add FakeDisplaySnapshot builder and related changes.

Adds a builder for FakeDisplaySnapshot to allow us to easily construct
the display snapshots. Will update more tests to use this functionality
in a follow up CL.

Update platform_screen_ozone_unittests.cc to use FakeDisplaySnapshot
instead of DisplaySnapshotVirtual.

Update --screen-config to take display DPI and able to start without any
initial displays.

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

[commit-bot: I haz the power, 2016-09-20 20:24:41]

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

[commit-bot: I haz the power, 2016-09-20 20:24:42]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[kylechar, 2016-09-20 20:39:50]

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

[commit-bot: I haz the power, 2016-09-20 20:40:20]

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-20 21:29:36]

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

[commit-bot: I haz the power, 2016-09-20 21:29:37]

Dry run: This issue passed the CQ dry run.

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

The CQ bit was checked by kylechar@chromium.org

[kylechar, 2016-09-20 22:55:56]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2340383002/#ps80001
(title: "Fixes for comments.")

[commit-bot: I haz the power, 2016-09-20 22:56:44]

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-20 23:04:33]

Description was changed from

==========
Add FakeDisplaySnapshot builder and related changes.

Adds a builder for FakeDisplaySnapshot to allow us to easily construct
the display snapshots. Will update more tests to use this functionality
in a follow up CL.

Update platform_screen_ozone_unittests.cc to use FakeDisplaySnapshot
instead of DisplaySnapshotVirtual.

Update --screen-config to take display DPI and able to start without any
initial displays.

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

to

==========
Add FakeDisplaySnapshot builder and related changes.

Adds a builder for FakeDisplaySnapshot to allow us to easily construct
the display snapshots. Will update more tests to use this functionality
in a follow up CL.

Update platform_screen_ozone_unittests.cc to use FakeDisplaySnapshot
instead of DisplaySnapshotVirtual.

Update --screen-config to take display DPI and able to start without any
initial displays.

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

[commit-bot: I haz the power, 2016-09-20 23:04:35]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-09-20 23:06:55]

Description was changed from

==========
Add FakeDisplaySnapshot builder and related changes.

Adds a builder for FakeDisplaySnapshot to allow us to easily construct
the display snapshots. Will update more tests to use this functionality
in a follow up CL.

Update platform_screen_ozone_unittests.cc to use FakeDisplaySnapshot
instead of DisplaySnapshotVirtual.

Update --screen-config to take display DPI and able to start without any
initial displays.

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

to

==========
Add FakeDisplaySnapshot builder and related changes.

Adds a builder for FakeDisplaySnapshot to allow us to easily construct
the display snapshots. Will update more tests to use this functionality
in a follow up CL.

Update platform_screen_ozone_unittests.cc to use FakeDisplaySnapshot
instead of DisplaySnapshotVirtual.

Update --screen-config to take display DPI and able to start without any
initial displays.

BUG=611475

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

[commit-bot: I haz the power, 2016-09-20 23:06:56]

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