Document has a RefPtr<MediaQueryMatcher>, and in ~Document it does this: if (m_mediaQueryMatcher) m_mediaQueryMatcher->documentDestroyed(); However, MediaQueryMatcher also references Document: MediaQueryMatcher has Vector<OwnPtr<Listener> > m_listeners: - MediaQueryMatcher::Listener has RefPtr<MediaQueryListListener> m_listener; - MediaQueryListListener has ScriptValue m_value; - In V8, ScriptValue has ScopedPersistent<v8::Value> m_value; and it ends up keeping the Document alive. This reference path is cleared in MediaQueryMatcher::documentDestroyed() which destroys the MediaQueryListListeners. So, for example, if a tab is reloaded in Chrome, and there are MediaQueryListListeners, ~Document never happens. This causes www.crbug.com/113983.
Created attachment 175973 [details] Patch
Comment on attachment 175973 [details] Patch Clearing flags on attachment: 175973 Committed r135709: <http://trac.webkit.org/changeset/135709>
All reviewed patches have been landed. Closing bug.
Is it correct that a document that has been detached can never be re-attached (and we'll never have such a code path)? > if (m_mediaQueryMatcher) > m_mediaQueryMatcher->documentDestroyed(); Calling this function from a function other than destructor is not telling the truth. You should have renamed it to documentDetached.
Comment on attachment 175973 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175973&action=review > Source/WebCore/dom/Document.cpp:2144 > + if (m_mediaQueryMatcher) > + m_mediaQueryMatcher->documentDestroyed(); Why does the m_mediaQueryMatcher need special handling here? Can't it use one of our more general mechanisms for getting notified about the Document's lifecycle?
Update: We're discussing with abarth@ what is the right way to fix this bug. So I won't do the name change documentDestroyed -> documentDetached before the real fix. This quick and dirty fix was merged to Chromium, and I'm willing to work on the real fix, as soon as we figure out how it should be done.
(In reply to comment #6) > Update: > > We're discussing with abarth@ what is the right way to fix this bug. So I won't do the name change documentDestroyed -> documentDetached before the real fix. > > This quick and dirty fix was merged to Chromium, and I'm willing to work on the real fix, as soon as we figure out how it should be done. Given the fact that this circular reference is caused by matchMedia listeners, I guess we could add a method removeAllListeners to MediaQueryMatcher and call that in Document::removeAllEventListeners()?