Profiles: Add a size check to ProfileImpl to make sure it no longer grows.

Profiles: Add a size check to ProfileImpl to make sure it no longer grows.

We need to prevent regressions where someone will add another service
to ProfileImpl that isn't managed by a ProfileKeyedServiceFactory.

BUG=77155
TEST=compiles on Linux x64


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

[Miranda Callahan, 2012-01-06 15:34:57]

Thank you!!

http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
File chrome/browser/profiles/profile_impl.cc (right):

http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
chrome/browser/profiles/profile_impl.cc:165: COMPILE_ASSERT(sizeof(ProfileImpl)
== 656u, profile_impl_size_unexpected);
Should we instead assert sizeof(ProfileImpl) <= 656u, so that we're allowed to
remove services without triggering errors?

http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
File chrome/browser/profiles/profile_impl.h (right):

http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
chrome/browser/profiles/profile_impl.h:322: // Profile use our new
ProfileKeyedSerivceFactory system instead. You can
nit/serivce/service

[Elliot Glaysher, 2012-01-06 18:23:58]

http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
File chrome/browser/profiles/profile_impl.cc (right):

http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
chrome/browser/profiles/profile_impl.cc:165: COMPILE_ASSERT(sizeof(ProfileImpl)
== 656u, profile_impl_size_unexpected);
On 2012/01/06 15:34:57, Miranda Callahan wrote:
> Should we instead assert sizeof(ProfileImpl) <= 656u, so that we're allowed to
> remove services without triggering errors?

I'm not crazy about this, but I changed it.

Using <= means I'll have to constantly maintain this, or else someone will
remove an object and then someone else may add an object without this
triggering.

[Miranda Callahan, 2012-01-06 18:31:44]

On 2012/01/06 18:23:58, Elliot Glaysher wrote:
>
http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
> File chrome/browser/profiles/profile_impl.cc (right):
> 
>
http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
> chrome/browser/profiles/profile_impl.cc:165:
COMPILE_ASSERT(sizeof(ProfileImpl)
> == 656u, profile_impl_size_unexpected);
> On 2012/01/06 15:34:57, Miranda Callahan wrote:
> > Should we instead assert sizeof(ProfileImpl) <= 656u, so that we're allowed
to
> > remove services without triggering errors?
> 
> I'm not crazy about this, but I changed it.
> 
> Using <= means I'll have to constantly maintain this, or else someone will
> remove an object and then someone else may add an object without this
> triggering.

Hmm -- I thought of that, but my original thought was that it would be too
onerous to make each person that changed ProfileImpl adjust the size var here --
but maybe it isn't. What if we leave the compile assert error, but add a comment
above it that explains exactly what the user must do if they alter the size of
the ProfileImpl object (where to find the number, exactly which number to use)?
This may be trivial, but would probably be sufficient to stop all the "WTF"
emails you might get if you leave it out -- and keep you from having to
constantly maintain.

[Miranda Callahan, 2012-01-06 18:37:18]

On 2012/01/06 18:31:44, Miranda Callahan wrote:
> On 2012/01/06 18:23:58, Elliot Glaysher wrote:
> >
>
http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
> > File chrome/browser/profiles/profile_impl.cc (right):
> > 
> >
>
http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
> > chrome/browser/profiles/profile_impl.cc:165:
> COMPILE_ASSERT(sizeof(ProfileImpl)
> > == 656u, profile_impl_size_unexpected);
> > On 2012/01/06 15:34:57, Miranda Callahan wrote:
> > > Should we instead assert sizeof(ProfileImpl) <= 656u, so that we're
allowed
> to
> > > remove services without triggering errors?
> > 
> > I'm not crazy about this, but I changed it.
> > 
> > Using <= means I'll have to constantly maintain this, or else someone will
> > remove an object and then someone else may add an object without this
> > triggering.
> 
> Hmm -- I thought of that, but my original thought was that it would be too
> onerous to make each person that changed ProfileImpl adjust the size var here
--
> but maybe it isn't. What if we leave the compile assert error, but add a
comment
> above it that explains exactly what the user must do if they alter the size of
> the ProfileImpl object (where to find the number, exactly which number to
use)?
> This may be trivial, but would probably be sufficient to stop all the "WTF"
> emails you might get if you leave it out -- and keep you from having to
> constantly maintain.

(I mean, "leave the compile assert error as ==".)

[Elliot Glaysher, 2012-01-06 19:23:13]

On 2012/01/06 18:37:18, Miranda Callahan wrote:
> On 2012/01/06 18:31:44, Miranda Callahan wrote:
> > On 2012/01/06 18:23:58, Elliot Glaysher wrote:
> > >
> >
>
http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
> > > File chrome/browser/profiles/profile_impl.cc (right):
> > > 
> > >
> >
>
http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
> > > chrome/browser/profiles/profile_impl.cc:165:
> > COMPILE_ASSERT(sizeof(ProfileImpl)
> > > == 656u, profile_impl_size_unexpected);
> > > On 2012/01/06 15:34:57, Miranda Callahan wrote:
> > > > Should we instead assert sizeof(ProfileImpl) <= 656u, so that we're
> allowed
> > to
> > > > remove services without triggering errors?
> > > 
> > > I'm not crazy about this, but I changed it.
> > > 
> > > Using <= means I'll have to constantly maintain this, or else someone will
> > > remove an object and then someone else may add an object without this
> > > triggering.
> > 
> > Hmm -- I thought of that, but my original thought was that it would be too
> > onerous to make each person that changed ProfileImpl adjust the size var
here
> --
> > but maybe it isn't. What if we leave the compile assert error, but add a
> comment
> > above it that explains exactly what the user must do if they alter the size
of
> > the ProfileImpl object (where to find the number, exactly which number to
> use)?
> > This may be trivial, but would probably be sufficient to stop all the "WTF"
> > emails you might get if you leave it out -- and keep you from having to
> > constantly maintain.
> 
> (I mean, "leave the compile assert error as ==".)

On second thought, I'll just maintain it. We already use expectations everywhere
else that someone needs to update when we progress.

[Miranda Callahan, 2012-01-06 19:32:17]

Ok -- LGTM.

On 2012/01/06 19:23:13, Elliot Glaysher wrote:
> On 2012/01/06 18:37:18, Miranda Callahan wrote:
> > On 2012/01/06 18:31:44, Miranda Callahan wrote:
> > > On 2012/01/06 18:23:58, Elliot Glaysher wrote:
> > > >
> > >
> >
>
http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
> > > > File chrome/browser/profiles/profile_impl.cc (right):
> > > > 
> > > >
> > >
> >
>
http://codereview.chromium.org/9111035/diff/1/chrome/browser/profiles/profile...
> > > > chrome/browser/profiles/profile_impl.cc:165:
> > > COMPILE_ASSERT(sizeof(ProfileImpl)
> > > > == 656u, profile_impl_size_unexpected);
> > > > On 2012/01/06 15:34:57, Miranda Callahan wrote:
> > > > > Should we instead assert sizeof(ProfileImpl) <= 656u, so that we're
> > allowed
> > > to
> > > > > remove services without triggering errors?
> > > > 
> > > > I'm not crazy about this, but I changed it.
> > > > 
> > > > Using <= means I'll have to constantly maintain this, or else someone
will
> > > > remove an object and then someone else may add an object without this
> > > > triggering.
> > > 
> > > Hmm -- I thought of that, but my original thought was that it would be too
> > > onerous to make each person that changed ProfileImpl adjust the size var
> here
> > --
> > > but maybe it isn't. What if we leave the compile assert error, but add a
> > comment
> > > above it that explains exactly what the user must do if they alter the
size
> of
> > > the ProfileImpl object (where to find the number, exactly which number to
> > use)?
> > > This may be trivial, but would probably be sufficient to stop all the
"WTF"
> > > emails you might get if you leave it out -- and keep you from having to
> > > constantly maintain.
> > 
> > (I mean, "leave the compile assert error as ==".)
> 
> On second thought, I'll just maintain it. We already use expectations
everywhere
> else that someone needs to update when we progress.

[commit-bot: I haz the power, 2012-01-06 20:03:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/erg@chromium.org/9111035/4001

[commit-bot: I haz the power, 2012-01-06 21:27:18]

Change committed as 116743