Move chromeos specific utility functions for display to chromeos/display

Move chromeos specific utility functions for display to chromeos/display

BUG=240168
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=200466

[oshima, 2013-05-14 06:52:15]

On 2013/05/13 06:07:45, oshima wrote:

friendly ping

[Jun Mukai, 2013-05-14 07:03:02]

lgtm with nits

but it seems that kInvalidDisplayID should belong to the new output_util.

https://codereview.chromium.org/15067012/diff/11001/chromeos/display/output_c...
File chromeos/display/output_configurator.cc (right):

https://codereview.chromium.org/15067012/diff/11001/chromeos/display/output_c...
chromeos/display/output_configurator.cc:93: display_id(-1) {}
can't we use gfx::Display::kInvalidDisplayID?

https://codereview.chromium.org/15067012/diff/11001/chromeos/display/output_u...
File chromeos/display/output_util.cc (right):

https://codereview.chromium.org/15067012/diff/11001/chromeos/display/output_u...
chromeos/display/output_util.cc:33: (static_cast<int64>(product_code) << 8) |
output_index);
add DCHECK_NE(kInvalidDisplayID, new_id)

or just return the value rather than assigning to a local variable.

[oshima, 2013-05-14 11:16:38]

I want to keep chromeos/display independent of ui/gfx
unless it's absolutely necessary. I made change so that
the initial value of display id won't be used.

https://codereview.chromium.org/15067012/diff/11001/chromeos/display/output_u...
File chromeos/display/output_util.cc (right):

https://codereview.chromium.org/15067012/diff/11001/chromeos/display/output_u...
chromeos/display/output_util.cc:33: (static_cast<int64>(product_code) << 8) |
output_index);
On 2013/05/14 07:03:02, Jun Mukai wrote:
> add DCHECK_NE(kInvalidDisplayID, new_id)
> 
> or just return the value rather than assigning to a local variable.

Done.

[Jun Mukai, 2013-05-14 17:29:33]

LGTM++

indeed display isn't chromeos-only concept, we should keep kInvalidDisplayID in
gfx.  I imagined to introduce a dependency from gfx to chromeos, but that's also
unrealistic for the same reason.

https://codereview.chromium.org/15067012/diff/20001/chromeos/display/output_c...
File chromeos/display/output_configurator.cc (right):

https://codereview.chromium.org/15067012/diff/20001/chromeos/display/output_c...
chromeos/display/output_configurator.cc:93: display_id(0),
why did you change to 0?

[oshima, 2013-05-15 00:59:17]

https://codereview.chromium.org/15067012/diff/20001/chromeos/display/output_c...
File chromeos/display/output_configurator.cc (right):

https://codereview.chromium.org/15067012/diff/20001/chromeos/display/output_c...
chromeos/display/output_configurator.cc:93: display_id(0),
On 2013/05/14 17:29:33, Jun Mukai wrote:
> why did you change to 0?

I want this to be consistent with other fields (which are initialized to 0). The
value doesn't matter b/c initial value won't be used.

[oshima, 2013-05-15 01:00:17]

sky, can you approve changes in ui/?
It's simply moving chromeos specific things from ui/base/x to chromeos/display.

[sky, 2013-05-15 14:38:49]

LGTM - are you sure we won't need these for linux-aura? I suppose if we do we
can move then.

[oshima, 2013-05-16 00:53:31]

On 2013/05/15 14:38:49, sky wrote:
> LGTM - are you sure we won't need these for linux-aura? I suppose if we do we
> can move then.

no, linux-aura doesn't need them because it doesn't manage displays.

[commit-bot: I haz the power, 2013-05-16 00:55:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/15067012/28001

[commit-bot: I haz the power, 2013-05-16 06:24:49]

Change committed as 200466