Move kInvalidDisplayID to display_constants.h.

Move kInvalidDisplayID to display_constants.h.

There are targets that depend on //ui/display just for that constant.
There is a smaller target //ui/display/types that is more suitable for
this.  Move the constant and update all references in src/ui/*. Other
references to display::Display::kInvalidDisplayID will be updated in a
follow up CL.

BUG=667344
Committed: https://crrev.com/7bfba989d85ca0d33e316802507e2e59b4c756ab
Cr-Commit-Position: refs/heads/master@{#433638}

[kylechar, 2016-11-21 17:09:24]

Description was changed from

==========
Move kInvalidDisplayID to display_constants.h.

There are targets that depend on //ui/display just for that constant.
There is a smaller target //ui/display/types that is more suitable for
this.  Move the constant and update all references in src/ui/*. Other
references to display::Display::kInvalidDisplayID will be updated in a
follow up CL.

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

to

==========
Move kInvalidDisplayID to display_constants.h.

There are targets that depend on //ui/display just for that constant.
There is a smaller target //ui/display/types that is more suitable for
this.  Move the constant and update all references in src/ui/*. Other
references to display::Display::kInvalidDisplayID will be updated in a
follow up CL.

BUG=667344
==========

[kylechar, 2016-11-21 17:11:17]

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

[kylechar, 2016-11-21 17:11:18]

+derat for ui/display/*
+sadrul for the other ui/* files.

[sadrul, 2016-11-21 17:31:17]

lgtm

https://codereview.chromium.org/2519993002/diff/20001/ui/display/display.h
File ui/display/display.h (right):

https://codereview.chromium.org/2519993002/diff/20001/ui/display/display.h#ne...
ui/display/display.h:70: enum : int64_t { kInvalidDisplayID =
display::kInvalidDisplayID };
Get rid of it?

https://codereview.chromium.org/2519993002/diff/20001/ui/display/manager/disp...
File ui/display/manager/display_manager_utilities_unittest.cc (right):

https://codereview.chromium.org/2519993002/diff/20001/ui/display/manager/disp...
ui/display/manager/display_manager_utilities_unittest.cc:5: // #include
"ash/display/display_util.h"
Remove this too?

https://codereview.chromium.org/2519993002/diff/20001/ui/display/types/displa...
File ui/display/types/display_constants.h (right):

https://codereview.chromium.org/2519993002/diff/20001/ui/display/types/displa...
ui/display/types/display_constants.h:21: enum MultipleDisplayState {
We probably want to move these to display namespace too at some point.

[Daniel Erat, 2016-11-21 17:33:57]

lgtm

https://codereview.chromium.org/2519993002/diff/20001/ui/display/types/displa...
File ui/display/types/display_constants.h (right):

https://codereview.chromium.org/2519993002/diff/20001/ui/display/types/displa...
ui/display/types/display_constants.h:13: // before display ids are known.
nit: s/id/ID/g in this comment

https://codereview.chromium.org/2519993002/diff/20001/ui/display/types/displa...
ui/display/types/display_constants.h:14: constexpr int64_t kInvalidDisplayID =
-1;
it's a pretty big nit, but i think that "Id" is more common in constant names
than "ID":

% git grep 'const.*k[A-Z][a-zA-Z]*Id\b' | wc -l
3022
% git grep 'const.*k[A-Z][a-zA-Z]*ID\b' | wc -l
553

https://google.github.io/styleguide/cppguide.html#Function_Names touches on this
too: "Prefer to capitalize acronyms as single words (i.e. StartRpc(), not
StartRPC())." (a later sentence says that this also applies to constants.)

i realize that you're essentially just moving this out of a class, though, so
feel free to disregard this if it's a pain to update now. :-)

[kylechar, 2016-11-21 17:50:52]

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

[commit-bot: I haz the power, 2016-11-21 17:51:14]

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

[kylechar, 2016-11-21 17:51:18]

Thanks!

https://codereview.chromium.org/2519993002/diff/20001/ui/display/display.h
File ui/display/display.h (right):

https://codereview.chromium.org/2519993002/diff/20001/ui/display/display.h#ne...
ui/display/display.h:70: enum : int64_t { kInvalidDisplayID =
display::kInvalidDisplayID };
On 2016/11/21 17:31:17, sadrul wrote:
> Get rid of it?

It's still used everywhere that isn't src/ui/* but I'll switch that next and
remove it. Added a TODO.

https://codereview.chromium.org/2519993002/diff/20001/ui/display/manager/disp...
File ui/display/manager/display_manager_utilities_unittest.cc (right):

https://codereview.chromium.org/2519993002/diff/20001/ui/display/manager/disp...
ui/display/manager/display_manager_utilities_unittest.cc:5: // #include
"ash/display/display_util.h"
On 2016/11/21 17:31:17, sadrul wrote:
> Remove this too?

Done.

https://codereview.chromium.org/2519993002/diff/20001/ui/display/types/displa...
File ui/display/types/display_constants.h (right):

https://codereview.chromium.org/2519993002/diff/20001/ui/display/types/displa...
ui/display/types/display_constants.h:13: // before display ids are known.
On 2016/11/21 17:33:57, Daniel Erat wrote:
> nit: s/id/ID/g in this comment

Done.

https://codereview.chromium.org/2519993002/diff/20001/ui/display/types/displa...
ui/display/types/display_constants.h:14: constexpr int64_t kInvalidDisplayID =
-1;
On 2016/11/21 17:33:57, Daniel Erat wrote:
> it's a pretty big nit, but i think that "Id" is more common in constant names
> than "ID":
> 
> % git grep 'const.*k[A-Z][a-zA-Z]*Id\b' | wc -l
> 3022
> % git grep 'const.*k[A-Z][a-zA-Z]*ID\b' | wc -l
> 553
> 
> https://google.github.io/styleguide/cppguide.html#Function_Names touches on
this
> too: "Prefer to capitalize acronyms as single words (i.e. StartRpc(), not
> StartRPC())." (a later sentence says that this also applies to constants.)
> 
> i realize that you're essentially just moving this out of a class, though, so
> feel free to disregard this if it's a pain to update now. :-)

I actually like kInvalidDisplayId better too. Changed.

https://codereview.chromium.org/2519993002/diff/20001/ui/display/types/displa...
ui/display/types/display_constants.h:21: enum MultipleDisplayState {
On 2016/11/21 17:31:17, sadrul wrote:
> We probably want to move these to display namespace too at some point.

Totally agreed. There is a big chunk of ui/display/chromeos and ui/display/types
that needs to switch namespaces still.

[Daniel Erat, 2016-11-21 17:57:21]

lgtm

[kylechar, 2016-11-21 19:17:13]

The CQ bit was unchecked by kylechar@chromium.org

[kylechar, 2016-11-21 19:17:13]

The CQ bit was checked by kylechar@chromium.org

[kylechar, 2016-11-21 19:17:14]

The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/2519993002/#ps40001
(title: "Address comments.")

[commit-bot: I haz the power, 2016-11-21 19:17:28]

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-21 20:44:06]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1479755833581010,
"parent_rev": "daf1deed64a7997886eea131d94a99cc7f51302e", "commit_rev":
"ac913641de8e23d631e73f108ad1598b1db0bc6f"}

[commit-bot: I haz the power, 2016-11-21 20:44:33]

Description was changed from

==========
Move kInvalidDisplayID to display_constants.h.

There are targets that depend on //ui/display just for that constant.
There is a smaller target //ui/display/types that is more suitable for
this.  Move the constant and update all references in src/ui/*. Other
references to display::Display::kInvalidDisplayID will be updated in a
follow up CL.

BUG=667344
==========

to

==========
Move kInvalidDisplayID to display_constants.h.

There are targets that depend on //ui/display just for that constant.
There is a smaller target //ui/display/types that is more suitable for
this.  Move the constant and update all references in src/ui/*. Other
references to display::Display::kInvalidDisplayID will be updated in a
follow up CL.

BUG=667344
==========

[commit-bot: I haz the power, 2016-11-21 20:44:36]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-21 20:48:43]

Description was changed from

==========
Move kInvalidDisplayID to display_constants.h.

There are targets that depend on //ui/display just for that constant.
There is a smaller target //ui/display/types that is more suitable for
this.  Move the constant and update all references in src/ui/*. Other
references to display::Display::kInvalidDisplayID will be updated in a
follow up CL.

BUG=667344
==========

to

==========
Move kInvalidDisplayID to display_constants.h.

There are targets that depend on //ui/display just for that constant.
There is a smaller target //ui/display/types that is more suitable for
this.  Move the constant and update all references in src/ui/*. Other
references to display::Display::kInvalidDisplayID will be updated in a
follow up CL.

BUG=667344

Committed: https://crrev.com/7bfba989d85ca0d33e316802507e2e59b4c756ab
Cr-Commit-Position: refs/heads/master@{#433638}
==========

[commit-bot: I haz the power, 2016-11-21 20:48:45]

Patchset 3 (id:??) landed as
https://crrev.com/7bfba989d85ca0d33e316802507e2e59b4c756ab
Cr-Commit-Position: refs/heads/master@{#433638}