WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
144499
Postpone caption style sheet creation
https://bugs.webkit.org/show_bug.cgi?id=144499
Summary
Postpone caption style sheet creation
Eric Carlson
Reported
2015-05-01 13:58:08 PDT
CaptionUserPreferences::updateCaptionStyleSheetOverride() is called when a <video> element is created. This is a fairly expensive operation and the style sheet it generates is only needed when the <video> has captions, so it should be postponed as long as possible.
Attachments
Proposed patch.
(9.42 KB, patch)
2015-05-01 17:19 PDT
,
Eric Carlson
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Eric Carlson
Comment 1
2015-05-01 13:58:34 PDT
rdar://problem/20691225
Eric Carlson
Comment 2
2015-05-01 17:19:51 PDT
Created
attachment 252203
[details]
Proposed patch.
Simon Fraser (smfr)
Comment 3
2015-05-01 17:32:56 PDT
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?
Eric Carlson
Comment 4
2015-05-01 17:41:28 PDT
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.
Simon Fraser (smfr)
Comment 5
2015-05-01 17:55:01 PDT
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.
Eric Carlson
Comment 6
2015-05-01 18:16:01 PDT
Committed
r183705
:
https://trac.webkit.org/r183705
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug