Simplify and weaken DOMWindowProperty unregistration

Simplify and weaken DOMWindowProperty unregistration

A DOMWindowProperty object should not be kept alive if it is only
referenced from a LocalDOMWindow's set of registered DOMWindowProperty
objects. Adjust LocalDOMWindow's DOMObjectProperty references to be 
weak for Oilpan also.

In most cases, DOMWindowProperty objects will remain alive for at least
as long as their associated LocalDOMWindow, but this cannot be assumed.
The added test checks for that wrt DOMSelection objects created from
shadow roots.

Along with adjusting the weakness of these references, simplify
how LocalDOMWindow unregisters its DOMWindowProperty objects. That is,
no longer have the instances unregister themselves, but let the
LocalDOMWindow remove them all after having notified them of impending
destruction.

R=haraken,ager
BUG=413316
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181970

[sof, 2014-09-14 16:02:27]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[sof, 2014-09-14 16:02:27]

Please take a look.

Follow up and weaken what was reverted in
https://codereview.chromium.org/558213002/

The assumption is that epheremons allow the back reference from
DOMWindowProperty to remain a Member.

[haraken, 2014-09-15 05:08:52]

LGTM

[Mads Ager (chromium), 2014-09-15 07:58:21]

ager@chromium.org changed reviewers:
+ ager@chromium.org

[Mads Ager (chromium), 2014-09-15 07:58:22]

https://codereview.chromium.org/569643002/diff/40001/Source/core/frame/LocalD...
File Source/core/frame/LocalDOMWindow.cpp (left):

https://codereview.chromium.org/569643002/diff/40001/Source/core/frame/LocalD...
Source/core/frame/LocalDOMWindow.cpp:561:
WillBeHeapVector<RawPtrWillBeMember<DOMWindowProperty> > properties;
Do you still need the copy for the non-oilpan case? In the oilpan build you
don't because we strongify weak collections with pointers to them on the stack.
What prevents modification during iteration in the non-oilpan build? Has
something changed that ensures that the willDestroyGlobalObjectInFrame call
cannot lead to the death of a DOMWindowProperty which will then unregister
itself which will be a modification during iteration?

https://codereview.chromium.org/569643002/diff/40001/Source/core/frame/LocalD...
Source/core/frame/LocalDOMWindow.cpp:571:
WillBeHeapVector<RawPtrWillBeMember<DOMWindowProperty> > properties;
Ditto.

[sof, 2014-09-15 08:10:25]

https://codereview.chromium.org/569643002/diff/40001/Source/core/frame/LocalD...
File Source/core/frame/LocalDOMWindow.cpp (left):

https://codereview.chromium.org/569643002/diff/40001/Source/core/frame/LocalD...
Source/core/frame/LocalDOMWindow.cpp:561:
WillBeHeapVector<RawPtrWillBeMember<DOMWindowProperty> > properties;
On 2014/09/15 07:58:21, Mads Ager (chromium) wrote:
> Do you still need the copy for the non-oilpan case? In the oilpan build you
> don't because we strongify weak collections with pointers to them on the
stack.
> What prevents modification during iteration in the non-oilpan build? Has
> something changed that ensures that the willDestroyGlobalObjectInFrame call
> cannot lead to the death of a DOMWindowProperty which will then unregister
> itself which will be a modification during iteration?

No implementation of willDestroyGlobalObjectInFrame() unregisters nor deletes
itself, and DOMWindowProperty::willDestroyGlobalObjectInFrame() clears out the
back reference. This seems like a reasonably easy invariant to maintain "going
forward".

[Mads Ager (chromium), 2014-09-15 08:11:40]

LGTM

https://codereview.chromium.org/569643002/diff/40001/Source/core/frame/LocalD...
File Source/core/frame/LocalDOMWindow.cpp (left):

https://codereview.chromium.org/569643002/diff/40001/Source/core/frame/LocalD...
Source/core/frame/LocalDOMWindow.cpp:561:
WillBeHeapVector<RawPtrWillBeMember<DOMWindowProperty> > properties;
On 2014/09/15 08:10:25, sof wrote:
> On 2014/09/15 07:58:21, Mads Ager (chromium) wrote:
> > Do you still need the copy for the non-oilpan case? In the oilpan build you
> > don't because we strongify weak collections with pointers to them on the
> stack.
> > What prevents modification during iteration in the non-oilpan build? Has
> > something changed that ensures that the willDestroyGlobalObjectInFrame call
> > cannot lead to the death of a DOMWindowProperty which will then unregister
> > itself which will be a modification during iteration?
> 
> No implementation of willDestroyGlobalObjectInFrame() unregisters nor deletes
> itself, and DOMWindowProperty::willDestroyGlobalObjectInFrame() clears out the
> back reference. This seems like a reasonably easy invariant to maintain "going
> forward".

Sounds good. Thanks for double checking and explaining. :)

[sof, 2014-09-15 08:26:32]

Thanks for the reviews.

Should there be unexpected problems with the unregistration simplifications
here, will separate out the addition of WeakMember<> only. But I think they're
both fine.

[sof, 2014-09-15 08:26:41]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-09-15 08:27:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/569643002/40001

[commit-bot: I haz the power, 2014-09-15 09:59:46]

Committed patchset #3 (id:40001) as 181970