Simplify and weaken DOMWindowProperty unregistration

Source/core/frame/LocalDOMWindow.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 538 matching lines @@
 
 void LocalDOMWindow::willDetachFrameHost()
 {
     m_frame->host()->eventHandlerRegistry().didRemoveAllEventHandlers(*this);
     m_frame->console().messageStorage()->frameWindowDiscarded(this);
     InspectorInstrumentation::frameWindowDiscarded(m_frame, this);
 }
 
 void LocalDOMWindow::willDestroyDocumentInFrame()
 {
-    // It is necessary to copy m_properties to a separate vector because the DOMWindowProperties may
-    // unregister themselves from the LocalDOMWindow as a result of the call to willDestroyGlobalObjectInFrame.
-    WillBeHeapVector<RawPtrWillBeMember<DOMWindowProperty> > properties;

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

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?

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

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]

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

-    copyToVector(m_properties, properties);
-    for (size_t i = 0; i < properties.size(); ++i)
-        properties[i]->willDestroyGlobalObjectInFrame();
+    for (WillBeHeapHashSet<RawPtrWillBeWeakMember<DOMWindowProperty> >::const_iterator it = m_properties.begin(); it != m_properties.end(); ++it)
+        (*it)->willDestroyGlobalObjectInFrame();
 }
 
 void LocalDOMWindow::willDetachDocumentFromFrame()
 {
-    // It is necessary to copy m_properties to a separate vector because the DOMWindowProperties may
-    // unregister themselves from the LocalDOMWindow as a result of the call to willDetachGlobalObjectFromFrame.
-    WillBeHeapVector<RawPtrWillBeMember<DOMWindowProperty> > properties;

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

Ditto.

-    copyToVector(m_properties, properties);
-    for (size_t i = 0; i < properties.size(); ++i)
-        properties[i]->willDetachGlobalObjectFromFrame();
+    for (WillBeHeapHashSet<RawPtrWillBeWeakMember<DOMWindowProperty> >::const_iterator it = m_properties.begin(); it != m_properties.end(); ++it)
+        (*it)->willDetachGlobalObjectFromFrame();
 }
 
 void LocalDOMWindow::registerProperty(DOMWindowProperty* property)
 {
     m_properties.add(property);
 }
 
 void LocalDOMWindow::unregisterProperty(DOMWindowProperty* property)
 {
     m_properties.remove(property);
@@ skipping 1335 matching lines @@
     LifecycleContext<LocalDOMWindow>::trace(visitor);
 }
 
 v8::Handle<v8::Object> LocalDOMWindow::wrap(v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
 {
     ASSERT_NOT_REACHED(); // LocalDOMWindow has [Custom=ToV8].
     return v8::Handle<v8::Object>();
 }
 
 } // namespace blink