Set material hybrid by default on ChromeOS touch devices

Set material hybrid by default on ChromeOS touch devices

If a touchscreen is present, use the hybrid
variant of top chrome material design by default
on ChromeOS.

BUG=588880
TEST=manual

Committed: https://crrev.com/0b73cc0dda9b1f57478838fcc34cdeb1f5d6a752
Cr-Commit-Position: refs/heads/master@{#379569}

[tdanderson, 2016-02-22 23:47:40]

tdanderson@chromium.org changed reviewers:
+ bruthig@chromium.org

[tdanderson, 2016-02-22 23:47:40]

Hey Ben, here is a non-functioning CL that I'd like to chat with you about in
person tomorrow.

https://codereview.chromium.org/1720273002/diff/1/ui/base/material_design/mat...
File ui/base/material_design/material_design_controller.cc (right):

https://codereview.chromium.org/1720273002/diff/1/ui/base/material_design/mat...
ui/base/material_design/material_design_controller.cc:34: if
(GetTouchScreensAvailability() == TouchScreensAvailability::ENABLED)
Doesn't look like this will work as-is, since the resource bundle is initialized
(and hence IsModeMaterial() is called) before the device is registered in
DeviceDataManager.

[tdanderson, 2016-03-02 21:46:08]

tdanderson@chromium.org changed reviewers:
+ adlr@chromium.org

[tdanderson, 2016-03-02 21:46:09]

adlr@, can you please take a look and let me know if I have understood your
suggestion correctly?

[vapier, 2016-03-02 21:51:54]

vapier@chromium.org changed reviewers:
+ vapier@chromium.org

[vapier, 2016-03-02 21:51:54]

https://codereview.chromium.org/1720273002/diff/40001/ui/base/material_design...
File ui/base/material_design/material_design_controller.cc (right):

https://codereview.chromium.org/1720273002/diff/40001/ui/base/material_design...
ui/base/material_design/material_design_controller.cc:91: int fd =
open(path.value().c_str(), O_RDWR | O_NONBLOCK);
you should use O_CLOEXEC all the time unless you have a good reason not to

https://codereview.chromium.org/1720273002/diff/40001/ui/base/material_design...
ui/base/material_design/material_design_controller.cc:92: if (fd >= 0 &&
devinfo.Initialize(fd, path) && devinfo.HasTouchscreen())
doesn't this leak fd ?

[adlr1, 2016-03-03 19:24:49]

adlr@google.com changed reviewers:
+ adlr@google.com

[adlr1, 2016-03-03 19:24:50]

I'm happy as long as vapier's comments are addressed

[tdanderson, 2016-03-04 19:45:53]

vapier@ and adlr@, can you please take another look?

https://codereview.chromium.org/1720273002/diff/40001/ui/base/material_design...
File ui/base/material_design/material_design_controller.cc (right):

https://codereview.chromium.org/1720273002/diff/40001/ui/base/material_design...
ui/base/material_design/material_design_controller.cc:91: int fd =
open(path.value().c_str(), O_RDWR | O_NONBLOCK);
On 2016/03/02 21:51:54, vapier wrote:
> you should use O_CLOEXEC all the time unless you have a good reason not to

Done.

https://codereview.chromium.org/1720273002/diff/40001/ui/base/material_design...
ui/base/material_design/material_design_controller.cc:92: if (fd >= 0 &&
devinfo.Initialize(fd, path) && devinfo.HasTouchscreen())
On 2016/03/02 21:51:54, vapier wrote:
> doesn't this leak fd ?

Done.

[vapier, 2016-03-04 19:50:20]

looks OK to me

[tdanderson, 2016-03-04 19:57:35]

tdanderson@chromium.org changed reviewers:
+ sky@chromium.org

[tdanderson, 2016-03-04 19:57:36]

Hi Scott, can you please take a look?

[sky, 2016-03-04 23:02:59]

LGTM

[adlr1, 2016-03-06 03:54:20]

lgtm

[tdanderson, 2016-03-07 15:46:48]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2016-03-07 15:47:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1720273002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1720273002/120001

[commit-bot: I haz the power, 2016-03-07 17:19:39]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-03-07 17:20:54]

Description was changed from

==========
Set material hybrid by default on ChromeOS touch devices

If a touchscreen is present, use the hybrid
variant of top chrome material design by default
on ChromeOS.

BUG=588880
TEST=manual
==========

to

==========
Set material hybrid by default on ChromeOS touch devices

If a touchscreen is present, use the hybrid
variant of top chrome material design by default
on ChromeOS.

BUG=588880
TEST=manual

Committed: https://crrev.com/0b73cc0dda9b1f57478838fcc34cdeb1f5d6a752
Cr-Commit-Position: refs/heads/master@{#379569}
==========

[commit-bot: I haz the power, 2016-03-07 17:20:55]

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