Change Layout::adjustCompositedContainerForSpecialAncestors into an overloaded version of container…

Change Layout::adjustCompositedContainerForSpecialAncestors into an overloaded version of containerForPaintInvalidation

only one caller (LayoutBoxModelObject) -- all other callers call the original containerForPaintInvalidation without parameters

BUG=436473

[pilgrim_google, 2015-06-02 14:08:08]

pilgrim@chromium.org changed reviewers:
+ dsinclair@chromium.org, eae@chromium.org, leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-06-02 17:49:51]

https://codereview.chromium.org/1161863004/diff/1/Source/core/layout/LayoutOb...
File Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/1161863004/diff/1/Source/core/layout/LayoutOb...
Source/core/layout/LayoutObject.cpp:1065: const LayoutBoxModelObject*
LayoutObject::containerForPaintInvalidation(const LayoutBoxModelObject*
paintInvalidationContainer) const
I like the simplification ideal behind this patch, but I'm not convinced it's
actually clearer.

Calling LayoutObject::containerForPaintInvalidation with a LayoutBoxModelObject*
parameter named "paintInvalidationContainer" doesn't explain to me what's
actually going on in the function. Also passing in a null pointer to that method
doesn't do the same thing as calling LayoutObject::containerForPaintInvalidation
without an argument, even though it's clear on inspection that it's prepared to
deal with null pointers.

The previous name gave some vague idea of what the method actually did, though
I'll certainly agree it's not very clear.

[pilgrim_google, 2015-06-02 20:21:19]

On 2015/06/02 at 17:49:51, leviw wrote:
> Calling LayoutObject::containerForPaintInvalidation with a
LayoutBoxModelObject* parameter named "paintInvalidationContainer" doesn't
explain to me what's actually going on in the function. Also passing in a null
pointer to that method doesn't do the same thing as calling
LayoutObject::containerForPaintInvalidation without an argument, even though
it's clear on inspection that it's prepared to deal with null pointers.

If I fix it so cfpi(null) is the same as cfpi(), would you accept it?

[leviw_travelin_and_unemployed, 2015-06-02 20:23:14]

On 2015/06/02 at 20:21:19, pilgrim wrote:
> On 2015/06/02 at 17:49:51, leviw wrote:
> > Calling LayoutObject::containerForPaintInvalidation with a
LayoutBoxModelObject* parameter named "paintInvalidationContainer" doesn't
explain to me what's actually going on in the function. Also passing in a null
pointer to that method doesn't do the same thing as calling
LayoutObject::containerForPaintInvalidation without an argument, even though
it's clear on inspection that it's prepared to deal with null pointers.
> 
> If I fix it so cfpi(null) is the same as cfpi(), would you accept it?

I'd expect something like this: cfpi(LayoutBoxModelObject* =
enclosingCompositedContainer()) where the default parameter is what a caller
wants.