Switch CoreTabHelper to use WebContentsUserData.

Switch CoreTabHelper to use WebContentsUserData.

BUG=107201
TEST=no visible change


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

[Avi (use Gerrit), 2012-09-14 23:19:50]

sky: overall, and you're an instant/ owner
cbentzel: you own prerender/

[sky, 2012-09-14 23:36:00]

LGTM

https://codereview.chromium.org/10905301/diff/1/chrome/browser/ui/tab_content...
File chrome/browser/ui/tab_contents/core_tab_helper.cc (right):

https://codereview.chromium.org/10905301/diff/1/chrome/browser/ui/tab_content...
chrome/browser/ui/tab_contents/core_tab_helper.cc:18: int
CoreTabHelper::kUserDataKey;
// static

[gavinp, 2012-09-14 23:42:20]

https://codereview.chromium.org/10905301/diff/1/chrome/browser/prerender/prer...
File chrome/browser/prerender/prerender_manager.cc (right):

https://codereview.chromium.org/10905301/diff/1/chrome/browser/prerender/prer...
chrome/browser/prerender/prerender_manager.cc:401: 
Why create all these aliases, including a rename of a parameter, just to add one
call to CoreTabHelper::FromWebContents? I think the call would be shorter if you
just narrowed your changes to the one line?

https://codereview.chromium.org/10905301/diff/1/chrome/browser/prerender/prer...
chrome/browser/prerender/prerender_manager.cc:407:
CoreTabHelper::FromWebContents(old_web_contents)->delegate()->
i.e. change only this line.

[Avi (use Gerrit), 2012-09-17 16:05:12]

https://codereview.chromium.org/10905301/diff/1/chrome/browser/prerender/prer...
File chrome/browser/prerender/prerender_manager.cc (right):

https://codereview.chromium.org/10905301/diff/1/chrome/browser/prerender/prer...
chrome/browser/prerender/prerender_manager.cc:401: 
On 2012/09/14 23:42:21, gavinp wrote:
> Why create all these aliases?

I'm in the process of killing TabContents, so this is a bit more of a
forward-looking patch. I'm introducing WebContents versions of the data,
switching over to them where I can and only using the TabContents versions where
I must. Once I've switched enough things over to WebContents, the TabContents
versions go away.

[cbentzel, 2012-09-17 17:56:06]

Removing myself, Gavin is in the process of the prerender review.

[gavinp, 2012-09-19 04:03:51]

Apologies for being pokey responding Avi.

I'm pushing back just mildly: I certainly lack the perspective to know the whole
context of what you're doing, and I don't want to create a big pile of work for
you. At the same time, I'd like to make sure prerender_manager.cc is readable
the whole time.

http://codereview.chromium.org/10905301/diff/1/chrome/browser/prerender/prere...
File chrome/browser/prerender/prerender_manager.cc (right):

http://codereview.chromium.org/10905301/diff/1/chrome/browser/prerender/prere...
chrome/browser/prerender/prerender_manager.cc:401: 
On 2012/09/17 16:05:12, Avi wrote:
> On 2012/09/14 23:42:21, gavinp wrote:
> > Why create all these aliases?
> 
> I'm in the process of killing TabContents, so this is a bit more of a
> forward-looking patch. I'm introducing WebContents versions of the data,
> switching over to them where I can and only using the TabContents versions
where
> I must. Once I've switched enough things over to WebContents, the TabContents
> versions go away.

I'd rather review that change when it happens, I think. Even if the intermediate
state is temporary, I'd rather not see all these extra nearly synonymic
variables, even briefly.

Would it make sense to minimize to the one line for this commit, and then do the
bigger rename, removing the tab contents too, when the new webcontents interface
is available?

[gavinp, 2012-09-19 10:47:40]

Lgtm, consider my earlier comments nits if you're cleaning this up later.

On 2012/09/19 04:03:51, gavinp wrote:
> Apologies for being pokey responding Avi.
> 
> I'm pushing back just mildly: I certainly lack the perspective to know the
whole
> context of what you're doing, and I don't want to create a big pile of work
for
> you. At the same time, I'd like to make sure prerender_manager.cc is readable
> the whole time.
> 
>
http://codereview.chromium.org/10905301/diff/1/chrome/browser/prerender/prere...
> File chrome/browser/prerender/prerender_manager.cc (right):
> 
>
http://codereview.chromium.org/10905301/diff/1/chrome/browser/prerender/prere...
> chrome/browser/prerender/prerender_manager.cc:401: 
> On 2012/09/17 16:05:12, Avi wrote:
> > On 2012/09/14 23:42:21, gavinp wrote:
> > > Why create all these aliases?
> > 
> > I'm in the process of killing TabContents, so this is a bit more of a
> > forward-looking patch. I'm introducing WebContents versions of the data,
> > switching over to them where I can and only using the TabContents versions
> where
> > I must. Once I've switched enough things over to WebContents, the
TabContents
> > versions go away.
> 
> I'd rather review that change when it happens, I think. Even if the
intermediate
> state is temporary, I'd rather not see all these extra nearly synonymic
> variables, even briefly.
> 
> Would it make sense to minimize to the one line for this commit, and then do
the
> bigger rename, removing the tab contents too, when the new webcontents
interface
> is available?

[Avi (use Gerrit), 2012-09-19 15:47:03]

I'm undoing all the other changes, and restricting my changes to that one line
as you noted.

[commit-bot: I haz the power, 2012-09-19 16:10:45]

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

[commit-bot: I haz the power, 2012-09-19 17:44:54]

Retried try job too often for step(s) content_unittests

[commit-bot: I haz the power, 2012-09-19 19:36:03]

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

[commit-bot: I haz the power, 2012-09-19 22:23:00]

Change committed as 157629