Migrate WaitableEvent to enum-based constructor in ui/

Migrate WaitableEvent to enum-based constructor in ui/

Change automated with clang-tidy (details @ https://crbug.com/612843#c13)

BUG=612843
Committed: https://crrev.com/a0c6af14179a2ab33286a941997162b0d67f7e37
Cr-Commit-Position: refs/heads/master@{#397239}

[gab, 2016-06-01 19:00:08]

Description was changed from

==========
Migrate WaitableEvent to enum-based constructor in ui/

BUG=612843
R=sky@chromium.org
==========

to

==========
Migrate WaitableEvent to enum-based constructor in ui/

Change automated with clang-tidy (details @ https://crbug.com/612843#c13)

BUG=612843
==========

[sky, 2016-06-01 19:18:36]

https://codereview.chromium.org/2027323002/diff/1/ui/views/mus/views_mus_test...
File ui/views/mus/views_mus_test_suite.cc (right):

https://codereview.chromium.org/2027323002/diff/1/ui/views/mus/views_mus_test...
ui/views/mus/views_mus_test_suite.cc:70: base::WaitableEvent
wait(base::WaitableEvent::ResetPolicy::AUTOMATIC,
Did you consider making WaitableEvent take a bitmask of possible configurations
rather than two explicit policies? I suspect automatic and not-signaled are most
commonly used, so this code would become:

base::WaitableEvent event(base::WaitableEvent::DEFAULT);

Even better is to with a default value (which the style guide now allows) for
DEFAULT, so that this code becomes:

base::WaitableEvent event;

If you want to create manual, then it's:

event(base::WaitableEvent::MANUAL);

and if you want manual and signaled:

event(base::WaitableEvent::MANUAL | base::WaitableEvent::SIGNALED);

IMO this better matches how WaitableEvent is generally used and makes the api
nicer.

[gab, 2016-06-01 19:50:30]

https://codereview.chromium.org/2027323002/diff/1/ui/views/mus/views_mus_test...
File ui/views/mus/views_mus_test_suite.cc (right):

https://codereview.chromium.org/2027323002/diff/1/ui/views/mus/views_mus_test...
ui/views/mus/views_mus_test_suite.cc:70: base::WaitableEvent
wait(base::WaitableEvent::ResetPolicy::AUTOMATIC,
On 2016/06/01 19:18:35, sky wrote:
> Did you consider making WaitableEvent take a bitmask of possible
configurations
> rather than two explicit policies? I suspect automatic and not-signaled are
most
> commonly used, so this code would become:
> 
> base::WaitableEvent event(base::WaitableEvent::DEFAULT);
> 
> Even better is to with a default value (which the style guide now allows) for
> DEFAULT, so that this code becomes:
> 
> base::WaitableEvent event;
> 
> If you want to create manual, then it's:
> 
> event(base::WaitableEvent::MANUAL);
> 
> and if you want manual and signaled:
> 
> event(base::WaitableEvent::MANUAL | base::WaitableEvent::SIGNALED);
> 
> IMO this better matches how WaitableEvent is generally used and makes the api
> nicer.

Copied your suggestion on bug, danakj prefers the current API, please discuss it
there if you disagree.

[sky, 2016-06-01 20:56:18]

LGTM

[gab, 2016-06-01 20:57:24]

The CQ bit was checked by gab@chromium.org

[commit-bot: I haz the power, 2016-06-01 20:57:44]

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

[commit-bot: I haz the power, 2016-06-01 21:03:49]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-06-01 21:05:41]

Description was changed from

==========
Migrate WaitableEvent to enum-based constructor in ui/

Change automated with clang-tidy (details @ https://crbug.com/612843#c13)

BUG=612843
==========

to

==========
Migrate WaitableEvent to enum-based constructor in ui/

Change automated with clang-tidy (details @ https://crbug.com/612843#c13)

BUG=612843

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

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

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