Add Get/SetDisplayLayout to chrome.system.display extension API

Add Get/SetDisplayLayout to chrome.system.display extension API

This will allow Settings and kiosk apps to get and set display
layout info.

BUG=547080
Committed: https://crrev.com/c4ce17637acfa3139a57a001e6f074fe5b3244cd
Cr-Commit-Position: refs/heads/master@{#400863}

[stevenjb, 2016-06-16 00:57:05]

stevenjb@chromium.org changed reviewers:
+ asargent@chromium.org, oshima@chromium.org, xiyuan@chromium.org

[asargent_no_longer_on_chrome, 2016-06-16 19:08:31]

extensions parts lgtm w/ one optional suggestion

https://codereview.chromium.org/2072633002/diff/20001/extensions/common/api/s...
File extensions/common/api/system_display.idl (right):

https://codereview.chromium.org/2072633002/diff/20001/extensions/common/api/s...
extensions/common/api/system_display.idl:205: static void
setDisplayLayout(DisplayLayout[] layouts);
optional: you might consider adding a callback which reports back whether the
layouts passed were used successfully or had to be adjusted. Callers can likely
figure that out themselves by listening for the onDisplayChanged, but doing so
might be a bit more cumbersome than if you just told them directly.

[xiyuan, 2016-06-16 20:11:52]

lgtm

https://codereview.chromium.org/2072633002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/display_info_provider_chromeos.cc (right):

https://codereview.chromium.org/2072633002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/display_info_provider_chromeos.cc:70: break;
nit: unnecessary since we return in the above line?

https://codereview.chromium.org/2072633002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/display_info_provider_chromeos.cc:359:
extensions::system_display::DisplayMode GetDisplayMode(
nit: "extensions::" is also unnecessary since we are in extensions namespace
here.

https://codereview.chromium.org/2072633002/diff/20001/extensions/browser/api/...
File extensions/browser/api/system_display/system_display_api.cc (right):

https://codereview.chromium.org/2072633002/diff/20001/extensions/browser/api/...
extensions/browser/api/system_display/system_display_api.cc:77:
SetError("Function available only on ChromeOS.");
SetError(kCrosOnlyError);

[stevenjb, 2016-06-17 01:34:58]

stevenjb@chromium.org changed reviewers:
+ isherman@chromium.org

[stevenjb, 2016-06-17 01:34:59]

+ isherman@chromium.org for histograms

https://codereview.chromium.org/2072633002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/display_info_provider_chromeos.cc (right):

https://codereview.chromium.org/2072633002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/display_info_provider_chromeos.cc:70: break;
On 2016/06/16 20:11:51, xiyuan wrote:
> nit: unnecessary since we return in the above line?

Done.

https://codereview.chromium.org/2072633002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/display_info_provider_chromeos.cc:359:
extensions::system_display::DisplayMode GetDisplayMode(
On 2016/06/16 20:11:51, xiyuan wrote:
> nit: "extensions::" is also unnecessary since we are in extensions namespace
> here.

Done.

https://codereview.chromium.org/2072633002/diff/20001/extensions/browser/api/...
File extensions/browser/api/system_display/system_display_api.cc (right):

https://codereview.chromium.org/2072633002/diff/20001/extensions/browser/api/...
extensions/browser/api/system_display/system_display_api.cc:77:
SetError("Function available only on ChromeOS.");
On 2016/06/16 20:11:52, xiyuan wrote:
> SetError(kCrosOnlyError);

Done.

https://codereview.chromium.org/2072633002/diff/20001/extensions/common/api/s...
File extensions/common/api/system_display.idl (right):

https://codereview.chromium.org/2072633002/diff/20001/extensions/common/api/s...
extensions/common/api/system_display.idl:205: static void
setDisplayLayout(DisplayLayout[] layouts);
On 2016/06/16 19:08:30, Antony Sargent wrote:
> optional: you might consider adding a callback which reports back whether the
> layouts passed were used successfully or had to be adjusted. Callers can
likely
> figure that out themselves by listening for the onDisplayChanged, but doing so
> might be a bit more cumbersome than if you just told them directly.

I don't want to add too much complexity to the API in anticipation of usage that
will probably never exist, but this should at least call a void callback when
done so that the error state can be queried, same as with setDisplayProperties.
Done.

[stevenjb, 2016-06-17 01:35:09]

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

[commit-bot: I haz the power, 2016-06-17 01:35:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2072633002/40001

[Ilya Sherman, 2016-06-17 01:38:12]

histograms.xml lgtm

[commit-bot: I haz the power, 2016-06-17 01:49:10]

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

[commit-bot: I haz the power, 2016-06-17 01:49:11]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[oshima, 2016-06-20 21:30:33]

lgtm

https://codereview.chromium.org/2072633002/diff/60001/chrome/browser/extensio...
File chrome/browser/extensions/display_info_provider_chromeos.cc (right):

https://codereview.chromium.org/2072633002/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/display_info_provider_chromeos.cc:464: return false;
optinoal: Can this happen? If not, DCHECK or LOG(ERROR) may help troubleshooting
the feedback reports.

[stevenjb, 2016-06-20 23:28:56]

https://codereview.chromium.org/2072633002/diff/60001/chrome/browser/extensio...
File chrome/browser/extensions/display_info_provider_chromeos.cc (right):

https://codereview.chromium.org/2072633002/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/display_info_provider_chromeos.cc:464: return false;
On 2016/06/20 21:30:33, oshima wrote:
> optinoal: Can this happen? If not, DCHECK or LOG(ERROR) may help
troubleshooting
> the feedback reports.

It could if, e.g. the display were unplugged while a layout change was in
progress. I will add a LOG(ERROR). Done.

[stevenjb, 2016-06-20 23:28:59]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2016-06-20 23:29:00]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
asargent@chromium.org, isherman@chromium.org, oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/2072633002/#ps80001
(title: "Add more error logging")

[commit-bot: I haz the power, 2016-06-20 23:29:12]

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

[commit-bot: I haz the power, 2016-06-21 00:05:35]

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

[commit-bot: I haz the power, 2016-06-21 00:05:36]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[stevenjb, 2016-06-21 00:15:26]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2016-06-21 00:15:27]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
oshima@chromium.org, isherman@chromium.org, asargent@chromium.org
Link to the patchset: https://codereview.chromium.org/2072633002/#ps100001
(title: "Fix win")

[commit-bot: I haz the power, 2016-06-21 00:15:51]

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

[commit-bot: I haz the power, 2016-06-21 01:19:09]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-06-21 01:28:40]

Description was changed from

==========
Add Get/SetDisplayLayout to chrome.system.display extension API

This will allow Settings and kiosk apps to get and set display
layout info.

BUG=547080
==========

to

==========
Add Get/SetDisplayLayout to chrome.system.display extension API

This will allow Settings and kiosk apps to get and set display
layout info.

BUG=547080

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

[commit-bot: I haz the power, 2016-06-21 01:28:42]

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