Skip to content

GitLab

T
tangled
Commit 04e5fb3b authored by sigbjornf@opera.com's avatar sigbjornf@opera.com
Browse files
Options

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

Review URL: https://codereview.chromium.org/569643002

git-svn-id: svn://svn.chromium.org/blink/trunk@181970 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 516b85b3
Hide whitespace changes
Inline Side-by-side
Showing with 45 additions and 20 deletions
third_party/WebKit/LayoutTests/fast/dom/shadow/shadow-selection-detach-crash-expected.txt 0 → 100644
View file @ 04e5fb3b
Garbage collection of Selection objects with shorter lifetimes must not crash.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS No crash, including on shutdown..?
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/fast/dom/shadow/shadow-selection-detach-crash.html 0 → 100644
View file @ 04e5fb3b
<!DOCTYPE html>
<script src="../../../resources/js-test.js"></script>
<output id="out"></output>
<script>
description("Garbage collection of Selection objects with shorter lifetimes must not crash.");
// A Selection object that dies before the window object it is registered to, must detach
// itself from its window on finalization.
self.jsTestIsAsync = true;
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}
function runTest() {
var out = document.getElementById("out");
var root = out.createShadowRoot();
var sel = root.getSelection();
document.body.removeChild(out);
root = sel = out = null;
gc();
testPassed("No crash, including on shutdown..?");
finishJSTest();
}
window.onload = runTest;
</script>
third_party/WebKit/Source/core/frame/DOMWindowProperty.cpp
View file @ 04e5fb3b
......@@ -50,9 +50,6 @@ DOMWindowProperty::~DOMWindowProperty()
{
if (m_associatedDOMWindow)
m_associatedDOMWindow->unregisterProperty(this);
m_associatedDOMWindow = nullptr;
m_frame = nullptr;
}
#endif
......@@ -62,10 +59,10 @@ void DOMWindowProperty::willDestroyGlobalObjectInFrame()
ASSERT(m_frame);
ASSERT(m_associatedDOMWindow);
// DOMWindowProperty lifetime isn't tied directly to the LocalDOMWindow itself so it is important that it unregister
// itself from any LocalDOMWindow it is associated with if that LocalDOMWindow is going away.
if (m_associatedDOMWindow)
m_associatedDOMWindow->unregisterProperty(this);
// LocalDOMWindow will along with notifying DOMWindowProperty objects of
// impending destruction, unilaterally clear out its registered set.
// Consequently, no explicit unregisteration required by DOMWindowProperty;
// here or when finalized.
m_associatedDOMWindow = nullptr;
m_frame = nullptr;
}
......
third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp
View file @ 04e5fb3b
......@@ -557,22 +557,14 @@ void LocalDOMWindow::willDetachFrameHost()
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;
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;
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)
......
third_party/WebKit/Source/core/frame/LocalDOMWindow.h
View file @ 04e5fb3b
......@@ -366,7 +366,7 @@ private:
bool m_hasBeenReset;
#endif
WillBeHeapHashSet<RawPtrWillBeMember<DOMWindowProperty> > m_properties;
WillBeHeapHashSet<RawPtrWillBeWeakMember<DOMWindowProperty> > m_properties;
mutable RefPtrWillBeMember<Screen> m_screen;
mutable RefPtrWillBeMember<History> m_history;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment