Prepare SVGDocumentExtensions::m_elementDependencies for oilpan

Prepare SVGDocumentExtensions::m_elementDependencies for oilpan

|m_elementDependencies| had raw ptr to |SVGElement|s.
This was to avoid reference cycles, but we can convert this to strong references in oilpan.

If the whole document is dead, rebuild code is not called. This means that
rebuildElementReference is not called for referencing element, but the
referencing element must be in the same document and is going to die anyway.
If the whole document isn't dead (== the part of the document tree is going
away), the unregister/rebuild code is guaranteed to be called from removedFrom,
and rebuildElementReference is guaranteed to be triggered from there.

BUG=357163, 370834
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175880

[kouhei (in TOK), 2014-06-09 09:19:14]

Discussed offline with haraken. We are considering if we can remove the
dependency map and move the WeakSVGElementSet into SVGElement itself.
However, I'd like to land this patch anyway so we can move incrementally.

Would you take a look?

[kouhei (in TOK), 2014-06-09 09:21:41]

https://codereview.chromium.org/327473002/diff/1/Source/core/svg/SVGDocumentE...
File Source/core/svg/SVGDocumentExtensions.h (right):

https://codereview.chromium.org/327473002/diff/1/Source/core/svg/SVGDocumentE...
Source/core/svg/SVGDocumentExtensions.h:117: // FIXME: Oilpan: make below
HeapHashSet<WeakMember<SVGSVGElement> >
I'll move this in a separate CL.

[haraken, 2014-06-09 09:33:58]

https://codereview.chromium.org/327473002/diff/20001/Source/core/rendering/sv...
File Source/core/rendering/svg/RenderSVGResource.cpp (right):

https://codereview.chromium.org/327473002/diff/20001/Source/core/rendering/sv...
Source/core/rendering/svg/RenderSVGResource.cpp:194: typedef
WillBeHeapHashSet<Member<SVGElement> > SVGElementSet;

This should be WillBeHeapHashSet<RawPtrWillBeMember<SVGElement>>.

https://codereview.chromium.org/327473002/diff/20001/Source/core/svg/SVGDocum...
File Source/core/svg/SVGDocumentExtensions.h (right):

https://codereview.chromium.org/327473002/diff/20001/Source/core/svg/SVGDocum...
Source/core/svg/SVGDocumentExtensions.h:113: typedef
WillBeHeapHashMap<RawPtrWillBeWeakMember<SVGElement>,
OwnPtrWillBeMember<WeakSVGElementSet> > ElementDependenciesMap;

Erik: If the WeakSVGElementSet becomes empty, will the entry from a
WeakMember<SVGElement> to the Member<WeakSVGElementSet> be removed from the map
by default weak processing? Otherwise, this CL is wrong, because empty
WeakSVGElementSets will be left in the map.

(Either way I think we need to land Erik's CL first:
https://codereview.chromium.org/319593004/)

[kouhei (in TOK), 2014-06-09 10:11:23]

On 2014/06/09 09:33:58, haraken wrote:
>
https://codereview.chromium.org/327473002/diff/20001/Source/core/rendering/sv...
> File Source/core/rendering/svg/RenderSVGResource.cpp (right):
> 
>
https://codereview.chromium.org/327473002/diff/20001/Source/core/rendering/sv...
> Source/core/rendering/svg/RenderSVGResource.cpp:194: typedef
> WillBeHeapHashSet<Member<SVGElement> > SVGElementSet;
> 
> This should be WillBeHeapHashSet<RawPtrWillBeMember<SVGElement>>.
Done.

> empty WeakSVGElementSets will be left in the map.
That's possible, but I think we can ignore that for a moment.
If the set becomes empty, it is likely that the key element is destructed soon.

[haraken, 2014-06-09 10:37:02]

In terms of oilpan, LGTM.

> > This should be WillBeHeapHashSet<RawPtrWillBeMember<SVGElement>>.
> Done.
> 
> > empty WeakSVGElementSets will be left in the map.
> That's possible, but I think we can ignore that for a moment.
> If the set becomes empty, it is likely that the key element is destructed
soon.

I'd like to delegate this decision to pdr@. Please explain this kind of
non-trivial changes in the CL description.

[kouhei (in TOK), 2014-06-10 08:38:36]

Changed to strong refs, so the code is more clear. I'd like to land this CL now
as it doesn't affect trunk and only affect oilpan builds.

If the whole document is dead, rebuild code is not called. This means that
rebuildElementReference is not called for referencing element, but the
referencing element must be in the same document and is going to die anyway.
If the whole document isn't dead (== the part of the document tree is going
away), the unregister/rebuild code is guaranteed to be called from removedFrom,
and rebuildElementReference is guaranteed to be triggered from there.

[haraken, 2014-06-10 08:48:19]

Strong references sound reasonable.

https://codereview.chromium.org/327473002/diff/80001/Source/core/svg/SVGDocum...
File Source/core/svg/SVGDocumentExtensions.h (right):

https://codereview.chromium.org/327473002/diff/80001/Source/core/svg/SVGDocum...
Source/core/svg/SVGDocumentExtensions.h:114:
OwnPtrWillBeMember<ElementDependenciesMap> m_elementDependencies;

I wonder why you need to make this an OwnPtrWillBeMember. Doesn't
ElementDependenciesMap work?

https://codereview.chromium.org/327473002/diff/80001/Source/core/svg/SVGDocum...
Source/core/svg/SVGDocumentExtensions.h:117: // FIXME: Oilpan: make below
HeapHashSet<WeakMember<SVGSVGElement> >

I don't fully understand why this needs to be weak. Given that SVGSVGElements in
this hash set die together with the document, can we use a strong Member for
this?

[kouhei (in TOK), 2014-06-10 08:58:59]

https://codereview.chromium.org/327473002/diff/80001/Source/core/svg/SVGDocum...
File Source/core/svg/SVGDocumentExtensions.h (right):

https://codereview.chromium.org/327473002/diff/80001/Source/core/svg/SVGDocum...
Source/core/svg/SVGDocumentExtensions.h:114:
OwnPtrWillBeMember<ElementDependenciesMap> m_elementDependencies;
On 2014/06/10 08:48:19, haraken wrote:
> 
> I wonder why you need to make this an OwnPtrWillBeMember. Doesn't
> ElementDependenciesMap work?

Done.

https://codereview.chromium.org/327473002/diff/80001/Source/core/svg/SVGDocum...
Source/core/svg/SVGDocumentExtensions.h:117: // FIXME: Oilpan: make below
HeapHashSet<WeakMember<SVGSVGElement> >
On 2014/06/10 08:48:19, haraken wrote:
> 
> I don't fully understand why this needs to be weak. Given that SVGSVGElements
in
> this hash set die together with the document, can we use a strong Member for
> this?

This change was made in https://codereview.chromium.org/323803002/ and this was
landed as a strong ref.

[haraken, 2014-06-10 09:01:11]

LGTM

[kouhei (in TOK), 2014-06-10 09:01:26]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2014-06-10 09:02:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kouhei@chromium.org/327473002/120001

[commit-bot: I haz the power, 2014-06-10 10:05:32]

Change committed as 175880