Crash in DumpRenderTree at com.apple.WebCore: WebCore::CaptionUserPreferences::captio...
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Mar 2013 17:27:20 +0000 (17:27 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Mar 2013 17:27:20 +0000 (17:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=112051

Reviewed by Eric Carlson.

No new tests; fixes a crash during media/video-controls-captions-trackmenu.html.

Instead of relying on a registration system which can fail when an element's document does not have a page,
Elements will register for captionPreferencesChanged() notifications directly with their owning Document.
CaptionUserPreferences, in turn, will notify all Documents in its PageGroup, rather than only directly
registered listeners.

* dom/Document.cpp:
(WebCore::Document::registerForCaptionPreferencesChangedCallbacks): Added. Notify the CaptionUserPreferences that someone
    is interested in captionPreferencesChanged notfications.
(WebCore::Document::unregisterForCaptionPreferencesChangedCallbacks): Added.
(WebCore::Document::captionPreferencesChanged): Added. Pass to all registered elements.
* dom/Document.h:
* dom/Element.h:
(WebCore::Element::captionPreferencesChanged): Added. Empty; intended
    to be overridden by subclasses.
* history/CachedPage.cpp:
(WebCore::CachedPage::CachedPage): Initialize m_needsCaptionPreferenceChanged member.
(WebCore::CachedPage::restore): Call captionPreferencesChanged() if necessary.
* history/CachedPage.h:
(WebCore::CachedPage::markForCaptionPreferencesChanged): Set the m_needsCaptionPreferenceChanged member.
* history/PageCache.cpp:
(WebCore::PageCache::markPagesForCaptionPreferencesChanged): Pass to every CachedPage.
* history/PageCache.h:
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::HTMLMediaElement): Register with the Document.
(WebCore::HTMLMediaElement::~HTMLMediaElement): Unregister with same.
(WebCore::HTMLMediaElement::attach): Remove previous registration call.
* html/HTMLMediaElement.h:
* page/CaptionUserPreferences.cpp:
(WebCore::CaptionUserPreferences::captionPreferencesChanged): Pass to the
    PageGroup.
* page/CaptionUserPreferences.h:
(WebCore::CaptionUserPreferences::setInterestedInCaptionPreferenceChanges):
    Empty; intended to be overridden by subclasses.
* page/CaptionUserPreferencesMac.h:
* page/CaptionUserPreferencesMac.mm:
(WebCore::CaptionUserPreferencesMac::setInterestedInCaptionPreferenceChanges):
    Renamed from registerForPreferencesChangedCallbacks().
(WebCore::CaptionUserPreferencesMac::captionPreferencesChanged):
    Replace call to havePreferenceChangeListeners() with m_listeningForPreferenceChanges.
* page/Page.cpp:
(WebCore::Page::captionPreferencesChanged):
    Pass to every contained Document.
* page/Page.h:
* page/CaptionUserPreferences.cpp:
(WebCore::CaptionUserPreferences::captionPreferencesChanged): Pass to the PageGroup.
* page/CaptionUserPreferences.h:
* page/PageGroup.cpp:
(WebCore::PageGroup::captionPreferencesChanged): Pass to every page, as well as pages in the PageCache.
* page/PageGroup.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@145826 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/Element.h
Source/WebCore/history/CachedPage.cpp
Source/WebCore/history/CachedPage.h
Source/WebCore/history/PageCache.cpp
Source/WebCore/history/PageCache.h
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/html/HTMLMediaElement.h
Source/WebCore/page/CaptionUserPreferences.cpp
Source/WebCore/page/CaptionUserPreferences.h
Source/WebCore/page/CaptionUserPreferencesMac.h
Source/WebCore/page/CaptionUserPreferencesMac.mm
Source/WebCore/page/Page.cpp
Source/WebCore/page/Page.h
Source/WebCore/page/PageGroup.cpp
Source/WebCore/page/PageGroup.h

index cb952ba..87b4776 100644 (file)
@@ -1,3 +1,62 @@
+2013-03-11  Jer Noble  <jer.noble@apple.com>
+
+        Crash in DumpRenderTree at com.apple.WebCore: WebCore::CaptionUserPreferences::captionPreferencesChanged + 185
+        https://bugs.webkit.org/show_bug.cgi?id=112051
+
+        Reviewed by Eric Carlson.
+
+        No new tests; fixes a crash during media/video-controls-captions-trackmenu.html.
+
+        Instead of relying on a registration system which can fail when an element's document does not have a page,
+        Elements will register for captionPreferencesChanged() notifications directly with their owning Document.
+        CaptionUserPreferences, in turn, will notify all Documents in its PageGroup, rather than only directly
+        registered listeners.
+
+        * dom/Document.cpp:
+        (WebCore::Document::registerForCaptionPreferencesChangedCallbacks): Added. Notify the CaptionUserPreferences that someone
+            is interested in captionPreferencesChanged notfications.
+        (WebCore::Document::unregisterForCaptionPreferencesChangedCallbacks): Added.
+        (WebCore::Document::captionPreferencesChanged): Added. Pass to all registered elements.
+        * dom/Document.h:
+        * dom/Element.h:
+        (WebCore::Element::captionPreferencesChanged): Added. Empty; intended
+            to be overridden by subclasses.
+        * history/CachedPage.cpp:
+        (WebCore::CachedPage::CachedPage): Initialize m_needsCaptionPreferenceChanged member.
+        (WebCore::CachedPage::restore): Call captionPreferencesChanged() if necessary.
+        * history/CachedPage.h:
+        (WebCore::CachedPage::markForCaptionPreferencesChanged): Set the m_needsCaptionPreferenceChanged member.
+        * history/PageCache.cpp:
+        (WebCore::PageCache::markPagesForCaptionPreferencesChanged): Pass to every CachedPage.
+        * history/PageCache.h:
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::HTMLMediaElement): Register with the Document.
+        (WebCore::HTMLMediaElement::~HTMLMediaElement): Unregister with same.
+        (WebCore::HTMLMediaElement::attach): Remove previous registration call.
+        * html/HTMLMediaElement.h:
+        * page/CaptionUserPreferences.cpp:
+        (WebCore::CaptionUserPreferences::captionPreferencesChanged): Pass to the
+            PageGroup.
+        * page/CaptionUserPreferences.h:
+        (WebCore::CaptionUserPreferences::setInterestedInCaptionPreferenceChanges):
+            Empty; intended to be overridden by subclasses.
+        * page/CaptionUserPreferencesMac.h:
+        * page/CaptionUserPreferencesMac.mm:
+        (WebCore::CaptionUserPreferencesMac::setInterestedInCaptionPreferenceChanges):
+            Renamed from registerForPreferencesChangedCallbacks().
+        (WebCore::CaptionUserPreferencesMac::captionPreferencesChanged):
+            Replace call to havePreferenceChangeListeners() with m_listeningForPreferenceChanges.
+        * page/Page.cpp:
+        (WebCore::Page::captionPreferencesChanged):
+            Pass to every contained Document.
+        * page/Page.h:
+        * page/CaptionUserPreferences.cpp:
+        (WebCore::CaptionUserPreferences::captionPreferencesChanged): Pass to the PageGroup.
+        * page/CaptionUserPreferences.h:
+        * page/PageGroup.cpp:
+        (WebCore::PageGroup::captionPreferencesChanged): Pass to every page, as well as pages in the PageCache.
+        * page/PageGroup.h:
+
 2013-03-14  David Grogan  <dgrogan@chromium.org>
 
         IndexedDB: Histogram leveldb open errors.
index 45d7a0b..30183c2 100644 (file)
 #include "TraceEvent.h"
 #endif
 
+#if ENABLE(VIDEO_TRACK)
+#include "CaptionUserPreferences.h"
+#endif
+
 using namespace std;
 using namespace WTF;
 using namespace Unicode;
@@ -4154,6 +4158,28 @@ void Document::unregisterForPrivateBrowsingStateChangedCallbacks(Element* e)
     m_privateBrowsingStateChangedElements.remove(e);
 }
 
+#if ENABLE(VIDEO_TRACK)
+void Document::registerForCaptionPreferencesChangedCallbacks(Element* e)
+{
+    if (page())
+        page()->group().captionPreferences()->setInterestedInCaptionPreferenceChanges();
+
+    m_captionPreferencesChangedElements.add(e);
+}
+
+void Document::unregisterForCaptionPreferencesChangedCallbacks(Element* e)
+{
+    m_captionPreferencesChangedElements.remove(e);
+}
+
+void Document::captionPreferencesChanged()
+{
+    HashSet<Element*>::iterator end = m_captionPreferencesChangedElements.end();
+    for (HashSet<Element*>::iterator it = m_captionPreferencesChangedElements.begin(); it != end; ++it)
+        (*it)->captionPreferencesChanged();
+}
+#endif
+
 void Document::setShouldCreateRenderers(bool f)
 {
     m_createRenderers = f;
index f5a8243..48a5a21 100644 (file)
@@ -972,6 +972,12 @@ public:
     void storageBlockingStateDidChange();
     void privateBrowsingStateDidChange();
 
+#if ENABLE(VIDEO_TRACK)
+    void registerForCaptionPreferencesChangedCallbacks(Element*);
+    void unregisterForCaptionPreferencesChangedCallbacks(Element*);
+    void captionPreferencesChanged();
+#endif
+
     void setShouldCreateRenderers(bool);
     bool shouldCreateRenderers();
 
@@ -1464,8 +1470,11 @@ private:
     HashSet<Element*> m_documentSuspensionCallbackElements;
     HashSet<Element*> m_mediaVolumeCallbackElements;
     HashSet<Element*> m_privateBrowsingStateChangedElements;
+#if ENABLE(VIDEO_TRACK)
+    HashSet<Element*> m_captionPreferencesChangedElements;
+#endif
 
-    HashMap<StringImpl*, Element*, CaseFoldingHash> m_elementsByAccessKey;    
+    HashMap<StringImpl*, Element*, CaseFoldingHash> m_elementsByAccessKey;
     bool m_accessKeyMapValid;
 
     OwnPtr<SelectorQueryCache> m_selectorQueryCache;
index 4627b69..6ca5b22 100644 (file)
@@ -482,6 +482,10 @@ public:
     virtual void didBecomeFullscreenElement() { }
     virtual void willStopBeingFullscreenElement() { }
 
+#if ENABLE(VIDEO_TRACK)
+    virtual void captionPreferencesChanged() { }
+#endif
+
     bool isFinishedParsingChildren() const { return isParsingChildrenFinished(); }
     virtual void finishParsingChildren();
     virtual void beginParsingChildren();
index 28d03e5..5a9eb00 100644 (file)
@@ -56,6 +56,7 @@ CachedPage::CachedPage(Page* page)
     , m_cachedMainFrame(CachedFrame::create(page->mainFrame()))
     , m_needStyleRecalcForVisitedLinks(false)
     , m_needsFullStyleRecalc(false)
+    , m_needsCaptionPreferencesChanged(false)
 {
 #ifndef NDEBUG
     cachedPageCounter.increment();
@@ -96,6 +97,11 @@ void CachedPage::restore(Page* page)
     if (m_needsFullStyleRecalc)
         page->setNeedsRecalcStyleInAllFrames();
 
+#if ENABLE(VIDEO_TRACK)
+    if (m_needsCaptionPreferencesChanged)
+        page->captionPreferencesChanged();
+#endif
+
     clear();
 }
 
index 118f94e..7cff161 100644 (file)
@@ -54,6 +54,9 @@ public:
 
     void markForVistedLinkStyleRecalc() { m_needStyleRecalcForVisitedLinks = true; }
     void markForFullStyleRecalc() { m_needsFullStyleRecalc = true; }
+#if ENABLE(VIDEO_TRACK)
+    void markForCaptionPreferencesChanged() { m_needsCaptionPreferencesChanged = true; }
+#endif
 
 private:
     CachedPage(Page*);
@@ -63,6 +66,7 @@ private:
     RefPtr<CachedFrame> m_cachedMainFrame;
     bool m_needStyleRecalcForVisitedLinks;
     bool m_needsFullStyleRecalc;
+    bool m_needsCaptionPreferencesChanged;
 };
 
 } // namespace WebCore
index 5c1562a..d667500 100644 (file)
@@ -436,6 +436,14 @@ void PageCache::markPagesForFullStyleRecalc(Page* page)
     }
 }
 
+#if ENABLE(VIDEO_TRACK)
+void PageCache::markPagesForCaptionPreferencesChanged()
+{
+    for (HistoryItem* current = m_head; current; current = current->m_next)
+        current->m_cachedPage->markForCaptionPreferencesChanged();
+}
+#endif
+
 void PageCache::add(PassRefPtr<HistoryItem> prpItem, Page* page)
 {
     ASSERT(prpItem);
index 06fd4d9..6ee014c 100644 (file)
@@ -60,6 +60,10 @@ namespace WebCore {
         // Will mark all cached pages associated with the given page as needing style recalc.
         void markPagesForFullStyleRecalc(Page*);
 
+#if ENABLE(VIDEO_TRACK)
+        void markPagesForCaptionPreferencesChanged();
+#endif
+
 #if USE(ACCELERATED_COMPOSITING)
         bool shouldClearBackingStores() const { return m_shouldClearBackingStores; }
         void setShouldClearBackingStores(bool flag) { m_shouldClearBackingStores = flag; }
index 9a5ae9a..ae2d3b5 100644 (file)
@@ -98,6 +98,7 @@
 #endif
 
 #if ENABLE(VIDEO_TRACK)
+#include "CaptionUserPreferences.h"
 #include "HTMLTrackElement.h"
 #include "InbandTextTrack.h"
 #include "InbandTextTrackPrivate.h"
@@ -305,7 +306,7 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document* docum
     LOG(Media, "HTMLMediaElement::HTMLMediaElement");
     document->registerForMediaVolumeCallbacks(this);
     document->registerForPrivateBrowsingStateChangedCallbacks(this);
-    
+
     if (document->settings() && document->settings()->mediaPlaybackRequiresUserGesture()) {
         addBehaviorRestriction(RequireUserGestureForRateChangeRestriction);
         addBehaviorRestriction(RequireUserGestureForLoadRestriction);
@@ -315,6 +316,7 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document* docum
     addElementToDocumentMap(this, document);
 
 #if ENABLE(VIDEO_TRACK)
+    document->registerForCaptionPreferencesChangedCallbacks(this);
     if (document->page()) {
         CaptionUserPreferences* captionPreferences = document->page()->group().captionPreferences();
         if (captionPreferences->userHasCaptionPreferences())
@@ -332,6 +334,7 @@ HTMLMediaElement::~HTMLMediaElement()
     document()->unregisterForMediaVolumeCallbacks(this);
     document()->unregisterForPrivateBrowsingStateChangedCallbacks(this);
 #if ENABLE(VIDEO_TRACK)
+    document()->unregisterForCaptionPreferencesChangedCallbacks(this);
     if (m_textTracks)
         m_textTracks->clearOwner();
     if (m_textTracks) {
@@ -591,20 +594,6 @@ void HTMLMediaElement::attach()
             frame->loader()->client()->hideMediaPlayerProxyPlugin(m_proxyWidget.get());
     }
 #endif
-
-#if ENABLE(VIDEO_TRACK)
-    if (document()->page())
-        document()->page()->group().captionPreferences()->registerForPreferencesChangedCallbacks(this);
-#endif
-}
-
-void HTMLMediaElement::detach()
-{
-#if ENABLE(VIDEO_TRACK)
-    if (document()->page())
-        document()->page()->group().captionPreferences()->unregisterForPreferencesChangedCallbacks(this);
-#endif
-    HTMLElement::detach();
 }
 
 void HTMLMediaElement::didRecalcStyle(StyleChange)
index 5827f19..08c2dc6 100644 (file)
@@ -39,7 +39,6 @@
 #endif
 
 #if ENABLE(VIDEO_TRACK)
-#include "CaptionUserPreferences.h"
 #include "PODIntervalTree.h"
 #include "TextTrack.h"
 #include "TextTrackCue.h"
@@ -84,7 +83,7 @@ typedef Vector<CueInterval> CueList;
 
 class HTMLMediaElement : public HTMLElement, public MediaPlayerClient, public MediaPlayerSupportsTypeClient, private MediaCanStartListener, public ActiveDOMObject, public MediaControllerInterface
 #if ENABLE(VIDEO_TRACK)
-    , private TextTrackClient, private CaptionPreferencesChangedListener
+    , private TextTrackClient
 #endif
 #if USE(PLATFORM_TEXT_TRACK_MENU)
     , public PlatformTextTrackMenuClient
@@ -365,7 +364,6 @@ protected:
     virtual void finishParsingChildren();
     virtual bool isURLAttribute(const Attribute&) const OVERRIDE;
     virtual void attach() OVERRIDE;
-    virtual void detach() OVERRIDE;
 
     virtual void didMoveToNewDocument(Document* oldDocument) OVERRIDE;
 
index 3199046..765854f 100644 (file)
 #if ENABLE(VIDEO_TRACK)
 
 #include "CaptionUserPreferences.h"
+#include "PageGroup.h"
 
 namespace WebCore {
 
-void CaptionUserPreferences::registerForPreferencesChangedCallbacks(CaptionPreferencesChangedListener* listener)
-{
-    m_captionPreferenceChangeListeners.add(listener);
-}
-
-void CaptionUserPreferences::unregisterForPreferencesChangedCallbacks(CaptionPreferencesChangedListener* listener)
-{
-    if (m_captionPreferenceChangeListeners.isEmpty())
-        return;
-
-    m_captionPreferenceChangeListeners.remove(listener);
-}
-
 void CaptionUserPreferences::setUserPrefersCaptions(bool preference)
 {
     m_userPrefersCaptions = preference;
@@ -55,11 +43,7 @@ void CaptionUserPreferences::setUserPrefersCaptions(bool preference)
 
 void CaptionUserPreferences::captionPreferencesChanged()
 {
-    if (m_captionPreferenceChangeListeners.isEmpty())
-        return;
-    
-    for (HashSet<CaptionPreferencesChangedListener*>::iterator i = m_captionPreferenceChangeListeners.begin(); i != m_captionPreferenceChangeListeners.end(); ++i)
-        (*i)->captionPreferencesChanged();
+    m_pageGroup->captionPreferencesChanged();
 }
 
 Vector<String> CaptionUserPreferences::preferredLanguages() const
index b57216c..c0577c8 100644 (file)
@@ -38,13 +38,6 @@ namespace WebCore {
 
 class PageGroup;
 
-class CaptionPreferencesChangedListener {
-public:
-    virtual void captionPreferencesChanged() = 0;
-protected:
-    virtual ~CaptionPreferencesChangedListener() { }
-};
-
 class CaptionUserPreferences {
 public:
     static PassOwnPtr<CaptionUserPreferences> create(PageGroup* group) { return adoptPtr(new CaptionUserPreferences(group)); }
@@ -56,10 +49,9 @@ public:
     virtual float captionFontSizeScale(bool& important) const { important = false; return 0.05f; }
     virtual String captionsStyleSheetOverride() const { return emptyString(); }
 
-    virtual void registerForPreferencesChangedCallbacks(CaptionPreferencesChangedListener*);
-    virtual void unregisterForPreferencesChangedCallbacks(CaptionPreferencesChangedListener*);
+    virtual void setInterestedInCaptionPreferenceChanges() { }
+
     virtual void captionPreferencesChanged();
-    bool havePreferenceChangeListeners() const { return !m_captionPreferenceChangeListeners.isEmpty(); }
 
     virtual void setPreferredLanguage(String);
     virtual Vector<String> preferredLanguages() const;
@@ -81,7 +73,6 @@ protected:
     }
 
 private:
-    HashSet<CaptionPreferencesChangedListener*> m_captionPreferenceChangeListeners;
     PageGroup* m_pageGroup;
     String m_userPreferredLanguage;
     bool m_testingMode;
index 5460a53..52b533a 100644 (file)
@@ -47,7 +47,7 @@ public:
     virtual float captionFontSizeScale(bool&) const OVERRIDE;
     virtual String captionsStyleSheetOverride() const OVERRIDE;
 
-    virtual void registerForPreferencesChangedCallbacks(CaptionPreferencesChangedListener*) OVERRIDE;
+    virtual void setInterestedInCaptionPreferenceChanges() OVERRIDE;
 
     virtual void setPreferredLanguage(String) OVERRIDE;
     virtual Vector<String> preferredLanguages() const OVERRIDE;
index ae25af1..ed32b7b 100644 (file)
@@ -137,10 +137,8 @@ bool CaptionUserPreferencesMac::userHasCaptionPreferences() const
     return true;
 }
 
-void CaptionUserPreferencesMac::registerForPreferencesChangedCallbacks(CaptionPreferencesChangedListener* listener)
+void CaptionUserPreferencesMac::setInterestedInCaptionPreferenceChanges()
 {
-    CaptionUserPreferences::registerForPreferencesChangedCallbacks(listener);
-
     if (!MediaAccessibilityLibrary())
         return;
 
@@ -157,7 +155,7 @@ void CaptionUserPreferencesMac::registerForPreferencesChangedCallbacks(CaptionPr
 
 void CaptionUserPreferencesMac::captionPreferencesChanged()
 {
-    if (havePreferenceChangeListeners())
+    if (m_listeningForPreferenceChanges)
         updateCaptionStyleSheetOveride();
 
     CaptionUserPreferences::captionPreferencesChanged();
index 22f312f..d3bdb0e 100644 (file)
@@ -1490,6 +1490,14 @@ void Page::reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const
     info.ignoreMember(m_validationMessageClient);
 }
 
+#if ENABLE(VIDEO_TRACK)
+void Page::captionPreferencesChanged()
+{
+    for (Frame* frame = mainFrame(); frame; frame = frame->tree()->traverseNext())
+        frame->document()->captionPreferencesChanged();
+}
+#endif
+
 Page::PageClients::PageClients()
     : alternativeTextClient(0)
     , chromeClient(0)
index 01cd1dc..c0c7318 100644 (file)
@@ -380,6 +380,10 @@ public:
 
     void reportMemoryUsage(MemoryObjectInfo*) const;
 
+#if ENABLE(VIDEO_TRACK)
+    void captionPreferencesChanged();
+#endif
+
 private:
     void initGroup();
 
index 60d0a42..f5bb21d 100644 (file)
@@ -417,6 +417,13 @@ void PageGroup::invalidatedInjectedStyleSheetCacheInAllFrames()
 }
 
 #if ENABLE(VIDEO_TRACK)
+void PageGroup::captionPreferencesChanged()
+{
+    for (HashSet<Page*>::iterator i = m_pages.begin(); i != m_pages.end(); ++i)
+        (*i)->captionPreferencesChanged();
+    pageCache()->markPagesForCaptionPreferencesChanged();
+}
+
 CaptionUserPreferences* PageGroup::captionPreferences()
 {
     if (!m_captionPreferences)
index e45c988..6831ded 100644 (file)
@@ -109,6 +109,7 @@ namespace WebCore {
         GroupSettings* groupSettings() const { return m_groupSettings.get(); }
 
 #if ENABLE(VIDEO_TRACK)
+        void captionPreferencesChanged();
         CaptionUserPreferences* captionPreferences();
 #endif