Use bit-fields for space saving in PointerEventsHitRules

Use bit-fields for space saving in PointerEventsHitRules

Use bit-fields for space saving in PointerEventsHitRules. This makes
the class size shrink to the size of an unsigned.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183233

[Sunil Ratnu, 2014-09-29 14:18:37]

sunil.ratnu@samsung.com changed reviewers:
+ keishi@chromium.org, rob.buis@samsung.com, tkent@chromium.org

[Sunil Ratnu, 2014-09-29 14:18:37]

Hi all,

A pretty simple change. Please review this.

Thanks!
Sunil

[rwlbuis, 2014-09-29 14:22:08]

Hi,

On 2014/09/29 14:18:37, Sunil Ratnu wrote:
> Hi all,
> 
> A pretty simple change. Please review this.

IIRC for this to work with MSVC it has to be unsigned for the bitfields. Also
can you add a COMPILE_ASSERT for the size you expect?

[Sunil Ratnu, 2014-09-29 14:28:08]

On 2014/09/29 14:22:08, rwlbuis wrote:
> Hi,
> 
> On 2014/09/29 14:18:37, Sunil Ratnu wrote:
> > Hi all,
> > 
> > A pretty simple change. Please review this.
> 
> IIRC for this to work with MSVC it has to be unsigned for the bitfields. Also
> can you add a COMPILE_ASSERT for the size you expect?

Thanks for the review Rob.
Grepping the already existing code, I see that there is already code present in
this format
[https://code.google.com/p/chromium/codesearch#search/&q=bool\%20[a-z]*\%20:\%201%20file:third_party/WebKit/&sq=package:chromium&type=cs]
and that works fine and there are no COMPILE_ASSERT for that as well.

Thanks!
Sunil

[Sunil Ratnu, 2014-10-01 07:29:43]

-tkent as he is already overloaded.

Hi Rob,

Could you please have a look? I will add COMPILE_ASSERT if its needed as per
your final take on this?

Thanks!
Sunil

[Sunil Ratnu, 2014-10-01 07:30:54]

sunil.ratnu@samsung.com changed reviewers:
- tkent@chromium.org

[rwlbuis, 2014-10-01 14:39:57]

On 2014/10/01 07:29:43, Sunil Ratnu wrote:
> -tkent as he is already overloaded.
> 
> Hi Rob,
> 
> Could you please have a look? I will add COMPILE_ASSERT if its needed as per
> your final take on this?

Yes you need to add a COMPILE_ASSERT. With that, we can check if there are any
problems on the windows bot.

[Sunil Ratnu, 2014-10-03 08:06:04]

On 2014/10/01 14:39:57, rwlbuis wrote:
> On 2014/10/01 07:29:43, Sunil Ratnu wrote:
> > -tkent as he is already overloaded.
> > 
> > Hi Rob,
> > 
> > Could you please have a look? I will add COMPILE_ASSERT if its needed as per
> > your final take on this?
> 
> Yes you need to add a COMPILE_ASSERT. With that, we can check if there are any
> problems on the windows bot.

Hi Rob,

I've made the changes suggested by you. Also I've made the earlier change you
suggested [s/bool/unsigned]. Please have a look.

Thanks!
Sunil

[rwlbuis, 2014-10-03 18:22:39]

lgtm

[rwlbuis, 2014-10-03 18:23:58]

Your description may need to be updated though. On my system the size went from
6 to 4 bytes.
Maybe you should just say "shrinks to the size of an unsigned" like the
COMPILE_ASSERT indicates.

[rwlbuis, 2014-10-03 18:25:45]

https://codereview.chromium.org/612823004/diff/20001/Source/core/rendering/Po...
File Source/core/rendering/PointerEventsHitRules.cpp (right):

https://codereview.chromium.org/612823004/diff/20001/Source/core/rendering/Po...
Source/core/rendering/PointerEventsHitRules.cpp:23: #include <base/macros.h>
not lgtm

I forgot about this. This does not seem to be the right
include. Is it even needed?

[Sunil Ratnu, 2014-10-04 09:01:50]

Patchset #3 (id:40001) has been deleted

[Sunil Ratnu, 2014-10-04 09:06:17]

Hi Rob,

Thanks for the review. I've corrected the #include now. Please have a look.

Thanks!
Sunil

https://codereview.chromium.org/612823004/diff/20001/Source/core/rendering/Po...
File Source/core/rendering/PointerEventsHitRules.cpp (right):

https://codereview.chromium.org/612823004/diff/20001/Source/core/rendering/Po...
Source/core/rendering/PointerEventsHitRules.cpp:23: #include <base/macros.h>
On 2014/10/03 18:25:45, rwlbuis wrote:
> not lgtm
> 
> I forgot about this. This does not seem to be the right
> include. Is it even needed?

Oops. My mistake. Corrected the include now.

[rwlbuis, 2014-10-04 17:41:29]

The CQ bit was checked by rob.buis@samsung.com

[rwlbuis, 2014-10-04 17:41:29]

lgtm

[commit-bot: I haz the power, 2014-10-04 17:42:26]

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

[commit-bot: I haz the power, 2014-10-04 19:58:33]

Committed patchset #3 (id:60001) as 183233