[webkit-reviews] review granted: [Bug 106972] Add a Setting to disable QTKit media engine. : [Attachment 182924] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted  review:
Bug 106972: Add a Setting to disable QTKit media engine.
https://bugs.webkit.org/show_bug.cgi?id=106972

Attachment 182924: Patch
https://bugs.webkit.org/attachment.cgi?id=182924&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=182924&action=review


>>> Source/WebCore/platform/graphics/MediaPlayer.cpp:229
>>> +
>> 
>> Having the check here means it won't really be possible to have a control in
an application's UI, because the user can't know if the media engines have
already been registered or not (eg. loading a document WebCore doesn't handle
natively has the side effect of registering all media engines to see if it is
possible to create a media document). I think it would be more useful to always
register the QTKit media engine, but then not use it when the pref is set by
checking engineDescription() in bestMediaEngineForTypeAndCodecs().
> 
> We don't have access to engineDescription() at that point, as
bestMediaEngineForTypeAndCodecs() just uses the MediaEngine factory methods,
not an instance of MediaPlayerPrivate.	But even if it were available, that
seems fragile (i.e., comparing strings returned from engineDescription()).
> 
> What if we recreate the list of media engine factories whenever one of those
preferences change?

Fair enough. Having to set the pref, quit, and relaunch is a pain but not the
end of the world.