Simplify TouchAction enum to be a simple bit flag

Simplify TouchAction enum to be a simple bit flag

Originally I used the value 0 for the default touch-action value
(TouchActionAuto).  But as touch-action has expanded, it would be
a lot more convenient if we used a pure bit field (where "auto"
was all flags set, and "none" was 0).  This was hard to change
before the blink merge, but is now pretty easy.

BUG=None

Committed: https://crrev.com/a8b478daf9eef9e4f8e1ba21327c081b0301945e
Cr-Commit-Position: refs/heads/master@{#357111}

[Rick Byers, 2015-10-26 18:33:12]

rbyers@chromium.org changed reviewers:
+ dtapuska@chromium.org

[Rick Byers, 2015-10-26 18:33:13]

Dave, PTAL (no rush)

[dtapuska, 2015-10-29 14:05:58]

On 2015/10/26 18:33:13, Rick Byers wrote:
> Dave, PTAL (no rush)

lgtm; this makes much more sense to me now than the previous way.

[Rick Byers, 2015-10-29 14:07:55]

The CQ bit was checked by rbyers@chromium.org

[commit-bot: I haz the power, 2015-10-29 14:08:26]

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

[commit-bot: I haz the power, 2015-10-29 14:15:56]

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

[commit-bot: I haz the power, 2015-10-29 14:15:56]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Rick Byers, 2015-10-29 15:15:14]

Description was changed from

==========
Simplify TouchAction enum to be a simple bit flag

Originally I used the value 0 for the default touch-action value
(TouchActionAuto).  But as touch-action has expanded, it would be
a lot more convenient if we used a pure bit field (where "auto"
was all flags set, and "none" was 0).  This was hard to change
before the blink merge, but is now pretty easy.

BUG=None
==========

to

==========
Simplify TouchAction enum to be a simple bit flag

Originally I used the value 0 for the default touch-action value
(TouchActionAuto).  But as touch-action has expanded, it would be
a lot more convenient if we used a pure bit field (where "auto"
was all flags set, and "none" was 0).  This was hard to change
before the blink merge, but is now pretty easy.

BUG=None
==========

[Rick Byers, 2015-10-29 15:15:15]

rbyers@chromium.org changed reviewers:
+ johcen@chromium.org

[Rick Byers, 2015-10-29 15:17:42]

rbyers@chromium.org changed reviewers:
+ tdresser@chromium.org

[Rick Byers, 2015-10-29 15:17:42]

Hah, oh right - the repos are merged now so there may be OWNERS I need <grin>.  

+tdresser@ for content/browser/renderer_host/input
+jochen@ for the rest of content OWNERS

[Rick Byers, 2015-10-29 15:29:44]

dtapuska: looks like I missed a couple changes on the blink sides (sorry,
thought I checked the test results but I guess I was thinking of another patch).
 Will upload a couple additional changes shortly and let you know when I have
green tests.

None of that should affect the chromium side though, so still need OWNERS stamps
there.

[tdresser, 2015-10-29 15:30:33]

tdresser@chromium.org changed reviewers:
+ jochen@chromium.org
- johcen@chromium.org

[tdresser, 2015-10-29 15:30:33]

tdresser@chromium.org changed reviewers:
+ jochen@chromium.org
- johcen@chromium.org

[tdresser, 2015-10-29 15:30:34]

content/browser/renderer_host/input/ LGTM.

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
File content/common/input/touch_action.h (right):

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
content/common/input/touch_action.h:37: // All actions are pemitted (the
default).
pemitted -> permitted

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
content/common/input/touch_action.h:45: return TouchAction(int(a) | int(b));
Use static_cast.

[tdresser, 2015-10-29 15:30:35]

content/browser/renderer_host/input/ LGTM.

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
File content/common/input/touch_action.h (right):

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
content/common/input/touch_action.h:37: // All actions are pemitted (the
default).
pemitted -> permitted

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
content/common/input/touch_action.h:45: return TouchAction(int(a) | int(b));
Use static_cast.

[dtapuska, 2015-10-29 15:38:45]

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
File content/common/input/touch_action.h (right):

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
content/common/input/touch_action.h:45: return TouchAction(int(a) | int(b));
On 2015/10/29 15:30:35, tdresser (OOO Oct 26 - 29) wrote:
> Use static_cast.

WebTouchAction also does this; and occurs in a few patterns inside blink as
well.. Guess it should be cleaned up to current standards.

[jdduke (slow), 2015-10-29 15:40:26]

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

[jdduke (slow), 2015-10-29 15:40:26]

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
File content/common/input/touch_action.h (right):

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
content/common/input/touch_action.h:13: enum TouchAction {
Now that we expose proper operators, should this be

enum class TouchAction : int

?

[jdduke (slow), 2015-10-29 15:41:44]

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
File content/common/input/touch_action.h (right):

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
content/common/input/touch_action.h:13: enum TouchAction {
On 2015/10/29 15:40:26, jdduke (slow) wrote:
> Now that we expose proper operators, should this be
> 
> enum class TouchAction : int
> 
> ?

Actually, not sure if that works if the operators are used in the enum
definition.

[tdresser, 2015-10-29 15:43:00]

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
File content/common/input/touch_action.h (right):

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
content/common/input/touch_action.h:13: enum TouchAction {
On 2015/10/29 15:40:26, jdduke (slow) wrote:
> Now that we expose proper operators, should this be
> 
> enum class TouchAction : int
> 
> ?

It would require renaming everything, but it's likely worth it.

IE, TOUCH_ACTION_NONE -> TouchAction::NONE.

[Rick Byers, 2015-10-29 16:50:36]

On 2015/10/29 15:43:00, tdresser wrote:
>
https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
> File content/common/input/touch_action.h (right):
> 
>
https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
> content/common/input/touch_action.h:13: enum TouchAction {
> On 2015/10/29 15:40:26, jdduke (slow) wrote:
> > Now that we expose proper operators, should this be
> > 
> > enum class TouchAction : int
> > 
> > ?
> 
> It would require renaming everything, but it's likely worth it.
> 
> IE, TOUCH_ACTION_NONE -> TouchAction::NONE.

When I started this CL I did a quick bit of research and I thought enum classes
were inappropriate for flags (they're "strongly typed" afterall - not supposed
to have values that aren't one of their members).  Is that not actually the
case?

[tdresser, 2015-10-29 16:53:08]

On 2015/10/29 16:50:36, Rick Byers wrote:
> On 2015/10/29 15:43:00, tdresser wrote:
> >
>
https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
> > File content/common/input/touch_action.h (right):
> > 
> >
>
https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
> > content/common/input/touch_action.h:13: enum TouchAction {
> > On 2015/10/29 15:40:26, jdduke (slow) wrote:
> > > Now that we expose proper operators, should this be
> > > 
> > > enum class TouchAction : int
> > > 
> > > ?
> > 
> > It would require renaming everything, but it's likely worth it.
> > 
> > IE, TOUCH_ACTION_NONE -> TouchAction::NONE.
> 
> When I started this CL I did a quick bit of research and I thought enum
classes
> were inappropriate for flags (they're "strongly typed" afterall - not supposed
> to have values that aren't one of their members).  Is that not actually the
> case?

You're right - enum classes aren't appropriate here.

[Rick Byers, 2015-10-29 16:56:04]

Ok Dave, I've fixed the layout tests - PTAL at the diff between PS1 and PS2
(there's no rebase).

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
File content/common/input/touch_action.h (right):

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
content/common/input/touch_action.h:37: // All actions are pemitted (the
default).
On 2015/10/29 15:30:35, tdresser wrote:
> pemitted -> permitted

Done.

https://codereview.chromium.org/1422513006/diff/20001/content/common/input/to...
content/common/input/touch_action.h:45: return TouchAction(int(a) | int(b));
On 2015/10/29 15:38:44, dtapuska wrote:
> On 2015/10/29 15:30:35, tdresser (OOO Oct 26 - 29) wrote:
> > Use static_cast.
> 
> WebTouchAction also does this; and occurs in a few patterns inside blink as
> well.. Guess it should be cleaned up to current standards.

I think I've found them all and switched to static_cast.  This CL also let me
eliminate some bit manipulation (eg. now have a defined constant for
Manipulation, tests can be simpler).

[jochen (gone - plz use gerrit), 2015-10-30 11:57:58]

lgtm

[Rick Byers, 2015-10-30 14:15:27]

The CQ bit was checked by rbyers@chromium.org

[Rick Byers, 2015-10-30 14:15:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/1422513006/#ps40001
(title: "Fix computed style / layout tests and CR feedback")

[commit-bot: I haz the power, 2015-10-30 14:15:57]

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

[commit-bot: I haz the power, 2015-10-30 15:49:17]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-30 15:50:15]

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