Bug 185839

Summary: Avoid loading AVFoundation to check supported MIME types if possible
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: ASSIGNED ---    
Severity: Normal CC: commit-queue, ews-watchlist, jer.noble, rniwa, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews103 for mac-sierra
none
Archive of layout-test-results from ews113 for mac-sierra
none
Archive of layout-test-results from ews107 for mac-sierra-wk2
none
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
Patch
none
Patch
jer.noble: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews100 for mac-sierra
none
Archive of layout-test-results from ews104 for mac-sierra-wk2
none
Archive of layout-test-results from ews126 for ios-simulator-wk2
none
Archive of layout-test-results from ews113 for mac-sierra
none
Patch for landing
ews-watchlist: commit-queue-
Archive of layout-test-results from ews200 for win-future none

Description Eric Carlson 2018-05-21 14:17:54 PDT
Loading AVFoundation to call +[AVURLAssetClass audiovisualMIMETypes] triggers an allocation of more than 2MB. Move the call to the UI process and pass the list of types to each new web process so we can do it once per application instead of once per web process.
Comment 1 Eric Carlson 2018-05-21 14:18:54 PDT
<rdar://problem/40182010>
Comment 2 Eric Carlson 2018-05-21 14:39:45 PDT
Created attachment 340895 [details]
Patch
Comment 3 Simon Fraser (smfr) 2018-05-21 14:47:46 PDT
Comment on attachment 340895 [details]
Patch

How much time does this add to Safari launch on a slow iOS device? Before, we would do this lazily in the web process when we found an extension we didn't know how to handle. Now, you're paying the cost every time. I'm not sure this is an improvement.
Comment 4 Eric Carlson 2018-05-21 14:53:42 PDT
Comment on attachment 340895 [details]
Patch

Removing r?, this needs to be revised to work in WK1.
Comment 5 Jer Noble 2018-05-21 14:54:47 PDT
Comment on attachment 340895 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=340895&action=review

So it appears that, as it stands, this patch will break WebKitLegacy, since there's no code path for it to initialize the AVFoundationMIMETypeCache. One idea to circumvent this problem would be to leave the AVFoundationMIMETypeCache implementation as-is, but add a "non-initializing singleton()" accessor that WK2 could use to fill the cache with pre-initialized values.

> Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:361
> -    if (!supportsMediaType(MediaType::Video))
> -        return nullptr;
> +    if (AVFoundationMIMETypeCache::singleton().types().contains(mimeType))
> +        return true;

I would change this sightly:

    ContentType type  { mimeType };
    if (!AVFoundationMIMETypeCache::singleton().types().contains(type.contentType()))
        return flase;

That way, if the content-type portion of the MIME isn't an AVFoundation supported type, we don't bother asking the AVURLAsset for something which it's not going to support.
Comment 6 EWS Watchlist 2018-05-21 15:20:49 PDT
Comment on attachment 340895 [details]
Patch

Attachment 340895 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7757570

Number of test failures exceeded the failure limit.
Comment 7 EWS Watchlist 2018-05-21 15:20:50 PDT
Created attachment 340907 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 8 EWS Watchlist 2018-05-21 15:24:18 PDT
Comment on attachment 340895 [details]
Patch

Attachment 340895 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/7757505

Number of test failures exceeded the failure limit.
Comment 9 EWS Watchlist 2018-05-21 15:24:19 PDT
Created attachment 340908 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 10 EWS Watchlist 2018-05-21 16:02:15 PDT
Comment on attachment 340895 [details]
Patch

Attachment 340895 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7757907

New failing tests:
fast/images/animated-image-mp4.html
http/tests/images/image-supports-video.html
fast/images/animated-image-mp4-crash.html
Comment 11 EWS Watchlist 2018-05-21 16:02:17 PDT
Created attachment 340915 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 12 EWS Watchlist 2018-05-21 16:20:08 PDT
Comment on attachment 340895 [details]
Patch

Attachment 340895 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/7757756

New failing tests:
fast/images/animated-image-mp4.html
fast/images/animated-image-mp4-crash.html
Comment 13 EWS Watchlist 2018-05-21 16:20:10 PDT
Created attachment 340919 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 14 Eric Carlson 2018-05-23 13:41:16 PDT
*** Bug 185817 has been marked as a duplicate of this bug. ***
Comment 15 Eric Carlson 2018-05-23 13:42:33 PDT
Created attachment 341124 [details]
Patch
Comment 16 Eric Carlson 2018-05-23 13:59:14 PDT
Created attachment 341126 [details]
Patch
Comment 17 Jer Noble 2018-05-23 14:07:39 PDT
Comment on attachment 341126 [details]
Patch

r=me with a general nit. Rather than check whether the m_cache.isEmpty() to see whether it's been initialized, it might be better to have a "bool m_isInitialized" or change m_cache to an std::optional. That way, you can disambiguate between "the list of supported mime types is empty" and "the cache hasn't been initialized yet".
Comment 18 EWS Watchlist 2018-05-23 14:50:14 PDT
Comment on attachment 341126 [details]
Patch

Attachment 341126 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7780928

New failing tests:
fast/images/animated-image-mp4-crash.html
http/tests/images/image-supports-video.html
fast/images/animated-image-mp4.html
Comment 19 EWS Watchlist 2018-05-23 14:50:15 PDT
Created attachment 341132 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 20 EWS Watchlist 2018-05-23 15:03:08 PDT
Comment on attachment 341126 [details]
Patch

Attachment 341126 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7780953

New failing tests:
fast/images/animated-image-mp4.html
fast/images/animated-image-mp4-crash.html
http/tests/images/image-supports-video.html
Comment 21 EWS Watchlist 2018-05-23 15:03:10 PDT
Created attachment 341133 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 22 EWS Watchlist 2018-05-23 15:47:20 PDT
Comment on attachment 341126 [details]
Patch

Attachment 341126 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/7781188

New failing tests:
fast/images/animated-image-mp4-crash.html
fast/images/animated-image-mp4.html
Comment 23 EWS Watchlist 2018-05-23 15:47:22 PDT
Created attachment 341138 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 24 EWS Watchlist 2018-05-23 15:48:15 PDT
Comment on attachment 341126 [details]
Patch

Attachment 341126 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/7781239

New failing tests:
fast/images/animated-image-mp4-crash.html
fast/images/animated-image-mp4.html
Comment 25 EWS Watchlist 2018-05-23 15:48:16 PDT
Created attachment 341139 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 26 Eric Carlson 2018-05-23 16:01:01 PDT
Created attachment 341142 [details]
Patch for landing
Comment 27 WebKit Commit Bot 2018-05-23 17:01:00 PDT
Comment on attachment 341142 [details]
Patch for landing

Clearing flags on attachment: 341142

Committed r232136: <https://trac.webkit.org/changeset/232136>
Comment 28 EWS Watchlist 2018-05-23 17:42:00 PDT
Comment on attachment 341142 [details]
Patch for landing

Attachment 341142 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7782529

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html
Comment 29 EWS Watchlist 2018-05-23 17:42:12 PDT
Created attachment 341152 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 30 Simon Fraser (smfr) 2018-05-23 20:44:20 PDT
Could you address my comment #3 please?
Comment 31 Eric Carlson 2018-05-24 10:46:18 PDT
(In reply to Simon Fraser (smfr) from comment #30)
> Could you address my comment #3 please?

Please read the patch.