| Summary: | [Mac] Disable QTKit by default. | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Said Abou-Hallawa <sabouhallawa> | ||||||||||||||
| Component: | New Bugs | Assignee: | Said Abou-Hallawa <sabouhallawa> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | buildbot, commit-queue, rniwa | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Said Abou-Hallawa
2015-06-26 11:02:14 PDT
Created attachment 255649 [details]
Patch
Comment on attachment 255649 [details]
Patch
This is wrong, it disables AVFoundation on all versions of OS X and does not change QTKit. Instead we should always set Settings::gQTKitEnabled to false and Settings::gAVFoundationEnabled to true.
Created attachment 255653 [details]
Patch
(In reply to comment #3) > Comment on attachment 255649 [details] > Patch > > This is wrong, it disables AVFoundation on all versions of OS X and does not > change QTKit. Instead we should always set Settings::gQTKitEnabled to false > and Settings::gAVFoundationEnabled to true. Sorry. Done. Comment on attachment 255653 [details] Patch Attachment 255653 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6021608846131200 New failing tests: http/tests/media/remove-while-loading.html http/tests/security/contentSecurityPolicy/media-src-allowed.html http/tests/media/video-served-as-text.html http/tests/media/video-accept-encoding.html http/tests/media/video-throttled-load-metadata.html http/tests/media/video-cancel-load.html Created attachment 255657 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 255653 [details] Patch Attachment 255653 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6492062383865856 New failing tests: http/tests/media/remove-while-loading.html http/tests/security/contentSecurityPolicy/media-src-allowed.html http/tests/media/video-served-as-text.html http/tests/media/video-accept-encoding.html http/tests/media/video-throttled-load-metadata.html http/tests/media/video-cancel-load.html Created attachment 255659 [details]
Archive of layout-test-results from ews101 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 255672 [details]
Patch
Comment on attachment 255672 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255672&action=review > Source/WebCore/ChangeLog:13 > + * page/Settings.cpp: > + (WebCore::invalidateAfterGenericFamilyChange): Remove the requirement for > + enabling QTKit and AVFoundation. QTKit should be disabled always. And > + AVFoundation should be enabled always. Should gAVFoundationEnabled exist at all any more, then? Created attachment 255749 [details]
Patch
(In reply to comment #11) > Comment on attachment 255672 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=255672&action=review > > > Source/WebCore/ChangeLog:13 > > + * page/Settings.cpp: > > + (WebCore::invalidateAfterGenericFamilyChange): Remove the requirement for > > + enabling QTKit and AVFoundation. QTKit should be disabled always. And > > + AVFoundation should be enabled always. > > Should gAVFoundationEnabled exist at all any more, then? My comment was unclear. It is now fixed. This patch is fixing the initial state of enabling AVFoundation and QTKit. Both flags can exist in the same build: if USE(AVFOUNDATION) and PLATFORM(COCOA) are both true. They can register different media engines at the same time; see buildMediaEnginesVector(). And their values can be changed later separately through Settings::setAVFoundationEnabled() and Settings::setQTKitEnabled(). So I think merging them in one flag is incorrect or at least will affect the clarity of the code. Comment on attachment 255749 [details] Patch Clearing flags on attachment: 255749 Committed r186071: <http://trac.webkit.org/changeset/186071> All reviewed patches have been landed. Closing bug. |