Move both instances of address_of into stl_util.

Move both instances of address_of into stl_util.

The PrerenderManager ended up wanting address_of, and rather than introduce
a third instance of this relatively common pattern, I moved it into stl_util.h.

BUG=None

[gavinp, 2012-04-29 00:44:45]

Instead of adding a third instance of this template, I wrote this CL.

Does it belong in base:: ?

[dhollowa, 2012-04-30 16:03:09]

>Does it belong in base:: ?

It depends, where will your code be using it?  If your use is in chrome, then
chrome/common/stl_util.h might be more appropriate.  If it is in autofill code,
then chrome/browser/autofill/stl_util.h is best.  I.e. the principle of least
scope...

https://chromiumcodereview.appspot.com/10261004/diff/1/base/stl_util.h
File base/stl_util.h (right):

https://chromiumcodereview.appspot.com/10261004/diff/1/base/stl_util.h#newcode14
base/stl_util.h:14: // Find the address of an object as a unary functor.
s/Find/Return/

https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofil...
File chrome/browser/autofill/personal_data_manager.cc (left):

https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofil...
chrome/browser/autofill/personal_data_manager.cc:63: class DereferenceFunctor {
While we're at it, this |DereferenceFunctor| can be removed.  It is no longer
used.

https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata...
File chrome/browser/webdata/autofill_table.cc (left):

https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata...
chrome/browser/webdata/autofill_table.cc:39: // TODO(dhollowa): Find a common
place for this.  It is duplicated in
The TODO should be removed as well.

[gavinp, 2012-04-30 16:27:16]

Thanks for your review, David.

You may be right about moving it into chrome/common; my consumer is in
chrome/browser/prerender/, so that may make sense.  OTOH this code fits  in with
what's in that header already; so I feel pressure from a principle of putting
similar code together.

willchan, how do you balance these competing interests?   You're the base/
reviewer....?

https://chromiumcodereview.appspot.com/10261004/diff/1/base/stl_util.h
File base/stl_util.h (right):

https://chromiumcodereview.appspot.com/10261004/diff/1/base/stl_util.h#newcode14
base/stl_util.h:14: // Find the address of an object as a unary functor.
On 2012/04/30 16:03:09, dhollowa wrote:
> s/Find/Return/

Done.

https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofil...
File chrome/browser/autofill/personal_data_manager.cc (left):

https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofil...
chrome/browser/autofill/personal_data_manager.cc:63: class DereferenceFunctor {
On 2012/04/30 16:03:09, dhollowa wrote:
> While we're at it, this |DereferenceFunctor| can be removed.  It is no longer
> used.

Done.

https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata...
File chrome/browser/webdata/autofill_table.cc (left):

https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata...
chrome/browser/webdata/autofill_table.cc:39: // TODO(dhollowa): Find a common
place for this.  It is duplicated in
On 2012/04/30 16:03:09, dhollowa wrote:
> The TODO should be removed as well.

Doh.  That was left in due to rebasing slippery fingers.  Fixed.

[willchan no longer on Chromium, 2012-04-30 18:09:34]

It's not clear why this functor is even necessary, since it's not being
used to work around operator& (which we disallow in general in our
codebase). At a glance, I don't see why
AutofillProfile::AdjustInferredProfiles() relies on it.

On the other hand, C++11 is going to provide a std::addressof() anyway, so
if we decide we want to keep address_of() around, I'm fine with renaming it
to base::addressof() and putting it in stl_util.h. That said, I'd rather us
just not use it since it doesn't seem necessary. I'd like to hear what you
guys think about this.

On Mon, Apr 30, 2012 at 9:27 AM, <gavinp@chromium.org> wrote:

> Thanks for your review, David.
>
> You may be right about moving it into chrome/common; my consumer is in
> chrome/browser/prerender/, so that may make sense.  OTOH this code fits
>  in with
> what's in that header already; so I feel pressure from a principle of
> putting
> similar code together.
>
> willchan, how do you balance these competing interests?   You're the base/
> reviewer....?
>
>
>
>
https://chromiumcodereview.**appspot.com/10261004/diff/1/**base/stl_util.h<ht...
> File base/stl_util.h (right):
>
> https://chromiumcodereview.**appspot.com/10261004/diff/1/**
>
base/stl_util.h#newcode14<https://chromiumcodereview.appspot.com/10261004/diff/1/base/stl_util.h#newcode14>
> base/stl_util.h:14: // Find the address of an object as a unary functor.
> On 2012/04/30 16:03:09, dhollowa wrote:
>
>> s/Find/Return/
>>
>
> Done.
>
>
> https://chromiumcodereview.**appspot.com/10261004/diff/1/**
>
chrome/browser/autofill/**personal_data_manager.cc<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofill/personal_data_manager.cc>
> File chrome/browser/autofill/**personal_data_manager.cc (left):
>
> https://chromiumcodereview.**appspot.com/10261004/diff/1/**
>
chrome/browser/autofill/**personal_data_manager.cc#**oldcode63<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofill/personal_data_manager.cc#oldcode63>
> chrome/browser/autofill/**personal_data_manager.cc:63: class
> DereferenceFunctor {
> On 2012/04/30 16:03:09, dhollowa wrote:
>
>> While we're at it, this |DereferenceFunctor| can be removed.  It is no
>>
> longer
>
>> used.
>>
>
> Done.
>
>
> https://chromiumcodereview.**appspot.com/10261004/diff/1/**
>
chrome/browser/webdata/**autofill_table.cc<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata/autofill_table.cc>
> File chrome/browser/webdata/**autofill_table.cc (left):
>
> https://chromiumcodereview.**appspot.com/10261004/diff/1/**
>
chrome/browser/webdata/**autofill_table.cc#oldcode39<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata/autofill_table.cc#oldcode39>
> chrome/browser/webdata/**autofill_table.cc:39: // TODO(dhollowa): Find a
> common place for this.  It is duplicated in
> On 2012/04/30 16:03:09, dhollowa wrote:
>
>> The TODO should be removed as well.
>>
>
> Doh.  That was left in due to rebasing slippery fingers.  Fixed.
>
>
https://chromiumcodereview.**appspot.com/10261004/<https://chromiumcodereview...
>

[dhollowa, 2012-04-30 18:29:35]

On 2012/04/30 18:09:34, willchan wrote:
> It's not clear why this functor is even necessary, since it's not being
> used to work around operator& (which we disallow in general in our
> codebase). At a glance, I don't see why
> AutofillProfile::AdjustInferredProfiles() relies on it.
> 

AutofillProfile::AdjustInferredLabels does *not* itself rely on |address_of|. 
However, it takes a vector of pointers as its argument.  The caller holds a
vector of objects (not pointers) so that's why this conversion is applied:
namely to convert a vector of objects to a vector of pointers to those same
objects.

This code has always seemed clumsy to me, but there is valid use of address_of
given the existing interfaces.


> On the other hand, C++11 is going to provide a std::addressof() anyway, so
> if we decide we want to keep address_of() around, I'm fine with renaming it
> to base::addressof() and putting it in stl_util.h. That said, I'd rather us
> just not use it since it doesn't seem necessary. I'd like to hear what you
> guys think about this.
> 
> On Mon, Apr 30, 2012 at 9:27 AM, <mailto:gavinp@chromium.org> wrote:
> 
> > Thanks for your review, David.
> >
> > You may be right about moving it into chrome/common; my consumer is in
> > chrome/browser/prerender/, so that may make sense.  OTOH this code fits
> >  in with
> > what's in that header already; so I feel pressure from a principle of
> > putting
> > similar code together.
> >
> > willchan, how do you balance these competing interests?   You're the base/
> > reviewer....?
> >
> >
> >
> >
>
https://chromiumcodereview.**appspot.com/10261004/diff/1/**base/stl_util.h%3C...>
> > File base/stl_util.h (right):
> >
> > https://chromiumcodereview.**appspot.com/10261004/diff/1/**
> >
>
base/stl_util.h#newcode14<https://chromiumcodereview.appspot.com/10261004/diff/1/base/stl_util.h#newcode14>
> > base/stl_util.h:14: // Find the address of an object as a unary functor.
> > On 2012/04/30 16:03:09, dhollowa wrote:
> >
> >> s/Find/Return/
> >>
> >
> > Done.
> >
> >
> > https://chromiumcodereview.**appspot.com/10261004/diff/1/**
> >
>
chrome/browser/autofill/**personal_data_manager.cc<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofill/personal_data_manager.cc>
> > File chrome/browser/autofill/**personal_data_manager.cc (left):
> >
> > https://chromiumcodereview.**appspot.com/10261004/diff/1/**
> >
>
chrome/browser/autofill/**personal_data_manager.cc#**oldcode63<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofill/personal_data_manager.cc#oldcode63>
> > chrome/browser/autofill/**personal_data_manager.cc:63: class
> > DereferenceFunctor {
> > On 2012/04/30 16:03:09, dhollowa wrote:
> >
> >> While we're at it, this |DereferenceFunctor| can be removed.  It is no
> >>
> > longer
> >
> >> used.
> >>
> >
> > Done.
> >
> >
> > https://chromiumcodereview.**appspot.com/10261004/diff/1/**
> >
>
chrome/browser/webdata/**autofill_table.cc<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata/autofill_table.cc>
> > File chrome/browser/webdata/**autofill_table.cc (left):
> >
> > https://chromiumcodereview.**appspot.com/10261004/diff/1/**
> >
>
chrome/browser/webdata/**autofill_table.cc#oldcode39<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata/autofill_table.cc#oldcode39>
> > chrome/browser/webdata/**autofill_table.cc:39: // TODO(dhollowa): Find a
> > common place for this.  It is duplicated in
> > On 2012/04/30 16:03:09, dhollowa wrote:
> >
> >> The TODO should be removed as well.
> >>
> >
> > Doh.  That was left in due to rebasing slippery fingers.  Fixed.
> >
> >
>
https://chromiumcodereview.**appspot.com/10261004/%3Chttps://chromiumcoderevi...>
> >

[willchan no longer on Chromium, 2012-04-30 18:39:21]

On Mon, Apr 30, 2012 at 11:29 AM, <dhollowa@chromium.org> wrote:

> On 2012/04/30 18:09:34, willchan wrote:
>
>> It's not clear why this functor is even necessary, since it's not being
>> used to work around operator& (which we disallow in general in our
>> codebase). At a glance, I don't see why
>> AutofillProfile::**AdjustInferredProfiles() relies on it.
>>
>
>
> AutofillProfile::**AdjustInferredLabels does *not* itself rely on
> |address_of|.
> However, it takes a vector of pointers as its argument.  The caller holds a
> vector of objects (not pointers) so that's why this conversion is applied:
> namely to convert a vector of objects to a vector of pointers to those same
> objects.
>
> This code has always seemed clumsy to me, but there is valid use of
> address_of
> given the existing interfaces.


Yes, it's valid given the existing interfaces. Why don't we change the
interfaces?
http://code.google.com/p/chromium/source/search?q=AdjustInferredLabels&origq=...
makes
it look like there's only 1 real consumer of that function, and
http://code.google.com/p/chromium/source/search?q=PersonalDataManager%3A%3AMe...
similarly
only has one consumer.

I guess what I'd like to see is what *good* pattern is address_of() trying
to make simpler. If we agree the existing uses are clumsy, then I'd prefer
not to validate it by promoting the code to base/.


>
>
>
>  On the other hand, C++11 is going to provide a std::addressof() anyway, so
>> if we decide we want to keep address_of() around, I'm fine with renaming
>> it
>> to base::addressof() and putting it in stl_util.h. That said, I'd rather
>> us
>> just not use it since it doesn't seem necessary. I'd like to hear what you
>> guys think about this.
>>
>
>  On Mon, Apr 30, 2012 at 9:27 AM, <mailto:gavinp@chromium.org> wrote:
>>
>
>  > Thanks for your review, David.
>> >
>> > You may be right about moving it into chrome/common; my consumer is in
>> > chrome/browser/prerender/, so that may make sense.  OTOH this code fits
>> >  in with
>> > what's in that header already; so I feel pressure from a principle of
>> > putting
>> > similar code together.
>> >
>> > willchan, how do you balance these competing interests?   You're the
>> base/
>> > reviewer....?
>> >
>> >
>> >
>> >
>>
>
> https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/****
> base/stl_util.h%3Chttps://**chromiumcodereview.appspot.**
>
com/10261004/diff/1/base/stl_**util.h<http://appspot.com/10261004/diff/1/**base/stl_util.h%3Chttps://chromiumcodereview.appspot.com/10261004/diff/1/base/stl_util.h>
> >
>
>> > File base/stl_util.h (right):
>> >
>> >
https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/**<http://appspot....
>> >
>>
>
> base/stl_util.h#newcode14<http**s://chromiumcodereview.**
>
appspot.com/10261004/diff/1/**base/stl_util.h#newcode14<https://chromiumcodereview.appspot.com/10261004/diff/1/base/stl_util.h#newcode14>
> >
>
>> > base/stl_util.h:14: // Find the address of an object as a unary functor.
>> > On 2012/04/30 16:03:09, dhollowa wrote:
>> >
>> >> s/Find/Return/
>> >>
>> >
>> > Done.
>> >
>> >
>> >
https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/**<http://appspot....
>> >
>>
>
> chrome/browser/autofill/****personal_data_manager.cc<https**
> ://chromiumcodereview.appspot.**com/10261004/diff/1/chrome/**
>
browser/autofill/personal_**data_manager.cc<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofill/personal_data_manager.cc>
> >
>
>> > File chrome/browser/autofill/****personal_data_manager.cc (left):
>> >
>> >
https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/**<http://appspot....
>> >
>>
>
> chrome/browser/autofill/****personal_data_manager.cc#****oldcode63<
> https://**chromiumcodereview.appspot.**com/10261004/diff/1/chrome/**
>
browser/autofill/personal_**data_manager.cc#oldcode63<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofill/personal_data_manager.cc#oldcode63>
> >
>
>> > chrome/browser/autofill/****personal_data_manager.cc:63: class
>>
>> > DereferenceFunctor {
>> > On 2012/04/30 16:03:09, dhollowa wrote:
>> >
>> >> While we're at it, this |DereferenceFunctor| can be removed.  It is no
>> >>
>> > longer
>> >
>> >> used.
>> >>
>> >
>> > Done.
>> >
>> >
>> >
https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/**<http://appspot....
>> >
>>
>
> chrome/browser/webdata/****autofill_table.cc<https://**
> chromiumcodereview.appspot.**com/10261004/diff/1/chrome/**
>
browser/webdata/autofill_**table.cc<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata/autofill_table.cc>
> >
>
>> > File chrome/browser/webdata/****autofill_table.cc (left):
>> >
>> >
https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/**<http://appspot....
>> >
>>
>
> chrome/browser/webdata/****autofill_table.cc#oldcode39<ht**
> tps://chromiumcodereview.**appspot.com/10261004/diff/1/**
>
chrome/browser/webdata/**autofill_table.cc#oldcode39<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata/autofill_table.cc#oldcode39>
> >
>
>> > chrome/browser/webdata/****autofill_table.cc:39: // TODO(dhollowa):
>> Find a
>>
>> > common place for this.  It is duplicated in
>> > On 2012/04/30 16:03:09, dhollowa wrote:
>> >
>> >> The TODO should be removed as well.
>> >>
>> >
>> > Doh.  That was left in due to rebasing slippery fingers.  Fixed.
>> >
>> >
>>
>
> https://chromiumcodereview.**a**ppspot.com/10261004/%3Chttps:/**
>
/chromiumcodereview.appspot.**com/10261004/<http://appspot.com/10261004/%3Chttps://chromiumcodereview.appspot.com/10261004/>
> >
>
>> >
>>
>
>
>
https://chromiumcodereview.**appspot.com/10261004/<https://chromiumcodereview...
>

[gavinp, 2012-04-30 19:32:29]

On 2012/04/30 18:39:21, willchan wrote:

> I guess what I'd like to see is what *good* pattern is address_of() trying
> to make simpler. If we agree the existing uses are clumsy, then I'd prefer
> not to validate it by promoting the code to base/.
> 

My use is even more sketchy than the two above; I'm iterating a container, and
want to return a pointer to the found element, or NULL.  I didn't want to write
&(*i) <-- too confusing!, so I grepped for an existing address_?of, and found
this one to promote.

So perhaps I'll just write my code using an intermediate reference, and we can
avoid this going into base?

[dhollowa, 2012-04-30 19:35:16]

On 2012/04/30 18:39:21, willchan wrote:
> On Mon, Apr 30, 2012 at 11:29 AM, <mailto:dhollowa@chromium.org> wrote:
> 
> > On 2012/04/30 18:09:34, willchan wrote:
> >
> >> It's not clear why this functor is even necessary, since it's not being
> >> used to work around operator& (which we disallow in general in our
> >> codebase). At a glance, I don't see why
> >> AutofillProfile::**AdjustInferredProfiles() relies on it.
> >>
> >
> >
> > AutofillProfile::**AdjustInferredLabels does *not* itself rely on
> > |address_of|.
> > However, it takes a vector of pointers as its argument.  The caller holds a
> > vector of objects (not pointers) so that's why this conversion is applied:
> > namely to convert a vector of objects to a vector of pointers to those same
> > objects.
> >
> > This code has always seemed clumsy to me, but there is valid use of
> > address_of
> > given the existing interfaces.
> 
> 
> Yes, it's valid given the existing interfaces. Why don't we change the
> interfaces?
>
http://code.google.com/p/chromium/source/search?q=AdjustInferredLabels&origq=...
> makes
> it look like there's only 1 real consumer of that function, and
>
http://code.google.com/p/chromium/source/search?q=PersonalDataManager%253A%25...
> similarly
> only has one consumer.
> 
> I guess what I'd like to see is what *good* pattern is address_of() trying
> to make simpler. If we agree the existing uses are clumsy, then I'd prefer
> not to validate it by promoting the code to base/.
> 


I completely agree.  I would love to see |AutofillProfile::AdjustInferredLabels|
and |PersonalDataManager::MergeProfile| rationalized in this way.  One advantage
of the current approach however, especially for
|PersonalDataManager::MergeProfile| is that it doesn't require deep copies of
the data for intermediate results of computations.  However, this is maybe not
that big a deal.

I'd be curious to understand Gavin's use-case better before passing judgement.


> 
> >
> >
> >
> >  On the other hand, C++11 is going to provide a std::addressof() anyway, so
> >> if we decide we want to keep address_of() around, I'm fine with renaming
> >> it
> >> to base::addressof() and putting it in stl_util.h. That said, I'd rather
> >> us
> >> just not use it since it doesn't seem necessary. I'd like to hear what you
> >> guys think about this.
> >>
> >
> >  On Mon, Apr 30, 2012 at 9:27 AM, <mailto:gavinp@chromium.org> wrote:
> >>
> >
> >  > Thanks for your review, David.
> >> >
> >> > You may be right about moving it into chrome/common; my consumer is in
> >> > chrome/browser/prerender/, so that may make sense.  OTOH this code fits
> >> >  in with
> >> > what's in that header already; so I feel pressure from a principle of
> >> > putting
> >> > similar code together.
> >> >
> >> > willchan, how do you balance these competing interests?   You're the
> >> base/
> >> > reviewer....?
> >> >
> >> >
> >> >
> >> >
> >>
> >
> > https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/****
> > base/stl_util.h%3Chttps://**chromiumcodereview.appspot.**
> >
>
com/10261004/diff/1/base/stl_**util.h<http://appspot.com/10261004/diff/1/**base/stl_util.h%3Chttps://chromiumcodereview.appspot.com/10261004/diff/1/base/stl_util.h>
> > >
> >
> >> > File base/stl_util.h (right):
> >> >
> >> >
>
https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/**%3Chttp://appspo...>
> >> >
> >>
> >
> > base/stl_util.h#newcode14<http**s://chromiumcodereview.**
> >
>
appspot.com/10261004/diff/1/**base/stl_util.h#newcode14<https://chromiumcodereview.appspot.com/10261004/diff/1/base/stl_util.h#newcode14>
> > >
> >
> >> > base/stl_util.h:14: // Find the address of an object as a unary functor.
> >> > On 2012/04/30 16:03:09, dhollowa wrote:
> >> >
> >> >> s/Find/Return/
> >> >>
> >> >
> >> > Done.
> >> >
> >> >
> >> >
>
https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/**%3Chttp://appspo...>
> >> >
> >>
> >
> > chrome/browser/autofill/****personal_data_manager.cc<https**
> > ://chromiumcodereview.appspot.**com/10261004/diff/1/chrome/**
> >
>
browser/autofill/personal_**data_manager.cc<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofill/personal_data_manager.cc>
> > >
> >
> >> > File chrome/browser/autofill/****personal_data_manager.cc (left):
> >> >
> >> >
>
https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/**%3Chttp://appspo...>
> >> >
> >>
> >
> > chrome/browser/autofill/****personal_data_manager.cc#****oldcode63<
> > https://**chromiumcodereview.appspot.**com/10261004/diff/1/chrome/**
> >
>
browser/autofill/personal_**data_manager.cc#oldcode63<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/autofill/personal_data_manager.cc#oldcode63>
> > >
> >
> >> > chrome/browser/autofill/****personal_data_manager.cc:63: class
> >>
> >> > DereferenceFunctor {
> >> > On 2012/04/30 16:03:09, dhollowa wrote:
> >> >
> >> >> While we're at it, this |DereferenceFunctor| can be removed.  It is no
> >> >>
> >> > longer
> >> >
> >> >> used.
> >> >>
> >> >
> >> > Done.
> >> >
> >> >
> >> >
>
https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/**%3Chttp://appspo...>
> >> >
> >>
> >
> > chrome/browser/webdata/****autofill_table.cc<https://**
> > chromiumcodereview.appspot.**com/10261004/diff/1/chrome/**
> >
>
browser/webdata/autofill_**table.cc<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata/autofill_table.cc>
> > >
> >
> >> > File chrome/browser/webdata/****autofill_table.cc (left):
> >> >
> >> >
>
https://chromiumcodereview.**a**ppspot.com/10261004/diff/1/**%3Chttp://appspo...>
> >> >
> >>
> >
> > chrome/browser/webdata/****autofill_table.cc#oldcode39<ht**
> > tps://chromiumcodereview.**appspot.com/10261004/diff/1/**
> >
>
chrome/browser/webdata/**autofill_table.cc#oldcode39<https://chromiumcodereview.appspot.com/10261004/diff/1/chrome/browser/webdata/autofill_table.cc#oldcode39>
> > >
> >
> >> > chrome/browser/webdata/****autofill_table.cc:39: // TODO(dhollowa):
> >> Find a
> >>
> >> > common place for this.  It is duplicated in
> >> > On 2012/04/30 16:03:09, dhollowa wrote:
> >> >
> >> >> The TODO should be removed as well.
> >> >>
> >> >
> >> > Doh.  That was left in due to rebasing slippery fingers.  Fixed.
> >> >
> >> >
> >>
> >
> > https://chromiumcodereview.**a**ppspot.com/10261004/%253Chttps:/**
> >
>
/chromiumcodereview.appspot.**com/10261004/<http://appspot.com/10261004/%3Chttps://chromiumcodereview.appspot.com/10261004/>
> > >
> >
> >> >
> >>
> >
> >
> >
>
https://chromiumcodereview.**appspot.com/10261004/%3Chttps://chromiumcoderevi...>
> >

[gavinp, 2012-04-30 19:39:37]

Here's my use case:

http://codereview.chromium.org/10198040/diff/38003/chrome/browser/prerender/p...

I don't think that's a use case that justifies an API in base.

[gavinp, 2012-04-30 19:41:06]

Err, try again the link:
http://codereview.chromium.org/10198040/diff/38003/chrome/browser/prerender/p...

On 2012/04/30 19:39:37, gavinp wrote:
> Here's my use case:
> 
>
http://codereview.chromium.org/10198040/diff/38003/chrome/browser/prerender/p...
> 
> I don't think that's a use case that justifies an API in base.

[dhollowa, 2012-04-30 19:49:01]

On 2012/04/30 19:41:06, gavinp wrote:
> Err, try again the link:
>
http://codereview.chromium.org/10198040/diff/38003/chrome/browser/prerender/p...
> 
> On 2012/04/30 19:39:37, gavinp wrote:
> > Here's my use case:
> > 
> >
>
http://codereview.chromium.org/10198040/diff/38003/chrome/browser/prerender/p...
> > 
> > I don't think that's a use case that justifies an API in base.

Ha, ya.  I'd stick with |&| in that case.  :-)

[gavinp, 2012-04-30 19:53:51]

On 2012/04/30 19:49:01, dhollowa wrote:
> On 2012/04/30 19:41:06, gavinp wrote:
> > Err, try again the link:
> >
>
http://codereview.chromium.org/10198040/diff/38003/chrome/browser/prerender/p...
> > 
> > On 2012/04/30 19:39:37, gavinp wrote:
> > > Here's my use case:
> > > 
> > >
> >
>
http://codereview.chromium.org/10198040/diff/38003/chrome/browser/prerender/p...
> > > 
> > > I don't think that's a use case that justifies an API in base.
> 
> Ha, ya.  I'd stick with |&| in that case.  :-)

Yup.   It would have been MAYBE fine if the interface was already there; that's
why I'd have been happy to promote this if appropriate.  But given where this is
going, I think I should close this issue.

David, WDYT?

[dhollowa, 2012-04-30 20:00:19]

Fine by be to close, yes.  I'll try to get back to this TODO at some point and
rationalize the interfaces as described.


On 2012/04/30 19:53:51, gavinp wrote:
> On 2012/04/30 19:49:01, dhollowa wrote:
> > On 2012/04/30 19:41:06, gavinp wrote:
> > > Err, try again the link:
> > >
> >
>
http://codereview.chromium.org/10198040/diff/38003/chrome/browser/prerender/p...
> > > 
> > > On 2012/04/30 19:39:37, gavinp wrote:
> > > > Here's my use case:
> > > > 
> > > >
> > >
> >
>
http://codereview.chromium.org/10198040/diff/38003/chrome/browser/prerender/p...
> > > > 
> > > > I don't think that's a use case that justifies an API in base.
> > 
> > Ha, ya.  I'd stick with |&| in that case.  :-)
> 
> Yup.   It would have been MAYBE fine if the interface was already there;
that's
> why I'd have been happy to promote this if appropriate.  But given where this
is
> going, I think I should close this issue.
> 
> David, WDYT?

[willchan no longer on Chromium, 2012-04-30 20:30:42]

Great, thanks folks!

On Mon, Apr 30, 2012 at 1:00 PM, <dhollowa@chromium.org> wrote:

> Fine by be to close, yes.  I'll try to get back to this TODO at some point
> and
> rationalize the interfaces as described.
>
>
>
> On 2012/04/30 19:53:51, gavinp wrote:
>
>> On 2012/04/30 19:49:01, dhollowa wrote:
>> > On 2012/04/30 19:41:06, gavinp wrote:
>> > > Err, try again the link:
>> > >
>> >
>>
>
> http://codereview.chromium.**org/10198040/diff/38003/**
>
chrome/browser/prerender/**prerender_manager.cc#pair-1113<http://codereview.chromium.org/10198040/diff/38003/chrome/browser/prerender/prerender_manager.cc#pair-1113>
>
>> > >
>> > > On 2012/04/30 19:39:37, gavinp wrote:
>> > > > Here's my use case:
>> > > >
>> > > >
>> > >
>> >
>>
>
> http://codereview.chromium.**org/10198040/diff/38003/**
>
chrome/browser/prerender/**prerender_manager.cc#pair-1085<http://codereview.chromium.org/10198040/diff/38003/chrome/browser/prerender/prerender_manager.cc#pair-1085>
>
>> > > >
>> > > > I don't think that's a use case that justifies an API in base.
>> >
>> > Ha, ya.  I'd stick with |&| in that case.  :-)
>>
>
>  Yup.   It would have been MAYBE fine if the interface was already there;
>>
> that's
>
>> why I'd have been happy to promote this if appropriate.  But given where
>> this
>>
> is
>
>> going, I think I should close this issue.
>>
>
>  David, WDYT?
>>
>
>
>
>
https://chromiumcodereview.**appspot.com/10261004/<https://chromiumcodereview...
>