Make using WebContentsUserData simpler.

Make using WebContentsUserData simpler.

BUG=107201
TEST=no visible change


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=159158

[Avi (use Gerrit), 2012-09-27 18:43:28]

WDYT?

This simplifies the .h files by moving the key declaration into the template,
but holy cow is the definition in the .cc file made more complicated...

[Avi (use Gerrit), 2012-09-27 18:45:04]

Look at chrome/browser/common/web_contents_user_data.h and its unit test to see
what I mean.

[sky, 2012-09-27 20:38:15]

The template syntax makes it more obscure, but I like not having the member in
the header.

[Avi (use Gerrit), 2012-09-27 20:41:04]

On 2012/09/27 20:38:15, sky wrote:
> The template syntax makes it more obscure, but I like not having the member in
> the header.

Indeed.

Do you think the careful application of a macro helps here or hurts? (See patch
set 2.)

[sky, 2012-09-27 20:50:37]

I like the macro as it makes it clear where the member is used. It also avoids
making me wonder why no value is set. Nice!

[Avi (use Gerrit), 2012-09-27 21:03:46]

On 2012/09/27 20:50:37, sky wrote:
> I like the macro as it makes it clear where the member is used. It also avoids
> making me wonder why no value is set. Nice!

Thanks. It was Ben's suggestion, actually.

If the template magic looks good, and the macro looks good, can I get an LG?

[sky, 2012-09-27 21:10:00]

LGTM

[commit-bot: I haz the power, 2012-09-27 21:18:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10993064/7001

[commit-bot: I haz the power, 2012-09-27 23:52:44]

Change committed as 159158

[awong, 2012-10-10 17:03:46]

sorry for the late comments.

I like the idea, but the reliance on VC's linker not collapsing the memory
location still makes me uneasy.  Do we have any evidence showing more
definitively that this is just FUD?

https://codereview.chromium.org/10993064/diff/7001/chrome/browser/common/web_...
File chrome/browser/common/web_contents_user_data.h (right):

https://codereview.chromium.org/10993064/diff/7001/chrome/browser/common/web_...
chrome/browser/common/web_contents_user_data.h:46: 
Can this be private?

https://codereview.chromium.org/10993064/diff/7001/chrome/browser/common/web_...
chrome/browser/common/web_contents_user_data.h:53: // will be collapsed by the
Visual Studio linker.
This comment still makes me a bit uneasy.  Since we effectively have an unused
variable, with WPO, what special about having a definition in the .cc instead of
the .h that's keeping the linker from merging everything?

[Avi (use Gerrit), 2012-10-10 18:18:53]

That what is FUD?

The WCUD unittest ensures that it works properly. If you add "const" to the key,
the unittest fails with VS. If you use the constructor as a key, the unittest
fails with VS.

https://codereview.chromium.org/10993064/diff/7001/chrome/browser/common/web_...
File chrome/browser/common/web_contents_user_data.h (right):

https://codereview.chromium.org/10993064/diff/7001/chrome/browser/common/web_...
chrome/browser/common/web_contents_user_data.h:46: 
On 2012/10/10 17:03:46, awong wrote:
> Can this be private?

I don't know. Let me try. I'd be happy if it were.

https://codereview.chromium.org/10993064/diff/7001/chrome/browser/common/web_...
chrome/browser/common/web_contents_user_data.h:53: // will be collapsed by the
Visual Studio linker.
I wish I knew. I'm trying to remember who I had the conversation with who said
that this made it work. This also helps with the component builds to ensure that
it's single.