Move InvalidationGuard/InstanceUpdateBlocker out of SVGElementInstance

Move InvalidationGuard/InstanceUpdateBlocker out of SVGElementInstance

Move InvalidationGuard/InstanceUpdateBlocker out of SVGElementInstance and
rename InvalidationGuard to InstanceInvalidationGuard.

BUG=313438
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=173343

[rwlbuis, 2014-05-05 20:40:44]

PTAL :) This code will need a new home, I think it is good to get it out of the
way early.

[pdr., 2014-05-05 21:00:14]

LGTM with two nits

https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.cpp
File Source/core/svg/SVGElement.cpp (right):

https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.c...
Source/core/svg/SVGElement.cpp:1050: void
SVGElement::invalidateAllInstancesOfElement(SVGElement* element)
There are only a few callers of this. Could we go ahead and make this
non-static?

E.g.,
void SVGElement::invalidateAllInstancesOfElement()

https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.h
File Source/core/svg/SVGElement.h (right):

https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.h...
Source/core/svg/SVGElement.h:158: class InstanceInvalidationGuard {
I would have left this as InvalidationGuard since the instance concept will be
going away soon.

[rwlbuis, 2014-05-05 21:14:34]

On 2014/05/05 21:00:14, pdr wrote:
> LGTM with two nits
> 
>
https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.cpp
> File Source/core/svg/SVGElement.cpp (right):
> 
>
https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.c...
> Source/core/svg/SVGElement.cpp:1050: void
> SVGElement::invalidateAllInstancesOfElement(SVGElement* element)
> There are only a few callers of this. Could we go ahead and make this
> non-static?
> 
> E.g.,
> void SVGElement::invalidateAllInstancesOfElement()

Yeah makes sense. As discussed on irc the naming is the harder part...

> https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.h
> File Source/core/svg/SVGElement.h (right):
> 
>
https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.h...
> Source/core/svg/SVGElement.h:158: class InstanceInvalidationGuard {
> I would have left this as InvalidationGuard since the instance concept will be
> going away soon.

Good point, I still think just 'InvalidationGuard' may be a bit generic. So
naming problems again :)

[pdr., 2014-05-05 21:42:25]

On 2014/05/05 21:14:34, rwlbuis wrote:
> On 2014/05/05 21:00:14, pdr wrote:
> > LGTM with two nits
> > 
> >
>
https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.cpp
> > File Source/core/svg/SVGElement.cpp (right):
> > 
> >
>
https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.c...
> > Source/core/svg/SVGElement.cpp:1050: void
> > SVGElement::invalidateAllInstancesOfElement(SVGElement* element)
> > There are only a few callers of this. Could we go ahead and make this
> > non-static?
> > 
> > E.g.,
> > void SVGElement::invalidateAllInstancesOfElement()
> 
> Yeah makes sense. As discussed on irc the naming is the harder part...
> 
> >
https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.h
> > File Source/core/svg/SVGElement.h (right):
> > 
> >
>
https://codereview.chromium.org/268223003/diff/1/Source/core/svg/SVGElement.h...
> > Source/core/svg/SVGElement.h:158: class InstanceInvalidationGuard {
> > I would have left this as InvalidationGuard since the instance concept will
be
> > going away soon.
> 
> Good point, I still think just 'InvalidationGuard' may be a bit generic. So
> naming problems again :)

LGTM

[rwlbuis, 2014-05-05 21:45:15]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2014-05-05 21:45:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rob.buis@samsung.com/268223003/20001

[commit-bot: I haz the power, 2014-05-05 22:53:37]

Change committed as 173343