Summary: | Circular reference between Document and MediaQueryMatcher. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Marja Hölttä <marja> | ||||
Component: | New Bugs | Assignee: | Marja Hölttä <marja> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, ap, beidson, hayato, jochen, kenneth, luiz, marcoos+bwo, ojan, webkit.review.bot, yongjun_zhang | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Marja Hölttä
2012-11-26 04:38:14 PST
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()? |