Unuse bitfields for ElementRareData::m_proxyCount.

Unuse bitfields for ElementRareData::m_proxyCount.

This bitfields usage was introduced at
https://codereview.chromium.org/1064123002/
but now m_tabStop went away, it no longer has to be
a bitfield.

In addition, this caused Dr.Memory to detect
false positive as 'uninitialized read'.

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

[kochi, 2015-06-10 06:16:30]

kochi@chromium.org changed reviewers:
+ sadrul@chromium.org

[kochi, 2015-06-10 06:17:11]

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

[commit-bot: I haz the power, 2015-06-10 06:17:24]

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

[kochi, 2015-06-10 06:18:05]

Sadrul,

Could you take a look?

I think it should never be a problem to extend the
field to use full 16bits, it doesn't change the size
of ElementRareData.

[commit-bot: I haz the power, 2015-06-10 07:23:28]

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

[commit-bot: I haz the power, 2015-06-10 07:23:29]

Dry run: This issue passed the CQ dry run.

[sadrul, 2015-06-10 16:15:49]

sadrul@chromium.org changed reviewers:
+ esprehn@chromium.org

[sadrul, 2015-06-10 16:15:49]

+esprehn@

Looks good. Although maybe add a comment to make sure newer fields share
bitfields if possible, and that it is okay to cut down the proxy-count size?

[kochi, 2015-06-10 23:09:39]

On 2015/06/10 16:15:49, sadrul wrote:
> +esprehn@
> 
> Looks good. Although maybe add a comment to make sure newer fields share
> bitfields if possible, and that it is okay to cut down the proxy-count size?

How many bits can be cut down?
I assume 10 is big enough for proxy counts, but could it be smaller?

[esprehn, 2015-06-11 00:34:11]

On 2015/06/10 at 23:09:39, kochi wrote:
> On 2015/06/10 16:15:49, sadrul wrote:
> > +esprehn@
> > 
> > Looks good. Although maybe add a comment to make sure newer fields share
> > bitfields if possible, and that it is okay to cut down the proxy-count size?
> 
> How many bits can be cut down?
> I assume 10 is big enough for proxy counts, but could it be smaller?

That's 1024, it's a good question. We probably need to make the proxies fail on
creation if you hit that limit. I'm not sure what a reasonable number of proxies
is for an app, I bet it's a lot smaller than 1024 though.

[kochi, 2015-06-11 00:53:13]

On 2015/06/11 00:34:11, esprehn wrote:
> On 2015/06/10 at 23:09:39, kochi wrote:
> > On 2015/06/10 16:15:49, sadrul wrote:
> > > +esprehn@
> > > 
> > > Looks good. Although maybe add a comment to make sure newer fields share
> > > bitfields if possible, and that it is okay to cut down the proxy-count
size?
> > 
> > How many bits can be cut down?
> > I assume 10 is big enough for proxy counts, but could it be smaller?
> 
> That's 1024, it's a good question. We probably need to make the proxies fail
on
> creation if you hit that limit. I'm not sure what a reasonable number of
proxies
> is for an app, I bet it's a lot smaller than 1024 though.

Okay, I think if we change it to a bitfield like m_proxyCount : 10; then the
compiler
can take care of the overflow not to corrupt the neighbour bit, but then once
overflow
happens m_proxyCount suddenly goes 0, and user may see some funny bugs that is
hard to track down.

Anyway, 10bits was the number that m_proxyCounts used to be, so the potential
doesn't change.

Does the added comment look good?

[kochi, 2015-06-12 00:52:26]

Ping?

[esprehn, 2015-06-12 01:05:22]

lgtm

[esprehn, 2015-06-12 01:05:59]

This really seems like a bug in Dr. Memory though, having a bitfield and the
compiler inserting padding shouldn't generate an error.

[kochi, 2015-06-12 01:07:30]

The CQ bit was checked by kochi@chromium.org

[commit-bot: I haz the power, 2015-06-12 01:07:47]

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

[kochi, 2015-06-12 01:16:52]

On 2015/06/12 01:05:59, esprehn wrote:
> This really seems like a bug in Dr. Memory though, having a bitfield and the
> compiler inserting padding shouldn't generate an error.

FYI filed a bug there: https://github.com/DynamoRIO/drmemory/issues/1731

[commit-bot: I haz the power, 2015-06-12 02:26:02]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196989