Summary: | Postpone caption style sheet creation | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||
Component: | Media | Assignee: | Eric Carlson <eric.carlson> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Eric Carlson
2015-05-01 13:58:08 PDT
Created attachment 252203 [details]
Proposed patch.
Comment on attachment 252203 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=252203&action=review > Source/WebCore/html/HTMLMediaElement.cpp:480 > + if (m_haveRegisteredForCaptionPreferenceChanges) > + document.unregisterForCaptionPreferencesChangedCallbacks(this); Does this need to set m_haveRegisteredForCaptionPreferenceChanges=false? Or maybe my suggested rename would clarify. > Source/WebCore/html/HTMLMediaElement.h:888 > + bool m_haveRegisteredForCaptionPreferenceChanges { false }; Would this be better as m_hasHadTextTrack? Comment on attachment 252203 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=252203&action=review >> Source/WebCore/html/HTMLMediaElement.cpp:480 >> + document.unregisterForCaptionPreferencesChangedCallbacks(this); > > Does this need to set m_haveRegisteredForCaptionPreferenceChanges=false? Or maybe my suggested rename would clarify. No, because we want to re-register if the element is inserted back into the DOM (like the layout test does). I will rename it. Comment on attachment 252203 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=252203&action=review > Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:200 > + // Generating and registering the caption stylesheet can be expensive and this method is called indirectly when the parser creates an audio or Maybe early return on if (m_listeningForPreferenceChanges) before this whole block. Committed r183705: https://trac.webkit.org/r183705 |